#32798 closed Bug (duplicate)
StreamingHttpResponse raises SynchronousOnlyOperation in ASGI server
Reported by: | Ralph Broenink | Owned by: | Michael Brown |
---|---|---|---|
Component: | HTTP handling | Version: | 3.2 |
Severity: | Normal | Keywords: | ASGI, async |
Cc: | Andrew Godwin, Jon Janzen, Carles Pina Estany | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
When using a ASGI-compliant server, such as Daphne, StreamingHttpResponse's iterator will be executed in an asynchronous context, preventing database queries from being performed (raising SynchronousOnlyOperation). This is not expected behaviour, and appears not to be documented.
*Steps to reproduce*
- Create a simple project and app. Set-up for use with Channels. We are not going to use this component and the bug does seem to originate from Django itself, but Channels ensures that runserver is ASGI-compliant.
- Create the following view:
from django.contrib.contenttypes.models import ContentType from django.http import StreamingHttpResponse def test_view(request): def generate(): yield "hello\n" list(ContentType.objects.all()) yield "bye\n" return StreamingHttpResponse(generate(), content_type="text/plain")
- Open the page served by test_view
- Observe the following trace:
Exception inside application: You cannot call this from an async context - use a thread or sync_to_async. Traceback (most recent call last): File "venv/lib/python3.8/site-packages/channels/staticfiles.py", line 44, in __call__ return await self.application(scope, receive, send) File "venv/lib/python3.8/site-packages/channels/routing.py", line 71, in __call__ return await application(scope, receive, send) File "venv/lib/python3.8/site-packages/django/core/handlers/asgi.py", line 168, in __call__ await self.send_response(response, send) File "venv/lib/python3.8/site-packages/django/core/handlers/asgi.py", line 242, in send_response for part in response: File "channelstest/testapp/views.py", line 9, in generate list(ContentType.objects.all()) File "venv/lib/python3.8/site-packages/django/db/models/query.py", line 287, in __iter__ self._fetch_all() File "venv/lib/python3.8/site-packages/django/db/models/query.py", line 1303, in _fetch_all self._result_cache = list(self._iterable_class(self)) File "venv/lib/python3.8/site-packages/django/db/models/query.py", line 53, in __iter__ results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size) File "venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1152, in execute_sql cursor = self.connection.cursor() File "venv/lib/python3.8/site-packages/django/utils/asyncio.py", line 24, in inner raise SynchronousOnlyOperation(message) django.core.exceptions.SynchronousOnlyOperation: You cannot call this from an async context - use a thread or sync_to_async.
This error is probably caused by the fact that Django 3 now actively prevents this kind of error (it would have gone undetected in Django 2) and the fact that the iterator is called in an asynchronous context in handlers/asgi.py.
As mentioned above, this issue should at the very least be documented in the documentation, but preferably should be resolved altogether.
Change History (12)
comment:1 by , 3 years ago
Cc: | added |
---|---|
Keywords: | ASGI async added |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 3 years ago
Yeah, this is an unfortunate confluence of the way that generators get called (and where they get called) in the response stack - they're called as sync generators, not async, but also are run in an async thread.
I suspect we should probably look at running any response generators inside sync_to_async, unless we can detect they're an asynchronous generator somehow and then use async for
instead of for
?
comment:3 by , 3 years ago
I suspect we should probably look at running any response generators inside sync_to_async, unless we can detect they're an asynchronous generator somehow and then use
async for
instead offor
?
Even if we detect async vs sync generators at some point, I think response generators should be called inside sync_to_async
. Database calls in sync generators work in WSGIHandler, so it should also work in ASGIHandler. I can see a usecase for async response generators, but I think that's a different issue.
As for how to change for
to async for
, it would be nice if sync_to_async
handled async generators, but according to ​asgiref issue #142 this isn't the case. The pull request for that issue does something like call next
in sync_to_async
, with a workaround to convert StopIteration
to StopAsyncIteration
:
async def _sync_to_async_iterator(iterator): iterator = iter(iterator) def _next(): try: return next(iterator) except StopIteration: raise StopAsyncIteration while True: try: yield await sync_to_async(_next)() except StopAsyncIteration: break
Would it be more appropriate to use a queue here? Here is an implementation using a queue, but it's more complicated so I'm less sure it's correct (although it does work):
async def _sync_to_async_iterator(iterator): q = asyncio.Queue(maxsize=1) def sync_iterator(): for item in iterator: async_to_sync(q.put)(item) task = asyncio.create_task(sync_to_async(sync_iterator)()) try: while not task.done(): next_item = asyncio.create_task(q.get()) done, pending = await asyncio.wait([next_item, task], return_when=asyncio.FIRST_COMPLETED) if next_item in done: yield next_item.result() elif task in done: next_item.cancel() break finally: task.cancel() task.result()
If one of those options seems good then I can work on a patch with tests.
comment:4 by , 3 years ago
Separating these out as two issues:
- Generators should indeed be called within sync_to_async for now, as a safety measure. This is a patch I'd like to see in Django.
- At some point in future, maybe we can detect async generators and correctly handle them in both async and sync modes, but that can be an optional extra for some future time.
comment:5 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 3 years ago
Has patch: | set |
---|
Created a pull request: ​https://github.com/django/django/pull/14526
comment:7 by , 3 years ago
Fyi: I stumbled over this error when running wagtail with asgi, as it is using StreamingHttpResponse
for its db-backed CSV-export (see ​https://github.com/wagtail/wagtail/blob/c6017abca042031fb2f7931b406a4c12e7a9e8a4/wagtail/admin/views/mixins.py#L201).
comment:8 by , 3 years ago
New pull request for the simple fix of wrapping the streaming response code in sync_to_async
and using sync_to_async(send)
.
comment:9 by , 3 years ago
Needs tests: | set |
---|
The proposed fix is slow — but it's not clear exactly how slow. I'm going to put together a benchmark with locust so we can decide properly it's fast-enough, or if we'd be better erroring in this case, with a helpful error messages saying ≈"Use WSGI here". ​Comment on PR to that effect.
I'll mark as Needs tests for the moment.
comment:10 by , 2 years ago
Cc: | added |
---|
comment:11 by , 2 years ago
Resolution: | → duplicate |
---|---|
Status: | assigned → closed |
To simplify things I'm going to mark this as a duplicate of #33735 (which should be resolved by adding __aiter__
to StreamingHTTPResponse
)
comment:12 by , 2 years ago
Cc: | added |
---|
Yes, this won't work. It's not the streaming response (the block this is raised from explicitly handles that) but rather the ORM call.
I think (currently) it is the expected behaviour. It's covered in the ​Async safety section of the Asynchronous support docs.
Hmmm 🤔 but yes, it's a limitation on the async wrapping of synchronous views… I'll accept for now because I can't quite see a better place to pin it. It's likely this will not be resolved until we have a better async story for the ORM, but that's not yet at the ticketed stage. (If we had such a ticket open, I'd say it's a duplicate of that.)
I'll cc Andrew so it crosses his radar as a target use-case.