Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aiohttp): Add failed_request_status_codes #3551

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
Loading