fix(client): bound empty GET stream retries - #3258
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Thanks for the contribution. This repository only keeps pull requests open when they're linked to an issue that a maintainer has assigned to the author — CONTRIBUTING.md explains why and how we work. This PR has been closed for now because you aren't currently assigned to #3257. If a maintainer would like this change as a PR from you, they'll assign you to #3257 and this PR will reopen automatically — there's nothing more you need to do. (If you opened the issue, this PR already shows up on its timeline.) There's no need to open a new PR — this one will be reopened. While it's closed, please push any updates as new commits rather than force-pushing, since GitHub can't reopen a PR whose branch has been rewritten. Maintainers: reopening this PR, removing the |
Problem
StreamableHTTPTransport.handle_get_stream()can reconnect forever when a server returns a valid empty SSE stream and closes it cleanly. The loop'sMAX_RECONNECTION_ATTEMPTSbound is never reached because every empty close resets the attempt counter.Root Cause
The transport treated every normal SSE iterator completion as a productive stream, even when no SSE event had been received.
Solution
Track whether the stream delivered an event. Reset the retry counter only after a productive stream; count an empty clean close against the existing retry budget.
Changes
MAX_RECONNECTION_ATTEMPTS.200 text/event-streamresponse.Testing
PYTHONPATH=src;. py -3 -m pytest --import-mode=importlib tests/client/test_streamable_http.py -q? 29 passed.py -3 -m ruff format --check src/mcp/client/streamable_http.py tests/client/test_streamable_http.py? passed.py -3 -m ruff check src/mcp/client/streamable_http.py tests/client/test_streamable_http.py? passed.py -3 -m pyright --pythonpath D:\Python310\python.exe src/mcp/client/streamable_http.py tests/client/test_streamable_http.py? passed.git diff --check? passed.Compatibility / Risk
This only changes behavior for a cleanly closed stream that delivered zero events. Productive SSE streams continue to reset the retry budget and reconnect as before. The change prevents unbounded background polling against endpoints that terminate without sending data.
Notes for Reviewer
The regression uses the real transport with
httpx2.MockTransport, a valid empty SSE response, and a patched zero-delay sleep; it asserts exactly the configured retry budget is consumed.Linked Issue
Fixes #3257