diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 6068df2587..b8b1bc8f57 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -1,3 +1,4 @@ +import functools import inspect import sys import threading @@ -6,7 +7,12 @@ import sentry_sdk from sentry_sdk.consts import OP, SPANDATA, SPANNAME -from sentry_sdk.integrations import DidNotEnable, Integration, _check_minimum_version +from sentry_sdk.integrations import ( + _DEFAULT_FAILED_REQUEST_STATUS_CODES, + DidNotEnable, + Integration, + _check_minimum_version, +) from sentry_sdk.integrations._wsgi_common import ( DEFAULT_HTTP_METHODS_TO_CAPTURE, RequestExtractor, @@ -80,6 +86,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: + from collections.abc import Set from typing import Any, Callable, Dict, List, Optional, Union from django.core.handlers.wsgi import WSGIRequest @@ -87,7 +94,13 @@ from django.http.response import HttpResponse from django.utils.datastructures import MultiValueDict - from sentry_sdk._types import Event, EventProcessor, Hint, NotImplementedType + from sentry_sdk._types import ( + Event, + EventProcessor, + ExcInfo, + Hint, + NotImplementedType, + ) from sentry_sdk.integrations.wsgi import _ScopedResponse from sentry_sdk.traces import StreamedSpan from sentry_sdk.tracing import Span @@ -116,6 +129,11 @@ class DjangoIntegration(Integration): :param signals_spans: Whether to create spans for signals. Defaults to `True`. :param signals_denylist: A list of signals to ignore when creating spans. :param cache_spans: Whether to create spans for cache operations. Defaults to `False`. + :param failed_request_status_codes: Which HTTP error responses to report to Sentry. + Django answers some exceptions itself instead of failing: `raise Http404` gets + the user a 404 page, `PermissionDenied` a 403. Those are reported only if their + status code is in this set, which defaults to the 5xx range. Exceptions Django + gives up on end in a 500 and are always reported. """ identifier = "django" @@ -137,6 +155,8 @@ def __init__( db_transaction_spans: bool = False, signals_denylist: "Optional[list[signals.Signal]]" = None, http_methods_to_capture: "tuple[str, ...]" = DEFAULT_HTTP_METHODS_TO_CAPTURE, + *, + failed_request_status_codes: "Set[int]" = _DEFAULT_FAILED_REQUEST_STATUS_CODES, ) -> None: if transaction_style not in TRANSACTION_STYLE_VALUES: raise ValueError( @@ -154,6 +174,8 @@ def __init__( self.http_methods_to_capture = tuple(map(str.upper, http_methods_to_capture)) + self.failed_request_status_codes = failed_request_status_codes + @staticmethod def setup_once() -> None: _check_minimum_version(DjangoIntegration, DJANGO_VERSION) @@ -199,6 +221,8 @@ def sentry_patched_wsgi_handler( _patch_django_asgi_handler() + _patch_response_for_exception() + signals.got_request_exception.connect(_got_request_exception) @add_global_event_processor @@ -614,18 +638,89 @@ def _got_request_exception(request: "WSGIRequest" = None, **kwargs: "Any") -> No if integration is None: return + # Record that this exception is reported, so `_patch_response_for_exception` + # doesn't report it a second time. + with capture_internal_exceptions(): + request._sentry_exception_reported = True + + _capture_exception(sys.exc_info(), request, integration, handled=False) + + +def _capture_exception( + exc_info: "Union[BaseException, ExcInfo]", + request: "Optional[WSGIRequest]", + integration: "DjangoIntegration", + handled: bool, +) -> None: if request is not None and integration.transaction_style == "url": scope = sentry_sdk.get_current_scope() _attempt_resolve_again(request, scope, integration.transaction_style) event, hint = event_from_exception( - sys.exc_info(), - client_options=client.options, - mechanism={"type": "django", "handled": False}, + exc_info, + client_options=sentry_sdk.get_client().options, + mechanism={"type": "django", "handled": handled}, ) sentry_sdk.capture_event(event, hint=hint) +def _patch_response_for_exception() -> None: + """ + Report the errors Django answers itself. + + Django deals with every exception in one function, which boils down to: + + if isinstance(exc, Http404): return <404 page> + if isinstance(exc, PermissionDenied): return <403 page> + if isinstance(exc, SuspiciousOperation): return <400 page> + got_request_exception.send(...) # Django gives up + return <500 page> + + We only ever listened to that signal, so we heard about the exceptions Django + gives up on and about nothing else. Wrapping the function lets us see the rest + too, along with the status code Django picked for them. + """ + try: + from django.core.handlers import exception as exception_handler + except ImportError: + # Django < 1.10 does this in `BaseHandler`, nothing to patch here + return + + old_response_for_exception = getattr( + exception_handler, "response_for_exception", None + ) + if old_response_for_exception is None: + return + + @functools.wraps(old_response_for_exception) + def sentry_patched_response_for_exception( + request: "WSGIRequest", exc: Exception + ) -> "HttpResponse": + integration = sentry_sdk.get_client().get_integration(DjangoIntegration) + if integration is None: + return old_response_for_exception(request, exc) + + # Clear the flag before delegating. The same request can reach this + # function twice: first when the view raises, then again if a middleware + # raises while handing the response back out. Without the reset, the + # first exception would keep the second one from being reported. + with capture_internal_exceptions(): + request._sentry_exception_reported = False + + response = old_response_for_exception(request, exc) + + # The flag is set when Django gives up on the exception and fires + # `got_request_exception`, which means we reported it already. + if not getattr(request, "_sentry_exception_reported", False): + status_code = getattr(response, "status_code", None) + if status_code in integration.failed_request_status_codes: + _capture_exception(exc, request, integration, handled=True) + + return response + + exception_handler.response_for_exception = sentry_patched_response_for_exception + + class DjangoRequestExtractor(RequestExtractor): def __init__(self, request: "Union[WSGIRequest, ASGIRequest]") -> None: try: diff --git a/tests/integrations/django/asgi/test_asgi.py b/tests/integrations/django/asgi/test_asgi.py index 9abf52b120..74b50d782c 100644 --- a/tests/integrations/django/asgi/test_asgi.py +++ b/tests/integrations/django/asgi/test_asgi.py @@ -1126,3 +1126,37 @@ async def test_user_identity_error_event_data_collection( assert "id" not in event.get("user", {}) assert "email" not in event.get("user", {}) assert "username" not in event.get("user", {}) + + +@pytest.mark.parametrize("application", APPS) +@pytest.mark.asyncio +@pytest.mark.skipif( + django.VERSION < (3, 0), reason="Django ASGI support shipped in 3.0" +) +@pytest.mark.parametrize( + ("integration_kwargs", "expected_type"), + ( + ({}, None), + ({"failed_request_status_codes": {403, *range(500, 600)}}, "PermissionDenied"), + ), +) +async def test_failed_request_status_codes( + sentry_init, capture_events, application, integration_kwargs, expected_type +): + sentry_init(integrations=[DjangoIntegration(**integration_kwargs)]) + events = capture_events() + + comm = HttpCommunicator(application, "GET", "/permission-denied-exc") + response = await comm.get_response() + await comm.wait() + + assert response["status"] == 403 + + if expected_type is None: + assert not events + else: + (event,) = events + (exception,) = event["exception"]["values"] + assert exception["type"] == expected_type + assert exception["mechanism"]["handled"] is True + assert event["transaction"] == "/permission-denied-exc" diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 87d11b791b..743207161d 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -102,6 +102,11 @@ def path(path, *args, **kwargs): views.permission_denied_exc, name="permission_denied_exc", ), + path( + "http404-exc", + views.http404_exc, + name="http404_exc", + ), path( "csrf-hello-not-exempt", views.csrf_hello_not_exempt, diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py index 21f27455a5..4fab3c7969 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -7,7 +7,12 @@ from django.core.exceptions import PermissionDenied from django.db import transaction from django.dispatch import Signal -from django.http import HttpResponse, HttpResponseNotFound, HttpResponseServerError +from django.http import ( + Http404, + HttpResponse, + HttpResponseNotFound, + HttpResponseServerError, +) from django.shortcuts import render from django.template import Context, Template from django.template.response import TemplateResponse @@ -343,6 +348,11 @@ def permission_denied_exc(*args, **kwargs): raise PermissionDenied("bye") +@csrf_exempt +def http404_exc(*args, **kwargs): + raise Http404("bye") + + def csrf_hello_not_exempt(*args, **kwargs): return HttpResponse("ok") diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 93b5477010..6cd9b856e7 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -1761,6 +1761,96 @@ def test_does_not_capture_403( assert not events +@pytest.mark.parametrize( + ("integration_kwargs", "endpoint", "status", "expected_type"), + ( + # Django only turns exceptions into 4xx responses, so with the default + # (the 5xx range) none of them are reported + ({}, "permission_denied_exc", "403 forbidden", None), + ({}, "http404_exc", "404 not found", None), + ( + {"failed_request_status_codes": set()}, + "permission_denied_exc", + "403 forbidden", + None, + ), + ( + {"failed_request_status_codes": {403, *range(500, 600)}}, + "permission_denied_exc", + "403 forbidden", + "PermissionDenied", + ), + ( + {"failed_request_status_codes": {404, *range(500, 600)}}, + "http404_exc", + "404 not found", + "Http404", + ), + # Only the status codes that were opted into are reported + ( + {"failed_request_status_codes": {403}}, + "http404_exc", + "404 not found", + None, + ), + ), +) +def test_failed_request_status_codes( + sentry_init, + client, + capture_events, + integration_kwargs, + endpoint, + status, + expected_type, +): + sentry_init(integrations=[DjangoIntegration(**integration_kwargs)]) + events = capture_events() + + _, response_status, _ = unpack_werkzeug_response(client.get(reverse(endpoint))) + assert response_status.lower() == status + + # The test app's handler404 captures a message, ignore it here + error_events = [event for event in events if "exception" in event] + + if expected_type is None: + assert not error_events + else: + (event,) = error_events + (exception,) = event["exception"]["values"] + assert exception["type"] == expected_type + assert exception["mechanism"]["type"] == "django" + assert exception["mechanism"]["handled"] is True + + +@pytest.mark.parametrize( + "integration_kwargs", + ( + {}, + {"failed_request_status_codes": set()}, + {"failed_request_status_codes": {404}}, + ), +) +def test_failed_request_status_codes_unhandled_exception( + sentry_init, client, capture_events, integration_kwargs +): + """ + Exceptions Django gives up on are always reported, exactly once, no matter how + failed_request_status_codes is set. + """ + sentry_init(integrations=[DjangoIntegration(**integration_kwargs)]) + events = capture_events() + + _, status, _ = unpack_werkzeug_response(client.get(reverse("view_exc"))) + assert status.lower() == "500 internal server error" + + (event,) = events + (exception,) = event["exception"]["values"] + assert exception["type"] == "ZeroDivisionError" + assert exception["mechanism"]["type"] == "django" + assert exception["mechanism"]["handled"] is False + + @pytest.mark.parametrize("span_streaming", [True, False]) def test_render_spans( sentry_init,