django / daphne

Django Channels HTTP/WebSocket server
BSD 3-Clause "New" or "Revised" License
2.32k stars 256 forks source link

Daphne does not stream #413

Closed florianvazelle closed 1 year ago

florianvazelle commented 2 years ago

Using Daphne as a server for a Django application seems to cause a malfunction in streaming responses.

When you pass an iterator/file into a response, the contents of the response are not retrieved chunk by chunk, but from a single block when the iterator has finished iterating.

I have a minimal project that can reproduce the behavior, here.

This seems to happen only for daphne (tested with uvicorn, gunicorn and uwsgi) and when running the development server with the channels application (running runserver with the --noasgi option does not produce the issue).

This ticket echoes the channels ticket

https://github.com/django/channels/issues/1761.

Details
$ python --version
Python 3.10.4
$ pip freeze
asgiref==3.5.0
attrs==21.4.0
autobahn==22.3.2
Automat==20.2.0
cffi==1.15.0
constantly==15.1.0
cryptography==36.0.2
daphne==3.0.2
Django==3.2.12
hyperlink==21.0.0
idna==3.3
incremental==21.3.0
pyasn1==0.4.8
pyasn1-modules==0.2.8
pycparser==2.21
pyOpenSSL==22.0.0
pytz==2022.1
service-identity==21.1.0
six==1.16.0
sqlparse==0.4.2
Twisted==22.2.0
txaio==22.2.1
typing_extensions==4.1.1
zope.interface==5.4.0
carltongibson commented 2 years ago

#32798 (StreamingHttpResponse Raises SynchronousOnlyOperation in ASGI Server) – Django is likely also related.

florianvazelle commented 2 years ago

Ok @carltongibson, I tried with this fix (seen in your related issue), it seems to solve my problem too.

The Django modification allows to access the database in a stream response, with an ASGI server. It also allows to have a correct stream with daphne.

But would you know why the response is correctly stream, without this fix, with uvicorn which is also an ASGI server ?

carltongibson commented 2 years ago

I was thinking about uvicorn too. πŸ€”

I'd need to have a rummage to see what it's doing in this case. Fancy digging around?

florianvazelle commented 2 years ago

Yes, I'm going to investigate. :slightly_smiling_face:

florianvazelle commented 2 years ago

I think I found what I didn't understand in my issue.

Reason

Daphne and Uvicorn don't use the same libraries and therefore handle the streaming differently.

In Daphne, we use Twisted with an asyncioreactor. I managed to reproduce the bug in a minimal way: Minimal example of an "ASGI-like" streaming server with Twisted.

My streaming issue occurs when I call a blocking method in my iterator (io, sleep, request ...), which blocks the reactor. That's why the result is not done progressively.

The reactor does not get control of execution back until the end of the iteration.

Solution

To correct this behavior I need to add an asynchronous layer and use async / non-blocking alternatives.

In my sample application, my view would become:

import asyncio

from django.http.response import StreamingHttpResponse

async def iterable_content():
    for _ in range(5):
        await asyncio.sleep(1)
        print('Returning chunk')
        yield b'a' * 10000

def test_stream_view(request):
    return StreamingHttpResponse(iterable_content())

Real Issue

But django does not handle asynchronous generators. This is what is said, and that I had not seen, in the ticket that you had related.

At some point in the 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.

The MR that I had tested and that corrected my issue, is explained because the iteration was done in another context. But considering the performance, we could manage it differently here.

I tried to test my hypothesis by patching django, in a very basic way, like this: florianvazelle/django.

Is it possible to include in Django some asynchronous responses, when we are in ASGI context ?

What do you think ? Would it be possible to open a MR for this?

carltongibson commented 2 years ago

Hi @florianvazelle β€” interesting.

I'd say it's definitely worth opening a ticket on Django, and a PR to go with it (marked "Initial PoC" if you want to underplay it πŸ™‚) I can then take a proper look there.

Good stuff. Thanks for the continued effort! πŸ…

florianvazelle commented 1 year ago

Hi @carltongibson

I am trying to reproduce my initial issue with your fix in django (https://github.com/django/django/pull/16384), it's works fine now.

Thank you for your work. :slightly_smiling_face:

carltongibson commented 1 year ago

Thanks for confirming @florianvazelle! And thanks for your initial work here, it exposed the issue very clearly, and was essential in coming to the final design. Bring on Django 4.2 😜