From 35d86b69980632816b5e055a2d697cdecef14a36 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 Nov 2023 15:32:42 +0100 Subject: [PATCH] Make reading the request body work in Django ASGI apps. (#2495) Handle request body in ASGI based Django apps. Starting with Django 4.1 the stream representing the request body is closed immediately preventing us from reading it. This fix reads the request body early on, so it is cached by Django and can be then read by our integration to add to the events sent to Sentry. --------- Co-authored by Daniel Szoke Co-authored-by: Ivana Kellyerova --- sentry_sdk/integrations/django/__init__.py | 20 ++++-- sentry_sdk/integrations/django/asgi.py | 72 ++++++++++++++++++++- tests/integrations/django/asgi/test_asgi.py | 49 ++++++++++++++ tests/integrations/django/myapp/urls.py | 5 ++ tests/integrations/django/myapp/views.py | 8 +++ tox.ini | 4 +- 6 files changed, 151 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 73908bc333..95f18d00ab 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -47,6 +47,13 @@ from django.urls import Resolver404 except ImportError: from django.core.urlresolvers import Resolver404 + + # Only available in Django 3.0+ + try: + from django.core.handlers.asgi import ASGIRequest + except Exception: + ASGIRequest = None + except ImportError: raise DidNotEnable("Django not installed") @@ -410,7 +417,7 @@ def _before_get_response(request): _set_transaction_name_and_source(scope, integration.transaction_style, request) scope.add_event_processor( - _make_event_processor(weakref.ref(request), integration) + _make_wsgi_request_event_processor(weakref.ref(request), integration) ) @@ -462,9 +469,9 @@ def sentry_patched_get_response(self, request): patch_get_response_async(BaseHandler, _before_get_response) -def _make_event_processor(weak_request, integration): +def _make_wsgi_request_event_processor(weak_request, integration): # type: (Callable[[], WSGIRequest], DjangoIntegration) -> EventProcessor - def event_processor(event, hint): + def wsgi_request_event_processor(event, hint): # type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any] # if the request is gone we are fine not logging the data from # it. This might happen if the processor is pushed away to @@ -473,6 +480,11 @@ def event_processor(event, hint): if request is None: return event + django_3 = ASGIRequest is not None + if django_3 and type(request) == ASGIRequest: + # We have a `asgi_request_event_processor` for this. + return event + try: drf_request = request._sentry_drf_request_backref() if drf_request is not None: @@ -489,7 +501,7 @@ def event_processor(event, hint): return event - return event_processor + return wsgi_request_event_processor def _got_request_exception(request=None, **kwargs): diff --git a/sentry_sdk/integrations/django/asgi.py b/sentry_sdk/integrations/django/asgi.py index 41ebe18e62..48b27c50c8 100644 --- a/sentry_sdk/integrations/django/asgi.py +++ b/sentry_sdk/integrations/django/asgi.py @@ -11,16 +11,56 @@ from sentry_sdk import Hub, _functools from sentry_sdk._types import TYPE_CHECKING from sentry_sdk.consts import OP +from sentry_sdk.hub import _should_send_default_pii from sentry_sdk.integrations.asgi import SentryAsgiMiddleware +from sentry_sdk.utils import capture_internal_exceptions + +from django.core.handlers.wsgi import WSGIRequest + if TYPE_CHECKING: from typing import Any + from typing import Dict from typing import Union from typing import Callable + from django.core.handlers.asgi import ASGIRequest from django.http.response import HttpResponse + from sentry_sdk.integrations.django import DjangoIntegration + from sentry_sdk._types import EventProcessor + + +def _make_asgi_request_event_processor(request, integration): + # type: (ASGIRequest, DjangoIntegration) -> EventProcessor + def asgi_request_event_processor(event, hint): + # type: (Dict[str, Any], Dict[str, Any]) -> Dict[str, Any] + # if the request is gone we are fine not logging the data from + # it. This might happen if the processor is pushed away to + # another thread. + from sentry_sdk.integrations.django import ( + DjangoRequestExtractor, + _set_user_info, + ) + + if request is None: + return event + + if type(request) == WSGIRequest: + return event + + with capture_internal_exceptions(): + DjangoRequestExtractor(request).extract_into_event(event) + + if _should_send_default_pii(): + with capture_internal_exceptions(): + _set_user_info(request, event) + + return event + + return asgi_request_event_processor + def patch_django_asgi_handler_impl(cls): # type: (Any) -> None @@ -31,16 +71,46 @@ def patch_django_asgi_handler_impl(cls): async def sentry_patched_asgi_handler(self, scope, receive, send): # type: (Any, Any, Any, Any) -> Any - if Hub.current.get_integration(DjangoIntegration) is None: + hub = Hub.current + integration = hub.get_integration(DjangoIntegration) + if integration is None: return await old_app(self, scope, receive, send) middleware = SentryAsgiMiddleware( old_app.__get__(self, cls), unsafe_context_data=True )._run_asgi3 + return await middleware(scope, receive, send) cls.__call__ = sentry_patched_asgi_handler + modern_django_asgi_support = hasattr(cls, "create_request") + if modern_django_asgi_support: + old_create_request = cls.create_request + + def sentry_patched_create_request(self, *args, **kwargs): + # type: (Any, *Any, **Any) -> Any + hub = Hub.current + integration = hub.get_integration(DjangoIntegration) + if integration is None: + return old_create_request(self, *args, **kwargs) + + with hub.configure_scope() as scope: + request, error_response = old_create_request(self, *args, **kwargs) + + # read the body once, to signal Django to cache the body stream + # so we can read the body in our event processor + # (otherwise Django closes the body stream and makes it impossible to read it again) + _ = request.body + + scope.add_event_processor( + _make_asgi_request_event_processor(request, integration) + ) + + return request, error_response + + cls.create_request = sentry_patched_create_request + def patch_get_response_async(cls, _before_get_response): # type: (Any, Any) -> None diff --git a/tests/integrations/django/asgi/test_asgi.py b/tests/integrations/django/asgi/test_asgi.py index 85921cf364..57145b698d 100644 --- a/tests/integrations/django/asgi/test_asgi.py +++ b/tests/integrations/django/asgi/test_asgi.py @@ -7,6 +7,11 @@ from sentry_sdk.integrations.django import DjangoIntegration from tests.integrations.django.myapp.asgi import channels_application +try: + from django.urls import reverse +except ImportError: + from django.core.urlresolvers import reverse + try: from unittest import mock # python 3.3 and above except ImportError: @@ -353,3 +358,47 @@ async def test_trace_from_headers_if_performance_disabled(sentry_init, capture_e assert msg_event["contexts"]["trace"]["trace_id"] == trace_id assert error_event["contexts"]["trace"]["trace_id"] == trace_id + + +@pytest.mark.parametrize("application", APPS) +@pytest.mark.parametrize( + "body,expected_return_data", + [ + ( + b'{"username":"xyz","password":"xyz"}', + {"username": "xyz", "password": "xyz"}, + ), + (b"hello", ""), + (b"", None), + ], +) +@pytest.mark.asyncio +@pytest.mark.skipif( + django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1" +) +async def test_asgi_request_body( + sentry_init, capture_envelopes, application, body, expected_return_data +): + sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) + + envelopes = capture_envelopes() + + comm = HttpCommunicator( + application, + method="POST", + path=reverse("post_echo_async"), + body=body, + headers=[(b"content-type", b"application/json")], + ) + response = await comm.get_response() + + assert response["status"] == 200 + assert response["body"] == body + + (envelope,) = envelopes + event = envelope.get_event() + + if expected_return_data is not None: + assert event["request"]["data"] == expected_return_data + else: + assert "data" not in event["request"] diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 2a4535e588..be5a40239e 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -82,6 +82,11 @@ def path(path, *args, **kwargs): path("async/thread_ids", views.thread_ids_async, name="thread_ids_async") ) +if views.post_echo_async is not None: + urlpatterns.append( + path("post_echo_async", views.post_echo_async, name="post_echo_async") + ) + # rest framework try: urlpatterns.append( diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py index 1e909f2b38..6362adc121 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -235,7 +235,15 @@ def thread_ids_sync(*args, **kwargs): }) return HttpResponse(response)""" ) + + exec( + """@csrf_exempt +def post_echo_async(request): + sentry_sdk.capture_message("hi") + return HttpResponse(request.body)""" + ) else: async_message = None my_async_view = None thread_ids_async = None + post_echo_async = None diff --git a/tox.ini b/tox.ini index b99e08eb26..d5e0d753a9 100644 --- a/tox.ini +++ b/tox.ini @@ -288,8 +288,8 @@ deps = django: Werkzeug<2.1.0 django-v{1.11,2.0,2.1,2.2,3.0,3.1,3.2}: djangorestframework>=3.0.0,<4.0.0 - {py3.7,py3.8,py3.9,py3.10,py3.11}-django-v{1.11,2.0,2.1,2.2,3.0,3.1,3.2}: pytest-asyncio - {py3.7,py3.8,py3.9,py3.10,py3.11}-django-v{1.11,2.0,2.1,2.2,3.0,3.1,3.2}: channels[daphne]>2 + {py3.7,py3.8,py3.9,py3.10,py3.11}-django-v{1.11,2.0,2.1,2.2,3.0,3.1,3.2,4.0,4.1,4.2}: pytest-asyncio + {py3.7,py3.8,py3.9,py3.10,py3.11}-django-v{1.11,2.0,2.1,2.2,3.0,3.1,3.2,4.0,4.1,4.2}: channels[daphne]>2 django-v{1.8,1.9,1.10,1.11,2.0,2.1}: pytest-django<4.0 django-v{2.2,3.0,3.1,3.2}: pytest-django>=4.0