Skip to content

Commit ec8de42

Browse files
fix: silence respond() when concurrent cancellation already completed request
When a CancelledNotification arrives after a handler returns its result but before respond() is called, cancel() sets _completed = True and sends an error response. The subsequent respond() call was crashing with AssertionError because the assert guarded against double-respond but didn't account for this legitimate concurrent-cancellation window. Replace the assert with an early return so that respond() becomes a no-op when _completed is already True, matching the intended semantics: the request is already handled, so the late response is safely discarded. Fixes #2416
1 parent f27d2aa commit ec8de42

2 files changed

Lines changed: 110 additions & 2 deletions

File tree

src/mcp/shared/session.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,11 @@ async def respond(self, response: SendResultT | ErrorData) -> None:
126126
127127
Raises:
128128
RuntimeError: If not used within a context manager
129-
AssertionError: If request was already responded to
130129
"""
131130
if not self._entered: # pragma: no cover
132131
raise RuntimeError("RequestResponder must be used as a context manager")
133-
assert not self._completed, "Request already responded to"
132+
if self._completed:
133+
return
134134

135135
if not self.cancelled: # pragma: no branch
136136
self._completed = True
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
"""Test that respond() is a no-op when a concurrent cancellation already completed the request.
2+
3+
When a CancelledNotification arrives after the handler has returned its result but before
4+
respond() is called, cancel() sets _completed = True and sends an error response. The
5+
subsequent respond() call must return silently rather than crashing with AssertionError.
6+
"""
7+
8+
import anyio
9+
import pytest
10+
11+
from mcp import types
12+
from mcp.server.models import InitializationOptions
13+
from mcp.server.session import ServerSession
14+
from mcp.shared.message import SessionMessage
15+
from mcp.shared.session import RequestResponder
16+
from mcp.types import ServerCapabilities
17+
18+
19+
@pytest.mark.anyio
20+
async def test_respond_after_cancellation_is_silent() -> None:
21+
"""respond() must return silently when _completed is True.
22+
23+
This guards the race window in _handle_request where a CancelledNotification
24+
arrives after the handler returns but before respond() is called:
25+
1. cancel() sets _completed = True and sends an error response
26+
2. respond() is called — must return silently, not crash with AssertionError
27+
"""
28+
server_to_client_send, server_to_client_receive = anyio.create_memory_object_stream[SessionMessage](10)
29+
client_to_server_send, client_to_server_receive = anyio.create_memory_object_stream[SessionMessage | Exception](10)
30+
31+
respond_raised = False
32+
respond_called = False
33+
34+
async def run_server() -> None:
35+
nonlocal respond_raised, respond_called
36+
37+
async with ServerSession(
38+
client_to_server_receive,
39+
server_to_client_send,
40+
InitializationOptions(
41+
server_name="test-server",
42+
server_version="1.0.0",
43+
capabilities=ServerCapabilities(tools=types.ToolsCapability(list_changed=False)),
44+
),
45+
) as server_session:
46+
async for message in server_session.incoming_messages: # pragma: no branch
47+
if isinstance(message, Exception): # pragma: no cover
48+
raise message
49+
50+
if isinstance(message, RequestResponder):
51+
if isinstance(message.request, types.ListToolsRequest): # pragma: no branch
52+
with message:
53+
# Simulate: concurrent cancellation set _completed = True
54+
# (as if cancel() already ran and sent the error response)
55+
message._completed = True # type: ignore[reportPrivateUsage]
56+
respond_called = True
57+
try:
58+
await message.respond(types.ListToolsResult(tools=[]))
59+
except Exception: # pragma: no cover
60+
respond_raised = True
61+
return
62+
63+
if isinstance(message, types.ClientNotification): # pragma: no cover
64+
if isinstance(message, types.InitializedNotification):
65+
return
66+
67+
async def mock_client() -> None:
68+
await client_to_server_send.send(
69+
SessionMessage(
70+
types.JSONRPCRequest(
71+
jsonrpc="2.0",
72+
id=1,
73+
method="initialize",
74+
params=types.InitializeRequestParams(
75+
protocol_version=types.LATEST_PROTOCOL_VERSION,
76+
capabilities=types.ClientCapabilities(),
77+
client_info=types.Implementation(name="test-client", version="1.0.0"),
78+
).model_dump(by_alias=True, mode="json", exclude_none=True),
79+
)
80+
)
81+
)
82+
83+
await server_to_client_receive.receive() # InitializeResult
84+
85+
await client_to_server_send.send(
86+
SessionMessage(types.JSONRPCRequest(jsonrpc="2.0", id=2, method="tools/list"))
87+
)
88+
89+
# Drain any pending messages (server may have sent nothing for the silenced respond)
90+
with anyio.fail_after(3):
91+
try:
92+
while True:
93+
await server_to_client_receive.receive()
94+
except anyio.EndOfStream:
95+
pass
96+
97+
async with (
98+
client_to_server_send,
99+
client_to_server_receive,
100+
server_to_client_send,
101+
server_to_client_receive,
102+
anyio.create_task_group() as tg,
103+
):
104+
tg.start_soon(run_server)
105+
tg.start_soon(mock_client)
106+
107+
assert respond_called, "respond() was never invoked"
108+
assert not respond_raised, "respond() raised an exception after concurrent cancellation"

0 commit comments

Comments
 (0)