Repository navigation
fix(sse): reconnect when a stream drops instead of reporting failure - #291
Merged
Merged
Conversation
A dropped stream ended monitor_operation as FAILED while the operation ran on, and a caller could then retry a write still in progress. A drop, a clean close before the end, and a 429 or 5xx now reconnect. The retry budget resets on real events only, since the server sends connected on every open. OperatorClient keeps no stream retries, because its /status polling already covers a lost stream.
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.
Summary
A dropped operation stream ended
monitor_operationasFAILEDwhile the operation kept running, so a caller could retry a write that was still in progress. The client now reconnects and returns the operation's real result.Changes
SSEClientandAsyncSSEClient:429or5xxnow go through the retry path. A deploy can cause any of these.401, still emiterrorwithout retrying.stream_endstill stops without reconnecting.connectedon every open, so resetting on open or on that event would let a stream that opens and ends retry forever.OperatorClient: it setsmax_retries=0. Its/statuspolling already covers a lost stream, and retrying first would delay that by about 31 seconds.close()during backoff.clients/README.md: describes the new behavior.Worth a close look: a stream that keeps closing with no result used to raise
ended before a terminal eventstraight away. It now retries, and when the retries run out it returnsFAILEDwith the error, the same as other lost connections. Astream_endwith no result, which is what a paused operation sends, still raises as before.Compatibility
Fix only: no export or signature changes. Runtime behavior changes in the case above, and in
max_retries_exceedednow firing for drops that used to emiterror. Both correct false failures, so this fits a patch or minor, not a major.Testing
stream_end, the retry budget and the async client.failedafter 1 connection before,completedafter 2 connections with the fix.🤖 Generated with Claude Code