#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 , 4 years ago
| Cc: | added |
|---|---|
| Keywords: | ASGI async added |
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 4 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 , 4 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 forinstead 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 , 4 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 , 4 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 4 years ago
| Has patch: | set |
|---|
Created a pull request: ​https://github.com/django/django/pull/14526
comment:7 by , 4 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 , 4 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 , 4 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 , 3 years ago
| Cc: | added |
|---|
comment:11 by , 3 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 , 3 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.