ref(pyramid): Errorhandlers now swallow events (getsentry#382)

untitaker · web-flow · commit a7b6623486c2 · 2019-06-01T20:26:12.000+02:00
Exceptions caught by errorhandlers ("exception handling tweens" or whatever) are now not reported unless the errorhandler returned a 500 response.

Exceptions *raising* from errorhandlers are still reported.
diff --git a/sentry_sdk/integrations/pyramid.py b/sentry_sdk/integrations/pyramid.py
@@ -59,30 +59,42 @@ def __init__(self, transaction_style="route_name"):
     def setup_once():
         # type: () -> None
         from pyramid.router import Router  # type: ignore
+        from pyramid.request import Request  # type: ignore
 
         old_handle_request = Router.handle_request
 
         def sentry_patched_handle_request(self, request, *args, **kwargs):
             # type: (Any, Request, *Any, **Any) -> Response
             hub = Hub.current
             integration = hub.get_integration(PyramidIntegration)
-            if integration is None:
-                return old_handle_request(self, request, *args, **kwargs)
-
-            with hub.configure_scope() as scope:
-                scope.add_event_processor(
-                    _make_event_processor(weakref.ref(request), integration)
-                )
+            if integration is not None:
+                with hub.configure_scope() as scope:
+                    scope.add_event_processor(
+                        _make_event_processor(weakref.ref(request), integration)
+                    )
 
-            try:
-                return old_handle_request(self, request, *args, **kwargs)
-            except Exception:
-                exc_info = sys.exc_info()
-                _capture_exception(exc_info)
-                reraise(*exc_info)
+            return old_handle_request(self, request, *args, **kwargs)
 
         Router.handle_request = sentry_patched_handle_request
 
+        if hasattr(Request, "invoke_exception_view"):
+            old_invoke_exception_view = Request.invoke_exception_view
+
+            def sentry_patched_invoke_exception_view(self, *args, **kwargs):
+                rv = old_invoke_exception_view(self, *args, **kwargs)
+
+                if (
+                    self.exc_info
+                    and all(self.exc_info)
+                    and rv.status_int == 500
+                    and Hub.current.get_integration(PyramidIntegration) is not None
+                ):
+                    _capture_exception(self.exc_info)
+
+                return rv
+
+            Request.invoke_exception_view = sentry_patched_invoke_exception_view
+
         old_wsgi_call = Router.__call__
 
         def sentry_patched_wsgi_call(self, environ, start_response):
@@ -92,15 +104,23 @@ def sentry_patched_wsgi_call(self, environ, start_response):
             if integration is None:
                 return old_wsgi_call(self, environ, start_response)
 
-            return SentryWsgiMiddleware(lambda *a, **kw: old_wsgi_call(self, *a, **kw))(
+            def sentry_patched_inner_wsgi_call(environ, start_response):
+                try:
+                    return old_wsgi_call(self, environ, start_response)
+                except Exception:
+                    einfo = sys.exc_info()
+                    _capture_exception(einfo)
+                    reraise(*einfo)
+
+            return SentryWsgiMiddleware(sentry_patched_inner_wsgi_call)(
                 environ, start_response
             )
 
         Router.__call__ = sentry_patched_wsgi_call
 
 
-def _capture_exception(exc_info, **kwargs):
-    # type: (ExcInfo, **Any) -> None
+def _capture_exception(exc_info):
+    # type: (ExcInfo) -> None
     if exc_info[0] is None or issubclass(exc_info[0], HTTPException):
         return
     hub = Hub.current
diff --git a/tests/integrations/pyramid/test_pyramid.py b/tests/integrations/pyramid/test_pyramid.py
@@ -1,21 +1,25 @@
 import json
-
-from io import BytesIO
-
 import logging
-
+import pkg_resources
 import pytest
 
-from pyramid.authorization import ACLAuthorizationPolicy
+from io import BytesIO
+
 import pyramid.testing
 
+from pyramid.authorization import ACLAuthorizationPolicy
 from pyramid.response import Response
 
-from werkzeug.test import Client
-
 from sentry_sdk import capture_message, add_breadcrumb
 from sentry_sdk.integrations.pyramid import PyramidIntegration
 
+from werkzeug.test import Client
+
+
+PYRAMID_VERSION = tuple(
+    map(int, pkg_resources.get_distribution("pyramid").version.split("."))
+)
+
 
 def hi(request):
     capture_message("hi")
@@ -219,7 +223,7 @@ def errorhandler(exc, request):
     assert not events
 
 
-def test_errorhandler(
+def test_errorhandler_ok(
     sentry_init, pyramid_config, capture_exceptions, route, get_client
 ):
     sentry_init(integrations=[PyramidIntegration()])
@@ -237,8 +241,36 @@ def errorhandler(exc, request):
     client = get_client()
     client.get("/")
 
+    assert not errors
+
+
+@pytest.mark.skipif(
+    PYRAMID_VERSION < (1, 9),
+    reason="We don't have the right hooks in older Pyramid versions",
+)
+def test_errorhandler_500(
+    sentry_init, pyramid_config, capture_exceptions, route, get_client
+):
+    sentry_init(integrations=[PyramidIntegration()])
+    errors = capture_exceptions()
+
+    @route("/")
+    def index(request):
+        1 / 0
+
+    def errorhandler(exc, request):
+        return Response("bad request", status=500)
+
+    pyramid_config.add_view(errorhandler, context=Exception)
+
+    client = get_client()
+    app_iter, status, headers = client.get("/")
+    assert b"".join(app_iter) == b"bad request"
+    assert status.lower() == "500 internal server error"
+
     error, = errors
-    assert type(error) is Exception
+
+    assert isinstance(error, ZeroDivisionError)
 
 
 def test_error_in_errorhandler(
@@ -262,12 +294,9 @@ def error_handler(err, request):
     with pytest.raises(ZeroDivisionError):
         client.get("/")
 
-    event1, event2 = events
-
-    exception, = event1["exception"]["values"]
-    assert exception["type"] == "ValueError"
+    event, = events
 
-    exception = event2["exception"]["values"][-1]
+    exception = event["exception"]["values"][-1]
     assert exception["type"] == "ZeroDivisionError"
 
 
