Re: [Django] #33716: async get_response can be a regular function too (was: Async-capable middleware is not async-capable)

[Django]

#33716: async get_response can be a regular function too
-------------------------------+------------------------------------
Reporter: abetkin | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by abetkin):

* status: closed => new
* resolution: invalid =>

Old description:

> All the standard django middleware is marked as async-capable. As I
> understand that means that sync_to_async adapters will not be used if no
> other middleware is present.
>
> However, if you call an endpoint that is handled by an async view,
> somehow the adapters are still used, and middleware is executed in a
> separate thread.
>
> Moreover, if you add your custom middleware that is async_capable than it
> tries not to use adapters, and quickly fails:
>
> [[https://code.djangoproject.com/attachment/ticket/33716/bug.png]]
>
> {{{
> AttributeError: 'coroutine' object has no attribute 'get'
> }}}
>
> In this example this was XFrameOptionsMiddleware that failed
>
> Github project illustrating this: https://github.com/pwtail/django_bug
>
> Is reproduced in thae main branch and in 4.0

New description:

Here is what we have in MiddlewareMixin:

{{{
def _async_check(self):
"""
If get_response is a coroutine function, turns us into async mode
so
a thread is not consumed during a whole request.
"""
if asyncio.iscoroutinefunction(self.get_response):
# Mark the class as async-capable, but do the actual switch
# inside __call__ to avoid swapping out dunder methods
self._is_coroutine = asyncio.coroutines._is_coroutine
else:
self._is_coroutine = None
}}}

This checks if the next middleware is a coroutine, and if not fallbacks to
sync mode. However, we already mark all async-capable middleware as such,
why the redundancy?

A common usecase that is currently not supported:

{{{
def MyMiddleware(get_response):

def middleware(request):
# Do some stuff with request that does not involve I/O
request.vip_user = True
return get_response(request)

return middleware

MyMiddleware.async_capable=True
}}}

middleware(request) will return the response in sync case and a coroutine
in the async case, despite being a regular function (because get_response
is a coroutine function in the latter case).

So I propose to remove the redundant _async_check

Github project to see the error: https://github.com/pwtail/django_bug

--

--
Ticket URL: <https://code.djangoproject.com/ticket/33716#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.