#31928 closed Bug (fixed)
Coroutine passed to the first middleware's process_response() instead of HttpResponse.
Reported by: | Kordian Kowalski | Owned by: | Kevin Michel |
---|---|---|---|
Component: | HTTP handling | Version: | 3.1 |
Severity: | Release blocker | Keywords: | async asgi |
Cc: | Andrew Godwin, Carlton Gibson | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Like the title says, using ASGI (+ uvicorn in my case), the first middleware (according to the list in settings.py) receives a coroutine as its response parameter, while all other middlewares down the line receive a django.http.response.HttpResponse object.
This seems to have caused an issue in the django-cors-headers package which is often placed first in order:
https://github.com/adamchainz/django-cors-headers/issues/558
How to reproduce:
- Set up a django 3.1 project with an async server (uvicorn in my case)
- Create a dummy class-based middleware that prints the types of arguments it receives in its process_response method:
class DummyMiddleware(MiddlewareMixin): def process_response(self, request, response): print(request.__class__, response.__class__)
- Set up the middleware as the first one in settings.py:
MIDDLEWARE = [ 'django_uvicorn_test.middleware.DummyMiddleware', 'django.middleware.security.SecurityMiddleware', ...
- Launch the server and perform any request, observe console output:
<class 'django.core.handlers.asgi.ASGIRequest'> <class 'coroutine'>
- Move the middleware down on the list, restart the server and perform a request again:
<class 'django.core.handlers.asgi.ASGIRequest'> <class 'django.http.response.HttpResponse'>
Change History (13)
comment:1 by , 4 years ago
Type: | Uncategorized → Bug |
---|
comment:2 by , 4 years ago
Cc: | added |
---|---|
Keywords: | async asgi added |
Severity: | Normal → Release blocker |
Summary: | ASGI, coroutine passed to the first middleware's process_response instead of response object → Coroutine passed to the first middleware's process_response() instead of HttpResponse. |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 4 years ago
I think it's a bug in SecurityMiddleware : its __init__
does not call super().__init__()
.
This causes MiddlewareMixin._async_check()
to not be called during init and the
_is_coroutine
magic attribute to be missing despite the async_capable=True
declaration.
comment:4 by , 4 years ago
Hi Kevin. That's a good spot! :)
Would you fancy taking on a quick PR for this? (It's on my list if not but...)
Thanks
comment:5 by , 4 years ago
Hi !
Yeah I can do a PR for that :)
I see that the cache-related middlewares have the same issue,
would you prefer a common PR with all middlewares fixed or should I make a separate PR for the cache ?
The 3 cache middlewares (UpdateCacheMiddleware, FetchFromCacheMiddleware, CacheMiddleware)
will probably need to be changed together since they are related by inheritance.
comment:6 by , 4 years ago
Hi Kevin.
would you prefer a common PR with all middlewares fixed or should I make a separate PR for the cache ?
It’s a single issue no, a single PR, should be fine. (We can maybe do three commits if that seems better, but it’s a single issue no? :)
I didn’t dig in yet, so you’re ahead of me but, if you can narrow down the issue in a test case (or three...) then that’s the main thing I’d think.
Thanks for the input! Super. 👌
comment:8 by , 4 years ago
Nice catch - can't believe I didn't see this during testing. I agree with the diagnosis of the problem, too, I'll go take a look at the PR.
comment:9 by , 4 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Tentatively accepted for investigation. It's not about the first middleware because if you have only one it gets
HttpResponse
, but if you have at least two then then the first in a chain getscoroutine
.Andrew, Can you take a look?