fix(integrations): Falcon integration checks response status before reporting error (#2465)

* Falcon checks actual HTTP status before reporting error

* Only support custom error handlers on Falcon 3+

* Add Falcon 3.1 to tox.ini

This change fixes an issue where the Falcon integration would report an error occurring in a Falcon request handler to Sentry, even though a Falcon custom event handler was handling the exception, causing an HTTP status other than 5xx to be returned. From now on, Falcon will inspect the HTTP status on the response before sending the associated error event to Sentry, and the error will only be reported if the response status is a 5xx status.

Fixes GH-#1362

Author: szokeasaurusrex

sentry_sdk/integrations/falcon.py

@@ -175,17 +175,25 @@ def sentry_patched_handle_exception(self, *args):
         # NOTE(jmagnusson): falcon 2.0 changed falcon.API._handle_exception
         # method signature from `(ex, req, resp, params)` to
         # `(req, resp, ex, params)`
-        if isinstance(args[0], Exception):
-            ex = args[0]
-        else:
-            ex = args[2]
+        ex = response = None
+        with capture_internal_exceptions():
+            ex = next(argument for argument in args if isinstance(argument, Exception))
+            response = next(
+                argument for argument in args if isinstance(argument, falcon.Response)
+            )
 
         was_handled = original_handle_exception(self, *args)
 
+        if ex is None or response is None:
+            # Both ex and response should have a non-None value at this point; otherwise,
+            # there is an error with the SDK that will have been captured in the
+            # capture_internal_exceptions block above.
+            return was_handled
+
         hub = Hub.current
         integration = hub.get_integration(FalconIntegration)
 
-        if integration is not None and _exception_leads_to_http_5xx(ex):
+        if integration is not None and _exception_leads_to_http_5xx(ex, response):
             # If an integration is there, a client has to be there.
             client = hub.client  # type: Any
 
@@ -225,15 +233,28 @@ def sentry_patched_prepare_middleware(
     falcon_helpers.prepare_middleware = sentry_patched_prepare_middleware
 
 
-def _exception_leads_to_http_5xx(ex):
-    # type: (Exception) -> bool
+def _exception_leads_to_http_5xx(ex, response):
+    # type: (Exception, falcon.Response) -> bool
     is_server_error = isinstance(ex, falcon.HTTPError) and (ex.status or "").startswith(
         "5"
     )
     is_unhandled_error = not isinstance(
         ex, (falcon.HTTPError, falcon.http_status.HTTPStatus)
     )
-    return is_server_error or is_unhandled_error
+
+    # We only check the HTTP status on Falcon 3 because in Falcon 2, the status on the response
+    # at the stage where we capture it is listed as 200, even though we would expect to see a 500
+    # status. Since at the time of this change, Falcon 2 is ca. 4 years old, we have decided to
+    # only perform this check on Falcon 3+, despite the risk that some handled errors might be
+    # reported to Sentry as unhandled on Falcon 2.
+    return (is_server_error or is_unhandled_error) and (
+        not FALCON3 or _has_http_5xx_status(response)
+    )
+
+
+def _has_http_5xx_status(response):
+    # type: (falcon.Response) -> bool
+    return response.status.startswith("5")
 
 
 def _set_transaction_name_and_source(event, transaction_style, request):

tests/integrations/falcon/test_falcon.py

@@ -9,6 +9,7 @@
 import sentry_sdk
 from sentry_sdk.integrations.falcon import FalconIntegration
 from sentry_sdk.integrations.logging import LoggingIntegration
+from sentry_sdk.utils import parse_version
 
 
 try:
@@ -19,6 +20,9 @@
     import falcon.inspect  # We only need this module for the ASGI test
 
 
+FALCON_VERSION = parse_version(falcon.__version__)
+
+
 @pytest.fixture
 def make_app(sentry_init):
     def inner():
@@ -32,9 +36,22 @@ def on_get(self, req, resp, message_id):
                 sentry_sdk.capture_message("hi")
                 resp.media = "hi"
 
+        class CustomError(Exception):
+            pass
+
+        class CustomErrorResource:
+            def on_get(self, req, resp):
+                raise CustomError()
+
+        def custom_error_handler(*args, **kwargs):
+            raise falcon.HTTPError(status=falcon.HTTP_400)
+
         app = falcon.API()
         app.add_route("/message", MessageResource())
         app.add_route("/message/{message_id:int}", MessageByIdResource())
+        app.add_route("/custom-error", CustomErrorResource())
+
+        app.add_error_handler(CustomError, custom_error_handler)
 
         return app
 
@@ -418,3 +435,23 @@ def test_falcon_not_breaking_asgi(sentry_init):
         falcon.inspect.inspect_app(asgi_app)
     except TypeError:
         pytest.fail("Falcon integration causing errors in ASGI apps.")
+
+
+@pytest.mark.skipif(
+    (FALCON_VERSION or ()) < (3,),
+    reason="The Sentry Falcon integration only supports custom error handlers on Falcon 3+",
+)
+def test_falcon_custom_error_handler(sentry_init, make_app, capture_events):
+    """
+    When a custom error handler handles what otherwise would have resulted in a 5xx error,
+    changing the HTTP status to a non-5xx status, no error event should be sent to Sentry.
+    """
+    sentry_init(integrations=[FalconIntegration()])
+    events = capture_events()
+
+    app = make_app()
+    client = falcon.testing.TestClient(app)
+
+    client.simulate_get("/custom-error")
+
+    assert len(events) == 0

tox.ini

@@ -81,6 +81,7 @@ envlist =
     {py2.7,py3.5,py3.6,py3.7}-falcon-v{1.4}
     {py2.7,py3.5,py3.6,py3.7}-falcon-v{2.0}
     {py3.5,py3.6,py3.7,py3.8,py3.9,py3.10,py3.11}-falcon-v{3.0}
+    {py3.7,py3.8,py3.9,py3.10,py3.11}-falcon-v{3.1}
 
     # FastAPI
     {py3.7,py3.8,py3.9,py3.10,py3.11}-fastapi
@@ -312,6 +313,7 @@ deps =
     falcon-v1.4: falcon>=1.4,<1.5
     falcon-v2.0: falcon>=2.0.0rc3,<3.0
     falcon-v3.0: falcon>=3.0.0,<3.1.0
+    falcon-v3.1: falcon>=3.1.0,<3.2
 
     # FastAPI
     fastapi: fastapi