fix: propagate pre-endpoint errors in sse_client instead of deadlocking#2340
Draft
fix: propagate pre-endpoint errors in sse_client instead of deadlocking#2340
Conversation
9d53554 to
d0c7a71
Compare
When sse_reader encounters an error before receiving the endpoint event, the except handler tried to send the exception to read_stream_writer. With a zero-buffer stream and no reader (the caller is still blocked in tg.start() waiting for task_status.started()), send() blocks forever. Track whether started() has fired. Before it, re-raise so the exception propagates through tg.start(). After it, send to the stream as before. This also adds a guard for the case where a server sends a message event before the endpoint event, which would deadlock on the same send() call. The dedicated SSEError handler from #975 is removed since the started flag now handles all pre-endpoint exceptions uniformly. Github-Issue: #447
d0c7a71 to
54f02ed
Compare
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.
Fixes #447
Problem
sse_clientlaunchessse_readerviatg.start(), which blocks untiltask_status.started()is called. When an error occurs before the endpoint event is received, theexcept Exceptionhandler tries to send the exception toread_stream_writer. The stream has buffer size 0 and nobody is reading (the caller is still blocked intg.start()), sosend()blocks forever.This deadlocks in three scenarios:
endpointevent with a mismatched origin (the original report)messageevent before theendpointevent (protocol violation)Fix
Track whether
task_status.started()has fired with astartedflag. Before it fires, re-raise exceptions so they propagate throughtg.start()to the caller. After it fires, send them to the stream as before — the caller is reading by then.This generalizes the approach from #975, which fixed the same deadlock but only for
SSEError. The dedicatedSSEErrorhandler is removed since the flag now covers all pre-endpoint exceptions uniformly.Behavior change
Previously,
SSEErrorraised mid-stream (after the endpoint was received) would crash the task group. Now it is delivered on the read stream like other post-endpoint errors, letting the session layer handle it. This aligns with how stdio and websocket transports surface mid-stream errors.Testing
Four new regression tests in
tests/shared/test_sse.pycover:ValueErrorpromptlyRuntimeErrorpromptlyAll tests use
anyio.fail_after(5)guards — if the deadlock regresses, tests fail with timeout rather than hanging CI.AI Disclaimer