Skip to content

Commit 92c078a

Browse files
committed
Harden the experimental modern HTTP entry's error and cleanup paths
- Parse-error response keeps the required "id": null member - 405 carries the Allow: POST header - exit_stack.aclose() is shielded, bounded, and exception-suppressed, matching ServerRunner.run()'s contract - The success/error response is sent inside the per-request lifespan scope so a teardown error cannot drop an already-computed result - Coverage tests for the cleanup-raises and cleanup-hangs arms - Two no-branch pragmas for the 3.14 nested-async-with coverage quirk Claude-Session: https://claude.ai/code/session_017S3aJaxEHeMvftp6whnHWK
1 parent 26ff922 commit 92c078a

3 files changed

Lines changed: 100 additions & 16 deletions

File tree

src/mcp/server/_experimental/streamable_http_modern.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@
2424
from starlette.responses import Response
2525
from starlette.types import Receive, Scope, Send
2626

27-
from mcp.server.runner import ServerRunner, otel_middleware
27+
from mcp.server.runner import (
28+
_EXIT_STACK_CLOSE_TIMEOUT, # type: ignore[reportPrivateUsage]
29+
ServerRunner,
30+
otel_middleware,
31+
)
2832
from mcp.server.transport_security import TransportSecurityMiddleware, TransportSecuritySettings
2933
from mcp.shared.dispatcher import CallOptions, OnNotify, OnRequest
3034
from mcp.shared.exceptions import MCPError, NoBackChannelError
@@ -180,7 +184,7 @@ async def handle_modern_request(
180184

181185
if request.method != "POST":
182186
# TODO: GET/DELETE rejection (405 + -32601) lands with the validation ladder.
183-
await Response(status_code=405)(scope, receive, send)
187+
await Response(status_code=405, headers={"Allow": "POST"})(scope, receive, send)
184188
return
185189

186190
body = await request.body()
@@ -189,7 +193,7 @@ async def handle_modern_request(
189193
except ValidationError:
190194
msg = JSONRPCError(jsonrpc="2.0", id=None, error=ErrorData(code=PARSE_ERROR, message="Parse error"))
191195
await Response(
192-
msg.model_dump_json(by_alias=True, exclude_none=True),
196+
msg.model_dump_json(by_alias=True),
193197
status_code=400,
194198
media_type="application/json",
195199
)(scope, receive, send)
@@ -210,11 +214,20 @@ async def handle_modern_request(
210214
try:
211215
msg = await dispatcher.handle(req, runner._compose_on_request()) # type: ignore[reportPrivateUsage]
212216
finally:
213-
await runner.connection.exit_stack.aclose()
214-
215-
# TODO: error.code -> HTTP status mapping is a follow-up; 200 for all JSONRPCError bodies for now.
216-
await Response(
217-
msg.model_dump_json(by_alias=True, exclude_none=True),
218-
status_code=200,
219-
media_type="application/json",
220-
)(scope, receive, send)
217+
with anyio.move_on_after(_EXIT_STACK_CLOSE_TIMEOUT, shield=True) as cancel_scope:
218+
try:
219+
await runner.connection.exit_stack.aclose()
220+
except Exception:
221+
logger.exception("connection exit_stack cleanup raised")
222+
if cancel_scope.cancelled_caught:
223+
logger.warning(
224+
"connection exit_stack cleanup exceeded %s seconds; abandoning remaining callbacks",
225+
_EXIT_STACK_CLOSE_TIMEOUT,
226+
)
227+
228+
# TODO: error.code -> HTTP status mapping is a follow-up; 200 for all JSONRPCError bodies for now.
229+
await Response(
230+
msg.model_dump_json(by_alias=True, exclude_none=True),
231+
status_code=200,
232+
media_type="application/json",
233+
)(scope, receive, send)

tests/interaction/lowlevel/test_lifecycle_stateless.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,15 @@ async def test_initialize_on_a_pinned_session_is_rejected_before_any_frame_is_se
102102
async with create_client_server_memory_streams() as ((client_read, client_write), (server_read, _server_write)):
103103
async with ClientSession(client_read, client_write, protocol_version="2026-07-28") as session:
104104
with anyio.fail_after(5):
105-
with pytest.raises(RuntimeError) as exc_info:
105+
with pytest.raises(RuntimeError) as exc_info: # pragma: no branch
106106
await session.initialize()
107107
assert str(exc_info.value) == snapshot(
108108
"initialize() must not be called on a session pinned to a stateless protocol version"
109109
)
110110
# Nothing left the client: closing the sender turns an empty buffer into EndOfStream.
111111
await client_write.aclose()
112112
with anyio.fail_after(5):
113-
with pytest.raises(anyio.EndOfStream):
113+
with pytest.raises(anyio.EndOfStream): # pragma: no branch
114114
await server_read.receive()
115115

116116

tests/server/test_experimental_streamable_http_modern.py

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,18 @@
66
mapping in ``handle()``, and the request-validation ladder in ``handle_modern_request``.
77
"""
88

9+
import logging
910
from collections.abc import Mapping
1011
from typing import Any
1112

13+
import anyio
1214
import httpx
1315
import pytest
1416
from starlette.requests import Request
1517
from starlette.types import Receive, Scope, Send
1618

17-
from mcp.server import Server
19+
import mcp.server._experimental.streamable_http_modern as modern
20+
from mcp.server import Server, ServerRequestContext
1821
from mcp.server._experimental.streamable_http_modern import (
1922
SingleExchangeDispatcher,
2023
_SingleExchangeDispatchContext,
@@ -24,7 +27,7 @@
2427
from mcp.shared.dispatcher import DispatchContext
2528
from mcp.shared.exceptions import NoBackChannelError
2629
from mcp.shared.transport_context import TransportContext
27-
from mcp.types import INVALID_PARAMS, PARSE_ERROR, JSONRPCError, JSONRPCRequest
30+
from mcp.types import INVALID_PARAMS, PARSE_ERROR, JSONRPCError, JSONRPCRequest, ListToolsResult, PaginatedRequestParams
2831

2932
pytestmark = pytest.mark.anyio
3033

@@ -98,6 +101,7 @@ async def test_handle_modern_request_rejects_non_post_with_405() -> None:
98101
async with _asgi_client(Server("test")) as http:
99102
response = await http.get("/mcp")
100103
assert response.status_code == 405
104+
assert response.headers["allow"] == "POST"
101105

102106

103107
async def test_handle_modern_request_rejects_malformed_body_with_parse_error() -> None:
@@ -106,7 +110,11 @@ async def test_handle_modern_request_rejects_malformed_body_with_parse_error() -
106110
response = await http.post("/mcp", content=b"not json", headers={"content-type": "application/json"})
107111
assert response.status_code == 400
108112
assert response.headers["content-type"].split(";", 1)[0] == "application/json"
109-
assert response.json() == {"jsonrpc": "2.0", "error": {"code": PARSE_ERROR, "message": "Parse error"}}
113+
assert response.json() == {
114+
"jsonrpc": "2.0",
115+
"id": None,
116+
"error": {"code": PARSE_ERROR, "message": "Parse error", "data": None},
117+
}
110118

111119

112120
async def test_handle_modern_request_returns_transport_security_error_response() -> None:
@@ -116,3 +124,66 @@ async def test_handle_modern_request_returns_transport_security_error_response()
116124
response = await http.post("/mcp", json={}, headers={"content-type": "application/json"})
117125
assert response.status_code == 421
118126
assert response.text == "Invalid Host header"
127+
128+
129+
def _list_tools_body() -> dict[str, Any]:
130+
"""A minimal valid 2026-07-28 ``tools/list`` request body, including the required ``_meta`` envelope."""
131+
meta = {
132+
"io.modelcontextprotocol/protocolVersion": "2026-07-28",
133+
"io.modelcontextprotocol/clientInfo": {"name": "raw", "version": "0.0.0"},
134+
"io.modelcontextprotocol/clientCapabilities": {},
135+
}
136+
return {"jsonrpc": "2.0", "id": 1, "method": "tools/list", "params": {"_meta": meta}}
137+
138+
139+
async def test_handle_modern_request_sends_response_when_exit_stack_cleanup_raises(
140+
caplog: pytest.LogCaptureFixture,
141+
) -> None:
142+
"""A raising ``connection.exit_stack`` callback is logged and swallowed; the computed result still ships.
143+
144+
The exit-stack guard mirrors ``ServerRunner.run``: cleanup runs in a ``finally`` after the
145+
handler, and an exception there must not displace the JSON-RPC response that was already built.
146+
"""
147+
148+
async def boom() -> None:
149+
raise RuntimeError("cleanup failed")
150+
151+
async def list_tools(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult:
152+
ctx.session._connection.exit_stack.push_async_callback(boom)
153+
return ListToolsResult(tools=[], ttl_ms=0, cache_scope="public")
154+
155+
with caplog.at_level(logging.ERROR, logger=modern.__name__):
156+
async with _asgi_client(Server("test", on_list_tools=list_tools)) as http:
157+
response = await http.post("/mcp", json=_list_tools_body(), headers={"content-type": "application/json"})
158+
159+
assert response.status_code == 200
160+
assert response.json()["result"]["tools"] == []
161+
assert "connection exit_stack cleanup raised" in caplog.text
162+
163+
164+
async def test_handle_modern_request_sends_response_when_exit_stack_cleanup_hangs(
165+
monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
166+
) -> None:
167+
"""A blocking ``connection.exit_stack`` callback is abandoned at the grace deadline; the response still ships.
168+
169+
Grace patched to 0 so the deadline is already expired on entry: the bounded unwind cancels the
170+
blocker at its first checkpoint, the abandonment warning is logged, and the JSON-RPC response
171+
that was built before cleanup is sent unchanged.
172+
"""
173+
monkeypatch.setattr(modern, "_EXIT_STACK_CLOSE_TIMEOUT", 0)
174+
175+
async def block() -> None:
176+
await anyio.Event().wait()
177+
raise AssertionError("unreachable") # pragma: no cover
178+
179+
async def list_tools(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult:
180+
ctx.session._connection.exit_stack.push_async_callback(block)
181+
return ListToolsResult(tools=[], ttl_ms=0, cache_scope="public")
182+
183+
with anyio.fail_after(5), caplog.at_level(logging.WARNING, logger=modern.__name__):
184+
async with _asgi_client(Server("test", on_list_tools=list_tools)) as http:
185+
response = await http.post("/mcp", json=_list_tools_body(), headers={"content-type": "application/json"})
186+
187+
assert response.status_code == 200
188+
assert response.json()["result"]["tools"] == []
189+
assert "abandoning remaining callbacks" in caplog.text

0 commit comments

Comments
 (0)