Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASGI: Add affordances and recipe for long-polling #1808

Open
kgriffs opened this issue Dec 12, 2020 · 6 comments
Open

ASGI: Add affordances and recipe for long-polling #1808

kgriffs opened this issue Dec 12, 2020 · 6 comments
Labels
documentation enhancement good first issue Comment on this issue if you'd like to volunteer to work on this. Thanks! needs contributor Comment on this issue if you'd like to volunteer to work on this. Thanks!
Milestone

Comments

@kgriffs
Copy link
Member

kgriffs commented Dec 12, 2020

The 3.0 ASGI implementation already affords long-polling, but it would be useful to provide a recipe to demonstrate the pattern in the context of a Falcon app.

Also, for the sake of efficiency, we may want to provide a req.wait_disconnect() or similar method that can be executed in a background task within a responder. The responder could then use this in order to detect if it should abandon a long-poll early in the case that the client bails out. This is similar to what asgi.App does when streaming SSE's (using watch_disconnect().

@kgriffs kgriffs added documentation good first issue Comment on this issue if you'd like to volunteer to work on this. Thanks! enhancement needs contributor Comment on this issue if you'd like to volunteer to work on this. Thanks! labels Dec 12, 2020
@kgriffs kgriffs added this to the Version 3.1 milestone Dec 12, 2020
@vytas7
Copy link
Member

vytas7 commented Dec 15, 2020

Having some way to signal http.disconnect to the responder might also be useful for handling HTTP streaming requests (where the incoming Content-Length is unknown in advance). And more generally, it might potentially make interaction with the request stream more elegant.

In addition, should we add some request status attributes, as we have for WebSocket (such as asgi.WebSocket.closed etc)?

@kgriffs
Copy link
Member Author

kgriffs commented Dec 15, 2020

Use cases we will need to address and document:

  • Reading from a request body when content-length header is present but the client disconnects before all of the expected data is read
  • Reading from a request body when content-length header is not present and the client disconnects in a way that results in malformed data or a bad state
  • After reading from a request body, the client disconnects before a response can be sent
  • Holding a client request while waiting for an event (long-polling) and the client disconnects before said event occurs

Potential affordances:

  • Request status attributes (e.g., closed, open) - but is this already covered by req.stream?
  • req.wait_disconnect() to be executed in a background task within the responder.
  • Raising a "client disconnected" error while reading from a stream - presumably the client should stay on the line until it gets a response back from the server, rather than just disconnecting immediately at the end of the stream

@kgriffs
Copy link
Member Author

kgriffs commented Dec 15, 2020

Note: 3.0 behavior is to truncate the request body in the case of a premature disconnect. Mitigation is for apps to always validate their inputs!

@vytas7
Copy link
Member

vytas7 commented Feb 17, 2021

The current behaviour is somewhat awkward in the SSE scenario from the client perspective.
Even though asgi.App itself perfectly detects a disconnect event (via the mentioned watch_disconnect()), the event is not conveyed to the user's implementation.

Right now, it's possible to wrap the emitter's loop in a try.. block hoping to catch GeneratorExit when the async generator is torn down, and execute the cleanup code in the finally clause.

But not sure if it is a particularly elegant/recommended way to do it, and if it is, we should document recipes for that.

@kgriffs
Copy link
Member Author

kgriffs commented Feb 23, 2021

OK, so looks like we need to improve the SSE disconnect story, and overall we will need some recipes for SSE, WebSocket, and good-old-fashioned long polling.

@vytas7
Copy link
Member

vytas7 commented Feb 27, 2021

When using as async generator in conjunction with resp.stream, we happen to check

            if hasattr(stream, 'close'):
                await stream.close()

Maybe the something similar could be supported for the SSE generator too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement good first issue Comment on this issue if you'd like to volunteer to work on this. Thanks! needs contributor Comment on this issue if you'd like to volunteer to work on this. Thanks!
Projects
None yet
Development

No branches or pull requests

2 participants