diff --git a/sentry_sdk/integrations/falcon.py b/sentry_sdk/integrations/falcon.py index 7f360544ba..f1abb9c9d9 100644 --- a/sentry_sdk/integrations/falcon.py +++ b/sentry_sdk/integrations/falcon.py @@ -15,7 +15,7 @@ ) if TYPE_CHECKING: - from typing import Any, Dict, Optional + from typing import Any, Dict, Optional, Union from sentry_sdk._types import Event, EventProcessor @@ -237,8 +237,8 @@ def sentry_patched_prepare_middleware( def _exception_leads_to_http_5xx(ex: Exception, response: "falcon.Response") -> bool: - is_server_error = isinstance(ex, falcon.HTTPError) and (ex.status or "").startswith( - "5" + is_server_error = isinstance(ex, falcon.HTTPError) and _is_http_5xx_status( + ex.status or "" ) is_unhandled_error = not isinstance( ex, (falcon.HTTPError, falcon.http_status.HTTPStatus) @@ -250,12 +250,15 @@ def _exception_leads_to_http_5xx(ex: Exception, response: "falcon.Response") -> # 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) + not FALCON3 or _is_http_5xx_status(response.status) ) -def _has_http_5xx_status(response: "falcon.Response") -> bool: - return response.status.startswith("5") +def _is_http_5xx_status(status: "Union[int, str]") -> bool: + if isinstance(status, int): + return 500 <= status < 600 + if isinstance(status, str): + return status.startswith("5") def _set_transaction_name_and_source( diff --git a/tests/integrations/falcon/test_falcon.py b/tests/integrations/falcon/test_falcon.py index 79765bc537..9f5ee5345d 100644 --- a/tests/integrations/falcon/test_falcon.py +++ b/tests/integrations/falcon/test_falcon.py @@ -1,4 +1,5 @@ import logging +from http import HTTPStatus import falcon import falcon.testing @@ -228,6 +229,27 @@ def on_get(self, req, resp): assert " by zero" in event["exception"]["values"][0]["value"] +@pytest.mark.parametrize( + "status", + [ + falcon.HTTP_502, + pytest.param( + 502, + marks=pytest.mark.skipif( + (FALCON_VERSION or ()) < (4,), + reason="Integer HTTPError statuses require Falcon 4+", + ), + ), + pytest.param( + HTTPStatus.BAD_GATEWAY, + id="HTTPStatus.BAD_GATEWAY", + marks=pytest.mark.skipif( + (FALCON_VERSION or ()) < (4,), + reason="Integer HTTPError statuses require Falcon 4+", + ), + ), + ], +) @pytest.mark.parametrize("span_streaming", [True, False]) def test_raised_5xx_errors( sentry_init, @@ -235,7 +257,9 @@ def test_raised_5xx_errors( capture_events, capture_items, span_streaming, + status, ): + """String, integer, and http.HTTPStatus 5xx HTTPError statuses should be captured.""" sentry_init( integrations=[FalconIntegration()], trace_lifecycle="stream" if span_streaming else "static", @@ -243,7 +267,7 @@ def test_raised_5xx_errors( class Resource: def on_get(self, req, resp): - raise falcon.HTTPError(falcon.HTTP_502) + raise falcon.HTTPError(status) app = falcon.API() app.add_route("/", Resource()) @@ -273,6 +297,27 @@ def on_get(self, req, resp): assert event["exception"]["values"][0]["type"] == "HTTPError" +@pytest.mark.parametrize( + "status", + [ + falcon.HTTP_400, + pytest.param( + 400, + marks=pytest.mark.skipif( + (FALCON_VERSION or ()) < (4,), + reason="Integer HTTPError statuses require Falcon 4+", + ), + ), + pytest.param( + HTTPStatus.BAD_REQUEST, + id="HTTPStatus.BAD_REQUEST", + marks=pytest.mark.skipif( + (FALCON_VERSION or ()) < (4,), + reason="Integer HTTPError statuses require Falcon 4+", + ), + ), + ], +) @pytest.mark.parametrize("span_streaming", [True, False]) def test_raised_4xx_errors( sentry_init, @@ -280,7 +325,9 @@ def test_raised_4xx_errors( capture_events, capture_items, span_streaming, + status, ): + """String, integer, and http.HTTPStatus 4xx HTTPError statuses should not be captured.""" sentry_init( integrations=[FalconIntegration()], trace_lifecycle="stream" if span_streaming else "static", @@ -288,7 +335,7 @@ def test_raised_4xx_errors( class Resource: def on_get(self, req, resp): - raise falcon.HTTPError(falcon.HTTP_400) + raise falcon.HTTPError(status) app = falcon.API() app.add_route("/", Resource()) @@ -306,6 +353,27 @@ def on_get(self, req, resp): assert len(events) == 0 +@pytest.mark.parametrize( + "status", + [ + falcon.HTTP_508, + pytest.param( + 508, + marks=pytest.mark.skipif( + (FALCON_VERSION or ()) < (4,), + reason="Integer HTTPStatus statuses require Falcon 4+", + ), + ), + pytest.param( + HTTPStatus.LOOP_DETECTED, + id="HTTPStatus.LOOP_DETECTED", + marks=pytest.mark.skipif( + (FALCON_VERSION or ()) < (4,), + reason="Integer HTTPStatus statuses require Falcon 4+", + ), + ), + ], +) @pytest.mark.parametrize("span_streaming", [True, False]) def test_http_status( sentry_init, @@ -313,11 +381,9 @@ def test_http_status( capture_events, capture_items, span_streaming, + status, ): - """ - This just demonstrates, that if Falcon raises a HTTPStatus with code 500 - (instead of a HTTPError with code 500) Sentry will not capture it. - """ + """String, integer, and http.HTTPStatus 5xx falcon.HTTPStatus exceptions are not captured.""" sentry_init( integrations=[FalconIntegration()], trace_lifecycle="stream" if span_streaming else "static", @@ -325,7 +391,7 @@ def test_http_status( class Resource: def on_get(self, req, resp): - raise falcon.http_status.HTTPStatus(falcon.HTTP_508) + raise falcon.http_status.HTTPStatus(status) app = falcon.API() app.add_route("/", Resource()) @@ -346,6 +412,58 @@ def on_get(self, req, resp): assert len(events) == 0 +@pytest.mark.skipif( + (FALCON_VERSION or ()) < (3,), + reason="Integer Response.status values require Falcon 3+", +) +@pytest.mark.parametrize( + "status", + [ + 400, + pytest.param(HTTPStatus.BAD_REQUEST, id="HTTPStatus.BAD_REQUEST"), + 499, + 500, + pytest.param( + HTTPStatus.INTERNAL_SERVER_ERROR, id="HTTPStatus.INTERNAL_SERVER_ERROR" + ), + 599, + 600, + ], +) +@pytest.mark.parametrize("span_streaming", [True, False]) +def test_integer_response_status(sentry_init, capture_events, status, span_streaming): + """A handler's integer or HTTPStatus response status should capture only 5xx errors.""" + sentry_init( + integrations=[FalconIntegration()], + trace_lifecycle="stream" if span_streaming else "static", + ) + + class Resource: + def on_get(self, req, resp): + raise ValueError("handled error") + + def error_handler(req, resp, ex, params): + resp.status = status + + app = falcon.API() + app.add_route("/", Resource()) + app.add_error_handler(ValueError, error_handler) + events = capture_events() + + response = falcon.testing.TestClient(app).simulate_get("/") + + # this exists in falcon >= 3 + assert response.status_code == status + if 500 <= status < 600: + (event,) = events + exception = event["exception"]["values"][-1] + assert exception["type"] == "ValueError" + assert exception["value"] == "handled error" + assert exception["mechanism"] == {"type": "falcon", "handled": False} + else: + assert not events + + @pytest.mark.parametrize("max_value_length", [1024, None]) @pytest.mark.parametrize("span_streaming", [True, False]) def test_falcon_large_json_request(