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

Author: szokeasaurusrex

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)

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"