Skip to content

Commit

Permalink
Make reading the request body work in Django ASGI apps. (#2495)
Browse files Browse the repository at this point in the history
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 <daniel.szoke@sentry.io>
Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
  • Loading branch information
antonpirker committed Nov 10, 2023
1 parent 522abef commit 35d86b6
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 7 deletions.
20 changes: 16 additions & 4 deletions sentry_sdk/integrations/django/__init__.py
Expand Up @@ -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")

Expand Down Expand Up @@ -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)
)


Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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):
Expand Down
72 changes: 71 additions & 1 deletion sentry_sdk/integrations/django/asgi.py
Expand Up @@ -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
Expand All @@ -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
Expand Down
49 changes: 49 additions & 0 deletions tests/integrations/django/asgi/test_asgi.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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"]
5 changes: 5 additions & 0 deletions tests/integrations/django/myapp/urls.py
Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions tests/integrations/django/myapp/views.py
Expand Up @@ -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
4 changes: 2 additions & 2 deletions tox.ini
Expand Up @@ -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
Expand Down

0 comments on commit 35d86b6

Please sign in to comment.