Skip to content

Commit 8761f60

Browse files
chernistryclaude
andcommitted
fix: reset retry counter only on forward progress, not unconditionally
The previous fix (attempt + 1 unconditionally) broke legitimate multi-reconnection scenarios where the server checkpoints progress between disconnections. Now we check whether new SSE events were delivered (last_event_id changed) — if so, reset the counter; if not, increment it toward MAX_RECONNECTION_ATTEMPTS. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9630947 commit 8761f60

File tree

2 files changed

+16
-7
lines changed

2 files changed

+16
-7
lines changed

src/mcp/client/streamable_http.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,13 @@ async def _handle_reconnection(
423423

424424
# Stream ended again without response - reconnect again
425425
logger.info("SSE stream disconnected, reconnecting...")
426-
await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, attempt + 1)
426+
# Reset attempt counter only if the stream delivered new events
427+
# (i.e. made forward progress). If no new events arrived, the
428+
# server is connecting then dropping immediately — count that
429+
# towards the retry budget to avoid infinite loops (#2393).
430+
made_progress = reconnect_last_event_id != last_event_id
431+
next_attempt = 0 if made_progress else attempt + 1
432+
await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, next_attempt)
427433
except Exception as e: # pragma: no cover
428434
logger.debug(f"Reconnection failed: {e}")
429435
# Try to reconnect again if we still have an event ID

tests/shared/test_streamable_http.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2329,15 +2329,16 @@ async def test_streamable_http_client_preserves_custom_with_mcp_headers(
23292329

23302330
@pytest.mark.anyio
23312331
async def test_handle_reconnection_does_not_retry_infinitely():
2332-
"""Reconnection must count TOTAL attempts, not reset on each successful connect.
2332+
"""Reconnection must give up when no forward progress is made.
23332333
23342334
Regression test for #2393: when a stream connects successfully but drops
23352335
before delivering a response, the attempt counter was reset to 0 on the
23362336
recursive call, allowing an infinite retry loop.
23372337
23382338
This test simulates a stream that connects, yields one non-completing SSE
2339-
event, then ends — repeatedly. With MAX_RECONNECTION_ATTEMPTS the loop
2340-
must terminate.
2339+
event with the SAME event ID each time (no new data), then ends —
2340+
repeatedly. Without forward progress the loop must terminate within
2341+
MAX_RECONNECTION_ATTEMPTS.
23412342
"""
23422343
transport = StreamableHTTPTransport(url="http://localhost:8000/mcp")
23432344
transport.session_id = "test-session"
@@ -2354,13 +2355,15 @@ async def fake_aconnect_sse(*args: Any, **kwargs: Any) -> AsyncIterator[Any]:
23542355
mock_response = MagicMock()
23552356
mock_response.raise_for_status = MagicMock()
23562357

2357-
# Yield a single non-completing notification SSE event, then end the stream
2358-
# (simulating a server that drops the connection after partial delivery)
2358+
# Yield a single non-completing notification SSE event with the SAME
2359+
# event ID every time, then end the stream. Because the ID never
2360+
# changes, the transport sees no forward progress and should count
2361+
# each reconnection towards the retry budget.
23592362
async def aiter_sse() -> AsyncIterator[ServerSentEvent]:
23602363
yield ServerSentEvent(
23612364
event="message",
23622365
data='{"jsonrpc":"2.0","method":"notifications/progress","params":{"progressToken":"tok","progress":1,"total":10}}',
2363-
id=f"evt-{connect_count}",
2366+
id="evt-static",
23642367
retry=None,
23652368
)
23662369

0 commit comments

Comments
 (0)