Stream response body in ASGITransport#1032
Open
Kludex wants to merge 2 commits into
Open
Conversation
|
Docs preview: |
Merging this PR will not alter performance
Comparing Footnotes
|
Member
Author
|
I'm not sure about which one is the best: this or the new ASGI stream transport. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports encode/httpx#3059 (by @jhominal), but integrated into
ASGITransportitself rather than as a separateASGIStreamingTransportclass.The app now runs in a separate task, and response events are streamed as they arrive: a response is returned as soon as the app sends
http.response.start, and body chunks can be iterated before the app has fully run. This makes it possible to test streaming endpoints (e.g. SSE) through the ASGI transport, including apps that callsendfrom a sub-task such as Starlette'sStreamingResponse.Differences from the upstream PR:
anyiodirectly (hard dependency here), dropping the trio/asyncio dual-path helpers.GeneratorExitis caught at the yield and the app task is cancelled, instead of letting it propagate through the task group where anyio/trio would wrap it in aBaseExceptionGroup(found by the new early-close test; the upstream PR never exercised this path).ASGIResponseStream.__aiter__has nofinallycleanup - closing goes through theaclose()chain likeBoundAsyncStream, since awaiting cleanup in generator finalization is rejected by trio.Behavioural notes: the app runs in a separate task, so context variables set within the app are no longer visible to the caller, and exceptions raised after
http.response.startnow surface while reading the body rather than fromhandle_async_request(still raised fromclient.get()for non-streamed requests).AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.