Fail fast on server-to-client requests in JSON-response mode instead of hanging (#3195)
This commit is contained in:
@@ -2608,6 +2608,16 @@ REQUIREMENTS: dict[str, Requirement] = {
|
||||
"overtake anything already queued for the request."
|
||||
),
|
||||
),
|
||||
"transport:streamable-http:json-response-restrictions": Requirement(
|
||||
source="sdk",
|
||||
behavior=(
|
||||
"In JSON-response mode a handler's request-scoped server-initiated request fails fast with an "
|
||||
"INVALID_REQUEST protocol error and request-scoped notifications are not delivered, because the "
|
||||
"single JSON body carries only the response; the connection's standalone stream is unaffected."
|
||||
),
|
||||
transports=("streamable-http",),
|
||||
note="Only observable over streamable HTTP: JSON-response mode is an HTTP framing option.",
|
||||
),
|
||||
"transport:streamable-http:stateless": Requirement(
|
||||
source=f"{SPEC_BASE_URL}/basic/transports#streamable-http",
|
||||
behavior=(
|
||||
|
||||
@@ -64,7 +64,8 @@ def _smoke_server() -> MCPServer:
|
||||
async def ask(ctx: Context) -> str:
|
||||
"""Elicit a confirmation from the client and report the outcome."""
|
||||
answer = await ctx.elicit("Proceed?", Confirmation)
|
||||
# In stateless mode the elicit raises before this point: there is no session to call back through.
|
||||
# In stateless and JSON-response modes the elicit raises before this point: there is no
|
||||
# request-scoped channel to call back through.
|
||||
assert isinstance(answer, AcceptedElicitation)
|
||||
return f"confirmed={answer.data.confirmed}"
|
||||
|
||||
@@ -120,6 +121,48 @@ async def test_stateless_streamable_http_rejects_server_initiated_requests() ->
|
||||
assert exc_info.value.error.code == INVALID_REQUEST
|
||||
|
||||
|
||||
@requirement("transport:streamable-http:json-response-restrictions")
|
||||
async def test_json_response_streamable_http_rejects_request_scoped_server_requests() -> None:
|
||||
"""A handler that calls back to the client mid-request fails fast when the server answers with
|
||||
JSON: the one response body cannot carry the nested `elicitation/create`, so the request-scoped
|
||||
channel raises `NoBackChannelError` (a top-level `MCPError`) instead of parking a waiter no reply
|
||||
could ever reach. Bounded, because before the fix this call hung until it timed out."""
|
||||
async with connect_over_streamable_http(_smoke_server(), json_response=True) as client:
|
||||
with anyio.fail_after(5), pytest.raises(MCPError) as exc_info:
|
||||
await client.call_tool("ask", {})
|
||||
|
||||
assert exc_info.value.error.code == INVALID_REQUEST
|
||||
|
||||
|
||||
@requirement("transport:streamable-http:json-response-restrictions")
|
||||
@requirement("transport:streamable-http:unrelated-messages")
|
||||
@requirement("hosting:http:standalone-sse")
|
||||
async def test_json_response_streamable_http_delivers_only_unrelated_notifications() -> None:
|
||||
"""In JSON-response mode the call's own log notification has no stream to ride and never
|
||||
reaches the client, while the tool result comes back as the JSON body and the unrelated
|
||||
resource-updated notification arrives on the standalone stream. The handler writes both
|
||||
notifications before returning, so once the result and the unrelated message are in, no
|
||||
request-scoped message can still be in flight."""
|
||||
received: list[IncomingMessage] = []
|
||||
server_message_seen = anyio.Event()
|
||||
|
||||
async def collect(message: IncomingMessage) -> None:
|
||||
received.append(message)
|
||||
server_message_seen.set()
|
||||
|
||||
async with connect_over_streamable_http(_smoke_server(), json_response=True, message_handler=collect) as client:
|
||||
with anyio.fail_after(5):
|
||||
result = await client.call_tool("announce", {})
|
||||
await server_message_seen.wait()
|
||||
|
||||
assert result == snapshot(
|
||||
CallToolResult(content=[TextContent(text="announced")], structured_content={"result": "announced"})
|
||||
)
|
||||
assert received == snapshot(
|
||||
[ResourceUpdatedNotification(params=ResourceUpdatedNotificationParams(uri="file:///watched.txt"))]
|
||||
)
|
||||
|
||||
|
||||
@requirement("transport:streamable-http:notifications")
|
||||
@requirement("transport:streamable-http:unrelated-messages")
|
||||
@requirement("hosting:http:standalone-sse")
|
||||
|
||||
@@ -25,6 +25,25 @@ class _PrimingFailingStore(EventStore):
|
||||
raise NotImplementedError
|
||||
|
||||
|
||||
class _AsgiPost:
|
||||
"""A one-shot POST driven straight at `handle_request`, capturing what the transport sends."""
|
||||
|
||||
def __init__(self, body: bytes, headers: list[tuple[bytes, bytes]]) -> None:
|
||||
self.scope: Scope = {"type": "http", "method": "POST", "path": "/", "query_string": b"", "headers": headers}
|
||||
self.sent: list[Message] = []
|
||||
self._body = body
|
||||
self._body_sent = False
|
||||
|
||||
async def receive(self) -> Message:
|
||||
if not self._body_sent:
|
||||
self._body_sent = True
|
||||
return {"type": "http.request", "body": self._body, "more_body": False}
|
||||
raise NotImplementedError
|
||||
|
||||
async def send(self, message: Message) -> None:
|
||||
self.sent.append(message)
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_router_unconsumed_request_stream_does_not_block_siblings() -> None:
|
||||
"""A response whose `sse_writer` is not yet receiving must not park the router (#1764).
|
||||
@@ -73,35 +92,18 @@ async def test_priming_store_failure_leaves_no_per_request_state() -> None:
|
||||
event_store=_PrimingFailingStore(),
|
||||
)
|
||||
|
||||
body = b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}'
|
||||
scope: Scope = {
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/",
|
||||
"query_string": b"",
|
||||
"headers": [
|
||||
post = _AsgiPost(
|
||||
b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}',
|
||||
[
|
||||
(b"accept", b"application/json, text/event-stream"),
|
||||
(b"content-type", b"application/json"),
|
||||
(b"mcp-protocol-version", b"2025-11-25"),
|
||||
],
|
||||
}
|
||||
body_sent = False
|
||||
|
||||
async def receive() -> Message:
|
||||
nonlocal body_sent
|
||||
if not body_sent:
|
||||
body_sent = True
|
||||
return {"type": "http.request", "body": body, "more_body": False}
|
||||
raise NotImplementedError
|
||||
|
||||
sent: list[Message] = []
|
||||
|
||||
async def asgi_send(message: Message) -> None:
|
||||
sent.append(message)
|
||||
)
|
||||
|
||||
async with transport.connect() as (read_stream, _write_stream):
|
||||
async with anyio.create_task_group() as tg:
|
||||
tg.start_soon(transport.handle_request, scope, receive, asgi_send)
|
||||
tg.start_soon(transport.handle_request, post.scope, post.receive, post.send)
|
||||
with anyio.fail_after(5):
|
||||
forwarded = await read_stream.receive()
|
||||
assert isinstance(forwarded, Exception)
|
||||
@@ -110,7 +112,32 @@ async def test_priming_store_failure_leaves_no_per_request_state() -> None:
|
||||
assert transport._request_streams == {}
|
||||
assert transport._sse_stream_writers == {}
|
||||
|
||||
assert sent[0]["type"] == "http.response.start"
|
||||
assert sent[0]["status"] == 500
|
||||
body = b"".join(m.get("body", b"") for m in sent if m["type"] == "http.response.body")
|
||||
assert post.sent[0]["type"] == "http.response.start"
|
||||
assert post.sent[0]["status"] == 500
|
||||
body = b"".join(m.get("body", b"") for m in post.sent if m["type"] == "http.response.body")
|
||||
assert b"backend unavailable" not in body
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_json_post_answers_500_when_session_terminates_mid_request() -> None:
|
||||
"""A JSON-mode POST whose session is torn down before the handler answers gets a 500, not a stall."""
|
||||
transport = StreamableHTTPServerTransport(mcp_session_id="sid", is_json_response_enabled=True)
|
||||
post = _AsgiPost(
|
||||
b'{"jsonrpc":"2.0","id":"req-1","method":"tools/list","params":{}}',
|
||||
[
|
||||
(b"accept", b"application/json"),
|
||||
(b"content-type", b"application/json"),
|
||||
(b"mcp-session-id", b"sid"),
|
||||
(b"mcp-protocol-version", b"2025-11-25"),
|
||||
],
|
||||
)
|
||||
|
||||
async with transport.connect() as (read_stream, _write_stream):
|
||||
async with anyio.create_task_group() as tg:
|
||||
tg.start_soon(transport.handle_request, post.scope, post.receive, post.send)
|
||||
with anyio.fail_after(5):
|
||||
await read_stream.receive() # the request reached the session; the POST is parked
|
||||
await transport.terminate()
|
||||
|
||||
assert post.sent[0]["type"] == "http.response.start"
|
||||
assert post.sent[0]["status"] == 500
|
||||
|
||||
@@ -1329,6 +1329,45 @@ async def test_ctx_message_metadata_carries_inbound_request_metadata():
|
||||
assert seen[0] is metadata # the exact object, passed through verbatim
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_transport_stamped_can_send_request_makes_the_request_channel_refuse():
|
||||
"""A transport that marks a message `can_send_request=False` on its metadata gets a request-scoped
|
||||
channel that raises `NoBackChannelError` immediately - the default builder reads the transport's
|
||||
verdict off the message, so no driver has to wire it."""
|
||||
c2s_send, c2s_recv = anyio.create_memory_object_stream[SessionMessage | Exception](32)
|
||||
s2c_send, s2c_recv = anyio.create_memory_object_stream[SessionMessage | Exception](32)
|
||||
server: JSONRPCDispatcher[TransportContext] = JSONRPCDispatcher(c2s_recv, s2c_send)
|
||||
outcomes: list[bool | str] = []
|
||||
|
||||
async def on_request(ctx: DCtx, method: str, params: Mapping[str, Any] | None) -> dict[str, Any]:
|
||||
outcomes.append(ctx.can_send_request)
|
||||
try:
|
||||
await ctx.send_raw_request("elicitation/create", {})
|
||||
except NoBackChannelError as exc:
|
||||
outcomes.append(exc.method)
|
||||
return {}
|
||||
|
||||
async def on_notify(ctx: DCtx, method: str, params: Mapping[str, Any] | None) -> None:
|
||||
raise NotImplementedError
|
||||
|
||||
try:
|
||||
async with anyio.create_task_group() as tg:
|
||||
await tg.start(server.run, on_request, on_notify)
|
||||
await c2s_send.send(
|
||||
SessionMessage(
|
||||
message=JSONRPCRequest(jsonrpc="2.0", id=1, method="tools/call", params=None),
|
||||
metadata=ServerMessageMetadata(can_send_request=False),
|
||||
)
|
||||
)
|
||||
with anyio.fail_after(5):
|
||||
await s2c_recv.receive() # response sent => the handler has run
|
||||
tg.cancel_scope.cancel()
|
||||
finally:
|
||||
for s in (c2s_send, c2s_recv, s2c_send, s2c_recv):
|
||||
s.close()
|
||||
assert outcomes == [False, "elicitation/create"]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_ctx_message_metadata_carries_inbound_notification_metadata():
|
||||
"""Notifications get the same metadata pass-through as requests."""
|
||||
|
||||
Reference in New Issue
Block a user