Skip to content

Commit ddd2a37

Browse files
committed
fix: change _handle_reconnection to return bool and fix infinite recursion
_handle_reconnection previously returned None, making it impossible for callers to distinguish between a successful response delivery and exhausted retries. This changes the return type to bool (True on success, False when max attempts exceeded) and fixes two issues: - The attempt counter at line 426 was reset to 0 on each reconnection, causing infinite recursion when streams kept ending without delivering a response. Now increments attempt on each recursion. - All recursive calls now use `return await` so the result propagates back to the original caller. MAX_RECONNECTION_ATTEMPTS increased from 2 to 5 to accommodate legitimate multi-reconnection patterns where the server intentionally closes streams between checkpoints. Github-Issue: #1401
1 parent 0fe16dd commit ddd2a37

File tree

2 files changed

+52
-10
lines changed

2 files changed

+52
-10
lines changed

src/mcp/client/streamable_http.py

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747

4848
# Reconnection defaults
4949
DEFAULT_RECONNECTION_DELAY_MS = 1000 # 1 second fallback when server doesn't provide retry
50-
MAX_RECONNECTION_ATTEMPTS = 2 # Max retry attempts before giving up
50+
MAX_RECONNECTION_ATTEMPTS = 5 # Max retry attempts before giving up
5151

5252

5353
class StreamableHTTPError(Exception):
@@ -377,12 +377,17 @@ async def _handle_reconnection(
377377
last_event_id: str,
378378
retry_interval_ms: int | None = None,
379379
attempt: int = 0,
380-
) -> None:
381-
"""Reconnect with Last-Event-ID to resume stream after server disconnect."""
380+
) -> bool:
381+
"""Reconnect with Last-Event-ID to resume stream after server disconnect.
382+
383+
Returns:
384+
True if the response was successfully delivered, False if max
385+
reconnection attempts were exceeded without delivering a response.
386+
"""
382387
# Bail if max retries exceeded
383-
if attempt >= MAX_RECONNECTION_ATTEMPTS: # pragma: no cover
388+
if attempt >= MAX_RECONNECTION_ATTEMPTS:
384389
logger.debug(f"Max reconnection attempts ({MAX_RECONNECTION_ATTEMPTS}) exceeded")
385-
return
390+
return False
386391

387392
# Always wait - use server value or default
388393
delay_ms = retry_interval_ms if retry_interval_ms is not None else DEFAULT_RECONNECTION_DELAY_MS
@@ -419,15 +424,15 @@ async def _handle_reconnection(
419424
)
420425
if is_complete:
421426
await event_source.response.aclose()
422-
return
427+
return True
423428

424-
# Stream ended again without response - reconnect again (reset attempt counter)
429+
# Stream ended again without response - reconnect again
425430
logger.info("SSE stream disconnected, reconnecting...")
426-
await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, 0)
431+
return await self._handle_reconnection(ctx, reconnect_last_event_id, reconnect_retry_ms, attempt + 1)
427432
except Exception as e: # pragma: no cover
428433
logger.debug(f"Reconnection failed: {e}")
429434
# Try to reconnect again if we still have an event ID
430-
await self._handle_reconnection(ctx, last_event_id, retry_interval_ms, attempt + 1)
435+
return await self._handle_reconnection(ctx, last_event_id, retry_interval_ms, attempt + 1)
431436

432437
async def post_writer(
433438
self,

tests/shared/test_streamable_http.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,14 @@
2929

3030
from mcp import MCPError, types
3131
from mcp.client.session import ClientSession
32-
from mcp.client.streamable_http import StreamableHTTPTransport, streamable_http_client
32+
from mcp.client.streamable_http import (
33+
MAX_RECONNECTION_ATTEMPTS,
34+
StreamableHTTPTransport,
35+
streamable_http_client,
36+
)
37+
from mcp.client.streamable_http import (
38+
RequestContext as TransportRequestContext,
39+
)
3340
from mcp.server import Server, ServerRequestContext
3441
from mcp.server.streamable_http import (
3542
MCP_PROTOCOL_VERSION_HEADER,
@@ -2247,3 +2254,33 @@ async def test_streamable_http_client_preserves_custom_with_mcp_headers(
22472254

22482255
assert "content-type" in headers_data
22492256
assert headers_data["content-type"] == "application/json"
2257+
2258+
2259+
@pytest.mark.anyio
2260+
async def test_handle_reconnection_returns_false_on_max_attempts():
2261+
"""_handle_reconnection returns False when max attempts exceeded."""
2262+
transport = StreamableHTTPTransport(url="http://localhost:9999/mcp")
2263+
2264+
read_stream_writer, read_stream = anyio.create_memory_object_stream[SessionMessage | Exception](1)
2265+
2266+
message = JSONRPCRequest(jsonrpc="2.0", id=42, method="tools/call", params={"name": "test"})
2267+
session_message = SessionMessage(message)
2268+
2269+
ctx = TransportRequestContext(
2270+
client=httpx.AsyncClient(),
2271+
session_id="test-session",
2272+
session_message=session_message,
2273+
metadata=None,
2274+
read_stream_writer=read_stream_writer,
2275+
)
2276+
2277+
try:
2278+
with anyio.fail_after(5):
2279+
result = await transport._handle_reconnection(
2280+
ctx, last_event_id="evt-1", retry_interval_ms=None, attempt=MAX_RECONNECTION_ATTEMPTS
2281+
)
2282+
assert result is False
2283+
finally:
2284+
await read_stream_writer.aclose()
2285+
await read_stream.aclose()
2286+
await ctx.client.aclose()

0 commit comments

Comments
 (0)