Skip to content

Commit

Permalink
feat(aiohttp): Add failed_request_status_codes (#3551)
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
szokeasaurusrex committed Sep 23, 2024
1 parent 8060a64 commit 7e4992a
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 3 deletions.
23 changes: 20 additions & 3 deletions sentry_sdk/integrations/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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():
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
122 changes: 122 additions & 0 deletions tests/integrations/aiohttp/test_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"

0 comments on commit 7e4992a

Please sign in to comment.