Merge pull request #3029 from mmerickel/clear-request.exception

clear request.exception if the excview fails to handle the error

2 files changed, 61 insertions, 42 deletions

diff --git a/pyramid/tests/test_tweens.py b/pyramid/tests/test_tweens.py
index c8eada34c..2e74ad7cf 100644
--- a/pyramid/tests/test_tweens.py
+++ b/pyramid/tests/test_tweens.py
@@ -22,6 +22,8 @@ class Test_excview_tween_factory(unittest.TestCase):
         request = DummyRequest()
         result = tween(request)
         self.assertTrue(result is dummy_response)
+        self.assertIsNone(request.exception)
+        self.assertIsNone(request.exc_info)
 
     def test_it_catches_notfound(self):
         from pyramid.request import Request
@@ -31,8 +33,11 @@ class Test_excview_tween_factory(unittest.TestCase):
             raise HTTPNotFound
         tween = self._makeOne(handler)
         request = Request.blank('/')
+        request.registry = self.config.registry
         result = tween(request)
         self.assertEqual(result.status, '404 Not Found')
+        self.assertIsInstance(request.exception, HTTPNotFound)
+        self.assertEqual(request.exception, request.exc_info[1])
 
     def test_it_catches_with_predicate(self):
         from pyramid.request import Request
@@ -44,8 +49,11 @@ class Test_excview_tween_factory(unittest.TestCase):
             raise ValueError
         tween = self._makeOne(handler)
         request = Request.blank('/')
+        request.registry = self.config.registry
         result = tween(request)
         self.assertTrue(b'foo' in result.body)
+        self.assertIsInstance(request.exception, ValueError)
+        self.assertEqual(request.exception, request.exc_info[1])
 
     def test_it_reraises_on_mismatch(self):
         from pyramid.request import Request
@@ -55,8 +63,11 @@ class Test_excview_tween_factory(unittest.TestCase):
             raise ValueError
         tween = self._makeOne(handler)
         request = Request.blank('/')
+        request.registry = self.config.registry
         request.method = 'POST'
         self.assertRaises(ValueError, lambda: tween(request))
+        self.assertIsNone(request.exception)
+        self.assertIsNone(request.exc_info)
 
     def test_it_reraises_on_no_match(self):
         from pyramid.request import Request
@@ -64,10 +75,14 @@ class Test_excview_tween_factory(unittest.TestCase):
             raise ValueError
         tween = self._makeOne(handler)
         request = Request.blank('/')
+        request.registry = self.config.registry
         self.assertRaises(ValueError, lambda: tween(request))
+        self.assertIsNone(request.exception)
+        self.assertIsNone(request.exc_info)
 
 class DummyRequest:
-    pass
+    exception = None
+    exc_info = None
 
 class DummyResponse:
     pass
diff --git a/pyramid/tweens.py b/pyramid/tweens.py
index a842b1133..673429b06 100644
--- a/pyramid/tweens.py
+++ b/pyramid/tweens.py
@@ -10,6 +10,50 @@ from pyramid.interfaces import (
 from zope.interface import providedBy
 from pyramid.view import _call_view
 
+def _error_handler(request, exc):
+    # NOTE: we do not need to delete exc_info because this function
+    # should never be in the call stack of the exception
+    exc_info = sys.exc_info()
+
+    attrs = request.__dict__
+    attrs['exc_info'] = exc_info
+    attrs['exception'] = exc
+    # clear old generated request.response, if any; it may
+    # have been mutated by the view, and its state is not
+    # sane (e.g. caching headers)
+    if 'response' in attrs:
+        del attrs['response']
+    # we use .get instead of .__getitem__ below due to
+    # https://github.com/Pylons/pyramid/issues/700
+    request_iface = attrs.get('request_iface', IRequest)
+    provides = providedBy(exc)
+    try:
+        response = _call_view(
+            request.registry,
+            request,
+            exc,
+            provides,
+            '',
+            view_classifier=IExceptionViewClassifier,
+            request_iface=request_iface.combined
+            )
+
+    # if views matched but did not pass predicates then treat the
+    # same as not finding any matching views
+    except PredicateMismatch:
+        response = None
+
+    # re-raise the original exception as no exception views were
+    # able to handle the error
+    if response is None:
+        if 'exception' in attrs:
+            del attrs['exception']
+        if 'exc_info' in attrs:
+            del attrs['exc_info']
+        reraise(*exc_info)
+
+    return response
+
 def excview_tween_factory(handler, registry):
     """ A :term:`tween` factory which produces a tween that catches an
     exception raised by downstream tweens (or the main Pyramid request
@@ -17,50 +61,10 @@ def excview_tween_factory(handler, registry):
     :term:`exception view`."""
 
     def excview_tween(request):
-        attrs = request.__dict__
         try:
             response = handler(request)
         except Exception as exc:
-            # WARNING: do not assign the result of sys.exc_info() to a local
-            # var here, doing so will cause a leak.  We used to actually
-            # explicitly delete both "exception" and "exc_info" from ``attrs``
-            # in a ``finally:`` clause below, but now we do not because these
-            # attributes are useful to upstream tweens.  This actually still
-            # apparently causes a reference cycle, but it is broken
-            # successfully by the garbage collector (see
-            # https://github.com/Pylons/pyramid/issues/1223).
-            attrs['exc_info'] = sys.exc_info()
-            attrs['exception'] = exc
-            # clear old generated request.response, if any; it may
-            # have been mutated by the view, and its state is not
-            # sane (e.g. caching headers)
-            if 'response' in attrs:
-                del attrs['response']
-            # we use .get instead of .__getitem__ below due to
-            # https://github.com/Pylons/pyramid/issues/700
-            request_iface = attrs.get('request_iface', IRequest)
-            provides = providedBy(exc)
-            try:
-                response = _call_view(
-                    registry,
-                    request,
-                    exc,
-                    provides,
-                    '',
-                    view_classifier=IExceptionViewClassifier,
-                    request_iface=request_iface.combined
-                    )
-
-            # if views matched but did not pass predicates, squash the error
-            # and re-raise the original exception
-            except PredicateMismatch:
-                response = None
-
-            # re-raise the original exception as no exception views were
-            # able to handle the error
-            if response is None:
-                reraise(*attrs['exc_info'])
-
+            response = _error_handler(request, exc)
         return response
 
     return excview_tween