Skip to content

Commit 0dbac72

Browse files
committed
fix(client): reinitialize expired streamable HTTP sessions
1 parent 9436ace commit 0dbac72

1 file changed

Lines changed: 175 additions & 5 deletions

File tree

tests/client/test_notification_response.py

Lines changed: 175 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import json
88

9+
import anyio
910
import httpx2
1011
import mcp_types as types
1112
import pytest
@@ -17,6 +18,7 @@
1718

1819
from mcp import ClientSession, MCPError
1920
from mcp.client import IncomingMessage
21+
from mcp.client._transport import SESSION_EXPIRED, SESSION_EXPIRED_MARKER
2022
from mcp.client.streamable_http import streamable_http_client
2123

2224
pytestmark = pytest.mark.anyio
@@ -254,15 +256,183 @@ async def test_client_falls_back_to_generic_error_when_non_2xx_body_is_a_jsonrpc
254256
assert exc.value.error.code == types.INTERNAL_ERROR
255257

256258

257-
async def test_client_falls_back_to_session_terminated_when_404_body_is_malformed_json() -> None:
258-
"""SDK-defined: an unparseable ``application/json`` body on a 404 response is swallowed
259-
and the status-derived ``INVALID_REQUEST`` (session-terminated) fallback resolves the
260-
pending request — the parse failure never propagates."""
259+
async def test_client_reports_session_expiry_after_a_404_recovery_retry_has_malformed_json() -> None:
260+
"""SDK-defined: a malformed 404 body still triggers one session recovery attempt before failing.
261+
262+
The parse failure is not surfaced because HTTP 404 is the transport's session-expiry signal. A second
263+
404 after recovery is bounded and returns the private session-expired error rather than looping.
264+
"""
261265
app = _create_non_2xx_json_body_app(404, b"not valid json{{{")
262266
async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client:
263267
async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream):
264268
async with ClientSession(read_stream, write_stream) as session: # pragma: no branch
265269
await session.initialize()
266270
with pytest.raises(MCPError) as exc:
267271
await session.list_tools()
268-
assert exc.value.error.code == types.INVALID_REQUEST
272+
assert exc.value.error.code == SESSION_EXPIRED
273+
274+
275+
def _create_expired_session_recovery_app(requests: list[tuple[str, str | None]]) -> Starlette:
276+
"""Return a fresh session after rejecting one established-session request."""
277+
initialize_count = 0
278+
expire_once = True
279+
280+
async def handle_mcp_request(request: Request) -> Response:
281+
nonlocal expire_once, initialize_count
282+
data = json.loads(await request.body())
283+
method = data.get("method")
284+
session_id = request.headers.get("mcp-session-id")
285+
requests.append((method, session_id))
286+
287+
if method == "initialize":
288+
initialize_count += 1
289+
return JSONResponse(
290+
{"jsonrpc": "2.0", "id": data["id"], "result": INIT_RESPONSE},
291+
headers={"mcp-session-id": f"session-{initialize_count}"},
292+
)
293+
if method == "notifications/initialized":
294+
return Response(status_code=202)
295+
if method == "tools/list" and expire_once:
296+
expire_once = False
297+
assert session_id == "session-1"
298+
return Response(status_code=404)
299+
if method == "tools/list":
300+
assert session_id == "session-2"
301+
return JSONResponse({"jsonrpc": "2.0", "id": data["id"], "result": {"tools": []}})
302+
return Response(status_code=500)
303+
304+
return Starlette(debug=True, routes=[Route("/mcp", handle_mcp_request, methods=["POST"])])
305+
306+
307+
async def test_client_reinitializes_once_after_an_established_session_returns_404() -> None:
308+
"""Spec-mandated: a 404 for an established legacy session creates a fresh session and retries once.
309+
310+
The recovery initialize must omit the expired session id; its initialized notification and the retried
311+
request must carry the new id. This drives the public ``ClientSession`` API through an in-process ASGI app.
312+
"""
313+
requests: list[tuple[str, str | None]] = []
314+
app = _create_expired_session_recovery_app(requests)
315+
316+
async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client:
317+
async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream):
318+
async with ClientSession(read_stream, write_stream) as session:
319+
result = await session.initialize()
320+
assert result.server_info.name == "test-non-sdk-server"
321+
322+
tools = await session.list_tools()
323+
324+
assert tools.tools == []
325+
assert requests == [
326+
("initialize", None),
327+
("notifications/initialized", "session-1"),
328+
("tools/list", "session-1"),
329+
("initialize", None),
330+
("notifications/initialized", "session-2"),
331+
("tools/list", "session-2"),
332+
]
333+
334+
335+
def _create_repeated_expired_session_app(requests: list[tuple[str, str | None]]) -> Starlette:
336+
"""Always expire requests from an established session."""
337+
initialize_count = 0
338+
339+
async def handle_mcp_request(request: Request) -> Response:
340+
nonlocal initialize_count
341+
data = json.loads(await request.body())
342+
method = data.get("method")
343+
session_id = request.headers.get("mcp-session-id")
344+
requests.append((method, session_id))
345+
346+
if method == "initialize":
347+
initialize_count += 1
348+
return JSONResponse(
349+
{"jsonrpc": "2.0", "id": data["id"], "result": INIT_RESPONSE},
350+
headers={"mcp-session-id": f"session-{initialize_count}"},
351+
)
352+
if method == "notifications/initialized":
353+
return Response(status_code=202)
354+
if method == "tools/list":
355+
return Response(status_code=404)
356+
return Response(status_code=500)
357+
358+
return Starlette(debug=True, routes=[Route("/mcp", handle_mcp_request, methods=["POST"])])
359+
360+
361+
async def test_client_retries_an_expired_session_request_only_once() -> None:
362+
"""SDK-defined: a retry that also receives 404 surfaces an error instead of opening a recovery loop."""
363+
requests: list[tuple[str, str | None]] = []
364+
app = _create_repeated_expired_session_app(requests)
365+
366+
async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client:
367+
async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream):
368+
async with ClientSession(read_stream, write_stream) as session:
369+
await session.initialize()
370+
with pytest.raises(MCPError) as exc_info:
371+
await session.list_tools()
372+
373+
assert exc_info.value.code == SESSION_EXPIRED
374+
assert exc_info.value.data == {SESSION_EXPIRED_MARKER: True}
375+
assert requests == [
376+
("initialize", None),
377+
("notifications/initialized", "session-1"),
378+
("tools/list", "session-1"),
379+
("initialize", None),
380+
("notifications/initialized", "session-2"),
381+
("tools/list", "session-2"),
382+
]
383+
384+
385+
async def test_concurrent_expired_session_requests_share_one_reinitialization() -> None:
386+
"""SDK-defined: concurrent 404 responses recover one session generation, not one per caller."""
387+
requests: list[tuple[str, str | None]] = []
388+
old_requests_started = 0
389+
old_requests_ready = anyio.Event()
390+
release_old_requests = anyio.Event()
391+
initialize_count = 0
392+
393+
async def handle_mcp_request(request: Request) -> Response:
394+
nonlocal initialize_count, old_requests_started
395+
data = json.loads(await request.body())
396+
method = data.get("method")
397+
session_id = request.headers.get("mcp-session-id")
398+
requests.append((method, session_id))
399+
400+
if method == "initialize":
401+
initialize_count += 1
402+
return JSONResponse(
403+
{"jsonrpc": "2.0", "id": data["id"], "result": INIT_RESPONSE},
404+
headers={"mcp-session-id": f"session-{initialize_count}"},
405+
)
406+
if method == "notifications/initialized":
407+
return Response(status_code=202)
408+
if method == "tools/list" and session_id == "session-1":
409+
old_requests_started += 1
410+
if old_requests_started == 2:
411+
old_requests_ready.set()
412+
await release_old_requests.wait()
413+
return Response(status_code=404)
414+
if method == "tools/list" and session_id == "session-2":
415+
return JSONResponse({"jsonrpc": "2.0", "id": data["id"], "result": {"tools": []}})
416+
return Response(status_code=500)
417+
418+
app = Starlette(debug=True, routes=[Route("/mcp", handle_mcp_request, methods=["POST"])])
419+
results: list[types.ListToolsResult] = []
420+
421+
async def append_list_tools_result(session: ClientSession) -> None:
422+
results.append(await session.list_tools())
423+
424+
async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client:
425+
async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream):
426+
async with ClientSession(read_stream, write_stream) as session:
427+
await session.initialize()
428+
async with anyio.create_task_group() as task_group:
429+
task_group.start_soon(append_list_tools_result, session)
430+
task_group.start_soon(append_list_tools_result, session)
431+
with anyio.fail_after(5):
432+
await old_requests_ready.wait()
433+
release_old_requests.set()
434+
435+
assert [result.tools for result in results] == [[], []]
436+
assert requests.count(("initialize", None)) == 2
437+
assert requests.count(("notifications/initialized", "session-2")) == 1
438+
assert requests.count(("tools/list", "session-2")) == 2

0 commit comments

Comments
 (0)