From 7e4992ab28e9d596730db289bc97fa7195ca57e4 Mon Sep 17 00:00:00 2001 From: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com> Date: Mon, 23 Sep 2024 17:04:49 +0200 Subject: [PATCH] feat(aiohttp): Add `failed_request_status_codes` (#3551) `failed_request_status_codes` allows users to specify the status codes, whose corresponding `HTTPException` types, should be reported to Sentry. By default, these include 5xx statuses, which is a change from the previous default behavior, where no `HTTPException`s would be reported to Sentry. Closes #3535 --- sentry_sdk/integrations/aiohttp.py | 23 +++- tests/integrations/aiohttp/test_aiohttp.py | 122 +++++++++++++++++++++ 2 files changed, 142 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index b9840fcfa8..2c3779c828 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -48,6 +48,8 @@ from aiohttp.web_request import Request from aiohttp.web_urldispatcher import UrlMappingMatchInfo from aiohttp import TraceRequestStartParams, TraceRequestEndParams + + from collections.abc import Set from types import SimpleNamespace from typing import Any from typing import Optional @@ -59,20 +61,27 @@ TRANSACTION_STYLE_VALUES = ("handler_name", "method_and_path_pattern") +DEFAULT_FAILED_REQUEST_STATUS_CODES = frozenset(range(500, 600)) class AioHttpIntegration(Integration): identifier = "aiohttp" origin = f"auto.http.{identifier}" - def __init__(self, transaction_style="handler_name"): - # type: (str) -> None + def __init__( + self, + transaction_style="handler_name", # type: str + *, + failed_request_status_codes=DEFAULT_FAILED_REQUEST_STATUS_CODES, # type: Set[int] + ): + # type: (...) -> None if transaction_style not in TRANSACTION_STYLE_VALUES: raise ValueError( "Invalid value for transaction_style: %s (must be in %s)" % (transaction_style, TRANSACTION_STYLE_VALUES) ) self.transaction_style = transaction_style + self._failed_request_status_codes = failed_request_status_codes @staticmethod def setup_once(): @@ -100,7 +109,8 @@ def setup_once(): async def sentry_app_handle(self, request, *args, **kwargs): # type: (Any, Request, *Any, **Any) -> Any - if sentry_sdk.get_client().get_integration(AioHttpIntegration) is None: + integration = sentry_sdk.get_client().get_integration(AioHttpIntegration) + if integration is None: return await old_handle(self, request, *args, **kwargs) weak_request = weakref.ref(request) @@ -131,6 +141,13 @@ async def sentry_app_handle(self, request, *args, **kwargs): response = await old_handle(self, request) except HTTPException as e: transaction.set_http_status(e.status_code) + + if ( + e.status_code + in integration._failed_request_status_codes + ): + _capture_exception() + raise except (asyncio.CancelledError, ConnectionResetError): transaction.set_status(SPANSTATUS.CANCELLED) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index be372b6643..f952b82c35 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -7,6 +7,13 @@ from aiohttp import web, ClientSession from aiohttp.client import ServerDisconnectedError from aiohttp.web_request import Request +from aiohttp.web_exceptions import ( + HTTPInternalServerError, + HTTPNetworkAuthenticationRequired, + HTTPBadRequest, + HTTPNotFound, + HTTPUnavailableForLegalReasons, +) from sentry_sdk import capture_message, start_transaction from sentry_sdk.integrations.aiohttp import AioHttpIntegration @@ -617,3 +624,118 @@ async def handler(_): # Important to note that the ServerDisconnectedError indicates we have no error server-side. with pytest.raises(ServerDisconnectedError): await client.get("/") + + +@pytest.mark.parametrize( + ("integration_kwargs", "exception_to_raise", "should_capture"), + ( + ({}, None, False), + ({}, HTTPBadRequest, False), + ( + {}, + HTTPUnavailableForLegalReasons(None), + False, + ), # Highest 4xx status code (451) + ({}, HTTPInternalServerError, True), + ({}, HTTPNetworkAuthenticationRequired, True), # Highest 5xx status code (511) + ({"failed_request_status_codes": set()}, HTTPInternalServerError, False), + ( + {"failed_request_status_codes": set()}, + HTTPNetworkAuthenticationRequired, + False, + ), + ({"failed_request_status_codes": {404, *range(500, 600)}}, HTTPNotFound, True), + ( + {"failed_request_status_codes": {404, *range(500, 600)}}, + HTTPInternalServerError, + True, + ), + ( + {"failed_request_status_codes": {404, *range(500, 600)}}, + HTTPBadRequest, + False, + ), + ), +) +@pytest.mark.asyncio +async def test_failed_request_status_codes( + sentry_init, + aiohttp_client, + capture_events, + integration_kwargs, + exception_to_raise, + should_capture, +): + sentry_init(integrations=[AioHttpIntegration(**integration_kwargs)]) + events = capture_events() + + async def handle(_): + if exception_to_raise is not None: + raise exception_to_raise + else: + return web.Response(status=200) + + app = web.Application() + app.router.add_get("/", handle) + + client = await aiohttp_client(app) + resp = await client.get("/") + + expected_status = ( + 200 if exception_to_raise is None else exception_to_raise.status_code + ) + assert resp.status == expected_status + + if should_capture: + (event,) = events + assert event["exception"]["values"][0]["type"] == exception_to_raise.__name__ + else: + assert not events + + +@pytest.mark.asyncio +async def test_failed_request_status_codes_with_returned_status( + sentry_init, aiohttp_client, capture_events +): + """ + Returning a web.Response with a failed_request_status_code should not be reported to Sentry. + """ + sentry_init(integrations=[AioHttpIntegration(failed_request_status_codes={500})]) + events = capture_events() + + async def handle(_): + return web.Response(status=500) + + app = web.Application() + app.router.add_get("/", handle) + + client = await aiohttp_client(app) + resp = await client.get("/") + + assert resp.status == 500 + assert not events + + +@pytest.mark.asyncio +async def test_failed_request_status_codes_non_http_exception( + sentry_init, aiohttp_client, capture_events +): + """ + If an exception, which is not an instance of HTTPException, is raised, it should be captured, even if + failed_request_status_codes is empty. + """ + sentry_init(integrations=[AioHttpIntegration(failed_request_status_codes=set())]) + events = capture_events() + + async def handle(_): + 1 / 0 + + app = web.Application() + app.router.add_get("/", handle) + + client = await aiohttp_client(app) + resp = await client.get("/") + assert resp.status == 500 + + (event,) = events + assert event["exception"]["values"][0]["type"] == "ZeroDivisionError"