Fixed after_request for some obscure case.

3 files changed, 94 insertions, 27 deletions

diff --git a/bottle.py b/bottle.py
index fb74ee2..2673676 100755
--- a/bottle.py
+++ b/bottle.py
@@ -968,40 +968,44 @@ class Bottle(object):
         response.bind()
 
         try:
-            self.trigger_hook('before_request')
-        except HTTPResponse as E:
-            E.apply(response)
-            return E
-
-        try:
-            out = None
-            while True:
+            while True: # Remove in 0.14 together with RouteReset
+                out = None
                 try:
+                    self.trigger_hook('before_request')
                     route, args = self.router.match(environ)
                     environ['route.handle'] = route
                     environ['bottle.route'] = route
                     environ['route.url_args'] = args
                     out = route.call(**args)
+                    break
                 except HTTPResponse as E:
                     out = E
+                    break
                 except RouteReset:
+                    depr(0, 13, "RouteReset exception deprecated",
+                                "Call route.call() after route.reset() and "
+                                "return the result.")
                     route.reset()
                     continue
-                except (KeyboardInterrupt, SystemExit, MemoryError):
-                    raise
-                except Exception as E:
-                    if not self.catchall: raise
-                    stacktrace = format_exc()
-                    environ['wsgi.errors'].write(stacktrace)
-                    environ['wsgi.errors'].flush()
-                    out = HTTPError(500, "Internal Server Error", E, stacktrace)
-                break
+                finally:
+                    if isinstance(out, HTTPResponse):
+                        out.apply(response)
+                    try:
+                        self.trigger_hook('after_request')
+                    except HTTPResponse as E:
+                        out = E
+                        out.apply(response)
+        except (KeyboardInterrupt, SystemExit, MemoryError):
+            raise
+        except Exception as E:
+            if not self.catchall: raise
+            stacktrace = format_exc()
+            environ['wsgi.errors'].write(stacktrace)
+            environ['wsgi.errors'].flush()
+            out = HTTPError(500, "Internal Server Error", E, stacktrace)
+            out.apply(response)
 
-            if isinstance(out, HTTPResponse):
-                out.apply(response)
-            return out
-        finally:
-            self.trigger_hook('after_request')
+        return out
 
     def _cast(self, out, peek=None):
         """ Try to convert the parameter into something WSGI compatible and set
diff --git a/test/test_wsgi.py b/test/test_wsgi.py
index 88ff42c..bfc6bf3 100755
--- a/test/test_wsgi.py
+++ b/test/test_wsgi.py
@@ -264,18 +264,81 @@ class TestRouteDecorator(ServerTestBase):
         self.assertBody('before', '/test')
         self.assertHeader('X-Hook', 'after', '/test')
 
-    def test_environ_reflects_correct_response_after_request_in_hooks(self):
+    def test_after_request_sees_HTTPError_response(self):
         """ Issue #671  """
+        called = []
 
         @bottle.hook('after_request')
-        def log_request_a():
-            self.assertStatus(400)
-            self.assertBody("test")
+        def after_request():
+            called.append('after')
+            self.assertEqual(400, bottle.response.status_code)
 
         @bottle.get('/')
         def _get():
+            called.append("route")
             bottle.abort(400, 'test')
 
+        self.urlopen("/")
+        self.assertEqual(["route", "after"], called)
+
+    def test_after_request_hooks_run_after_exception(self):
+        """ Issue #671  """
+        called = []
+
+        @bottle.hook('before_request')
+        def before_request():
+            called.append('before')
+
+        @bottle.hook('after_request')
+        def after_request():
+            called.append('after')
+
+        @bottle.get('/')
+        def _get():
+            called.append("route")
+            1/0
+
+        self.urlopen("/")
+        self.assertEqual(["before", "route", "after"], called)
+
+    def test_after_request_hooks_run_after_exception_in_before_hook(self):
+        """ Issue #671  """
+        called = []
+
+        @bottle.hook('before_request')
+        def before_request():
+            called.append('before')
+            1 / 0
+
+        @bottle.hook('after_request')
+        def after_request():
+            called.append('after')
+
+        @bottle.get('/')
+        def _get():
+            called.append("route")
+
+        self.urlopen("/")
+        self.assertEqual(["before", "after"], called)
+
+    def test_after_request_hooks_may_rise_response_exception(self):
+        """ Issue #671  """
+        called = []
+
+        @bottle.hook('after_request')
+        def after_request():
+            called.append('after')
+            bottle.abort(400, "hook_content")
+
+        @bottle.get('/')
+        def _get():
+            called.append("route")
+            return "XXX"
+
+        self.assertInBody("hook_content", "/")
+        self.assertEqual(["route", "after"], called)
+
+
     def test_template(self):
         @bottle.route(template='test {{a}} {{b}}')
         def test(): return dict(a=5, b=6)
diff --git a/test/tools.py b/test/tools.py
index 48ac93a..832624b 100755
--- a/test/tools.py
+++ b/test/tools.py
@@ -97,7 +97,7 @@ class ServerTestBase(unittest.TestCase):
 
     def urlopen(self, path, method='GET', post='', env=None):
         result = {'code':0, 'status':'error', 'header':{}, 'body':tob('')}
-        def start_response(status, header):
+        def start_response(status, header, exc_info=None):
             result['code'] = int(status.split()[0])
             result['status'] = status.split(None, 1)[-1]
             for name, value in header: