Skip to content

Commit 171aa16

Browse files
r0roRomain Fliedel
andauthored
fix(django): Avoid ValueError in async middleware process_* hooks (#6698)
Stop wrapping async middleware hooks in sync functions to prevent misclassifications due to `iscoroutinefunction()` checks within Django. Co-authored-by: Romain Fliedel <romain@oqee.tv>
1 parent c3746b2 commit 171aa16

3 files changed

Lines changed: 132 additions & 10 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: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import django
22

33
if django.VERSION >= (3, 1):
4-
import asyncio
5-
64
from django.utils.decorators import sync_and_async_middleware
75

6+
from sentry_sdk.integrations.django.asgi import (
7+
iscoroutinefunction,
8+
markcoroutinefunction,
9+
)
10+
811
@sync_and_async_middleware
912
def simple_middleware(get_response):
10-
if asyncio.iscoroutinefunction(get_response):
13+
if iscoroutinefunction(get_response):
1114

1215
async def middleware(request):
1316
response = await get_response(request)
@@ -21,6 +24,53 @@ def middleware(request):
2124

2225
return middleware
2326

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

2575
def custom_urlconf_middleware(get_response):
2676
def middleware(request):

0 commit comments

Comments
 (0)