Skip to content

Commit 16903d3

Browse files
author
Romain Fliedel
committed
fix(django): Await async middleware process_* hooks
When an async Django middleware defines an `async def` hook such as `process_view` or `process_exception`, the SDK wrapped it in a synchronous function. That hid the coroutine from Django's `iscoroutinefunction` check in `BaseHandler.load_middleware`, so Django wrapped the hook in `SyncToAsync` and called it synchronously. The underlying coroutine was then never awaited, and the request failed with: ``` ValueError: The view ... didn't return an HttpResponse object. It returned an unawaited coroutine instead. ``` `_get_wrapped_method` in the Django middleware integration now produces an `async def` wrapper when the wrapped middleware hook is a coroutine function, instead of always producing a synchronous wrapper.
1 parent 7024d19 commit 16903d3

3 files changed

Lines changed: 127 additions & 7 deletions

File tree

sentry_sdk/integrations/django/middleware.py

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@
3232

3333
if not DJANGO_SUPPORTS_ASYNC_MIDDLEWARE:
3434
_asgi_middleware_mixin_factory = lambda _: object
35+
iscoroutinefunction = lambda _: False
3536
else:
36-
from .asgi import _asgi_middleware_mixin_factory
37+
from .asgi import _asgi_middleware_mixin_factory, iscoroutinefunction
3738

3839

3940
def patch_django_middlewares() -> None:
@@ -104,15 +105,39 @@ def _check_middleware_span(
104105

105106
def _get_wrapped_method(old_method: "F") -> "F":
106107
with capture_internal_exceptions():
108+
# Middleware hooks (e.g. `process_view`, `process_exception`) may be
109+
# `async def` when the middleware is async. A synchronous wrapper
110+
# would hide the coroutine from Django's `iscoroutinefunction` check,
111+
# causing Django to call the hook synchronously and never await the
112+
# returned coroutine. Wrap async hooks with an async wrapper so the
113+
# wrapped method continues to report as a coroutine function.
114+
if iscoroutinefunction is not None and iscoroutinefunction(old_method):
107115

108-
def sentry_wrapped_method(*args: "Any", **kwargs: "Any") -> "Any":
109-
middleware_span = _check_middleware_span(old_method)
116+
async def async_sentry_wrapped_method(
117+
*args: "Any", **kwargs: "Any"
118+
) -> "Any":
119+
middleware_span = _check_middleware_span(old_method)
110120

111-
if middleware_span is None:
112-
return old_method(*args, **kwargs)
121+
if middleware_span is None:
122+
return await old_method(*args, **kwargs)
113123

114-
with middleware_span:
115-
return old_method(*args, **kwargs)
124+
with middleware_span:
125+
return await old_method(*args, **kwargs)
126+
127+
sentry_wrapped_method = async_sentry_wrapped_method
128+
129+
else:
130+
131+
def sync_sentry_wrapped_method(*args: "Any", **kwargs: "Any") -> "Any":
132+
middleware_span = _check_middleware_span(old_method)
133+
134+
if middleware_span is None:
135+
return old_method(*args, **kwargs)
136+
137+
with middleware_span:
138+
return old_method(*args, **kwargs)
139+
140+
sentry_wrapped_method = sync_sentry_wrapped_method
116141

117142
try:
118143
# fails for __call__ of function on Python 2 (see py2.7-django-1.11)

tests/integrations/django/asgi/test_asgi.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,3 +1049,50 @@ async def test_transaction_http_method_custom(
10491049
(event1, event2) = events
10501050
assert event1["request"]["method"] == "OPTIONS"
10511051
assert event2["request"]["method"] == "HEAD"
1052+
1053+
1054+
@pytest.mark.asyncio
1055+
@pytest.mark.skipif(
1056+
django.VERSION < (3, 1),
1057+
reason="async views/middleware introduced in Django 3.1",
1058+
)
1059+
async def test_async_middleware_process_view_is_awaited(
1060+
sentry_init, settings, make_asgi_application
1061+
):
1062+
"""Regression test for async ``process_view`` being coerced to sync."""
1063+
sentry_init(integrations=[DjangoIntegration()])
1064+
1065+
settings.MIDDLEWARE = [
1066+
"tests.integrations.django.myapp.middleware.AsyncProcessViewMiddleware"
1067+
]
1068+
application = make_asgi_application()
1069+
1070+
comm = HttpCommunicator(application, "GET", "/simple_async_view")
1071+
response = await comm.get_response()
1072+
await comm.wait()
1073+
1074+
assert response["status"] == 200
1075+
1076+
1077+
@pytest.mark.asyncio
1078+
@pytest.mark.skipif(
1079+
django.VERSION < (3, 1),
1080+
reason="async views/middleware introduced in Django 3.1",
1081+
)
1082+
async def test_async_middleware_process_exception_is_awaited(
1083+
sentry_init, settings, make_asgi_application
1084+
):
1085+
"""Regression test for async ``process_exception`` being coerced to sync."""
1086+
sentry_init(integrations=[DjangoIntegration()])
1087+
1088+
settings.MIDDLEWARE = [
1089+
"tests.integrations.django.myapp.middleware.AsyncProcessExceptionMiddleware"
1090+
]
1091+
application = make_asgi_application()
1092+
1093+
comm = HttpCommunicator(application, "GET", "/view-exc")
1094+
response = await comm.get_response()
1095+
await comm.wait()
1096+
1097+
assert response["status"] == 200
1098+
assert response["body"] == b"handled by async process_exception"

tests/integrations/django/myapp/middleware.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
if django.VERSION >= (3, 1):
44
import asyncio
55

6+
from asgiref.sync import iscoroutinefunction, markcoroutinefunction
67
from django.utils.decorators import sync_and_async_middleware
78

89
@sync_and_async_middleware
@@ -21,6 +22,53 @@ def middleware(request):
2122

2223
return middleware
2324

25+
class AsyncProcessViewMiddleware:
26+
"""A correctly-written async-only middleware exposing an async ``process_view``.
27+
see: https://docs.djangoproject.com/en/5.2/topics/http/middleware/#asynchronous-support
28+
29+
This is the supported way to write async middleware per the Django docs:
30+
declare ``async_capable`` and mark the instance as a coroutine when the
31+
inner ``get_response`` is async.
32+
"""
33+
34+
async_capable = True
35+
sync_capable = False
36+
37+
def __init__(self, get_response):
38+
self.get_response = get_response
39+
if iscoroutinefunction(self.get_response):
40+
markcoroutinefunction(self)
41+
42+
async def __call__(self, request):
43+
return await self.get_response(request)
44+
45+
async def process_view(self, request, view_func, view_args, view_kwargs):
46+
return None
47+
48+
class AsyncProcessExceptionMiddleware:
49+
"""Async-only middleware exposing an async ``process_exception`` hook.
50+
51+
The view raises, so Django invokes ``process_exception``. If the async
52+
``process_exception`` is awaited correctly, it returns an ``HttpResponse``
53+
that short-circuits the error and the request succeeds with status 200.
54+
"""
55+
56+
async_capable = True
57+
sync_capable = False
58+
59+
def __init__(self, get_response):
60+
self.get_response = get_response
61+
if iscoroutinefunction(self.get_response):
62+
markcoroutinefunction(self)
63+
64+
async def __call__(self, request):
65+
return await self.get_response(request)
66+
67+
async def process_exception(self, request, exception):
68+
from django.http import HttpResponse
69+
70+
return HttpResponse("handled by async process_exception", status=200)
71+
2472

2573
def custom_urlconf_middleware(get_response):
2674
def middleware(request):

0 commit comments

Comments
 (0)