Skip to content

Commit f66e1a8

Browse files
committed
fix: stop sending JSON-RPC response on request cancellation
1 parent 3d7b311 commit f66e1a8

4 files changed

Lines changed: 41 additions & 35 deletions

File tree

src/mcp/server/lowlevel/server.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,9 +502,9 @@ async def _handle_request(
502502
response = err.error
503503
except anyio.get_cancelled_exc_class():
504504
if message.cancelled:
505-
# Client sent CancelledNotification; responder.cancel() already
506-
# sent an error response, so skip the duplicate.
507-
logger.info("Request %s cancelled - duplicate response suppressed", message.request_id)
505+
# Client sent CancelledNotification; per the MCP spec the
506+
# receiver MUST NOT send a response for the cancelled request.
507+
logger.info("Request %s cancelled - no response sent per spec", message.request_id)
508508
return
509509
# Transport-close cancellation from the TG in run(); re-raise so the
510510
# TG swallows its own cancellation.

src/mcp/shared/session.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,8 @@ async def cancel(self) -> None:
148148

149149
self._cancel_scope.cancel()
150150
self._completed = True # Mark as completed so it's removed from in_flight
151-
# Send an error response to indicate cancellation
152-
await self._session._send_response( # type: ignore[reportPrivateUsage]
153-
request_id=self.request_id,
154-
response=ErrorData(code=0, message="Request cancelled"),
155-
)
151+
# Per the MCP spec, receivers of cancellation notifications SHOULD NOT
152+
# send a response for the cancelled request.
156153

157154
@property
158155
def in_flight(self) -> bool: # pragma: no cover

tests/server/test_cancel_handling.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ async def first_request():
6666
await client.session.send_request(
6767
CallToolRequest(params=CallToolRequestParams(name="test_tool", arguments={})),
6868
CallToolResult,
69+
request_read_timeout_seconds=3,
6970
)
7071
pytest.fail("First request should have been cancelled") # pragma: no cover
7172
except MCPError:
72-
pass # Expected
73+
pass # Expected - request timed out since no response is sent for cancelled requests
7374

7475
# Start first request
7576
async with anyio.create_task_group() as tg:

tests/shared/test_session.py

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,27 @@ async def test_in_flight_requests_cleared_after_completion():
3838

3939
@pytest.mark.anyio
4040
async def test_request_cancellation():
41-
"""Test that requests can be cancelled while in-flight."""
41+
"""Test that requests can be cancelled while in-flight.
42+
43+
Per the MCP cancellation spec, the server MUST NOT send a response for a
44+
cancelled request. Instead, the server cancels the handler's cancel scope.
45+
The client may time out waiting for a response that will never arrive.
46+
"""
4247
ev_tool_called = anyio.Event()
43-
ev_cancelled = anyio.Event()
48+
ev_handler_cancelled = anyio.Event()
4449
request_id = None
4550

46-
# Create a server with a slow tool
51+
# Create a server with a slow tool that detects cancellation
4752
async def handle_call_tool(ctx: ServerRequestContext, params: types.CallToolRequestParams) -> types.CallToolResult:
48-
nonlocal request_id, ev_tool_called
53+
nonlocal request_id, ev_tool_called, ev_handler_cancelled
4954
if params.name == "slow_tool":
5055
request_id = ctx.request_id
5156
ev_tool_called.set()
52-
await anyio.sleep(10) # Long enough to ensure we can cancel
57+
try:
58+
await anyio.sleep(10) # Long enough to ensure we can cancel
59+
except anyio.get_cancelled_exc_class():
60+
ev_handler_cancelled.set()
61+
raise
5362
return types.CallToolResult(content=[]) # pragma: no cover
5463
raise ValueError(f"Unknown tool: {params.name}") # pragma: no cover
5564

@@ -64,27 +73,23 @@ async def handle_list_tools(
6473
on_list_tools=handle_list_tools,
6574
)
6675

67-
async def make_request(client: Client):
68-
nonlocal ev_cancelled
69-
try:
70-
await client.session.send_request(
71-
types.CallToolRequest(
72-
params=types.CallToolRequestParams(name="slow_tool", arguments={}),
73-
),
74-
types.CallToolResult,
75-
)
76-
pytest.fail("Request should have been cancelled") # pragma: no cover
77-
except MCPError as e:
78-
# Expected - request was cancelled
79-
assert "Request cancelled" in str(e)
80-
ev_cancelled.set()
81-
8276
async with Client(server) as client:
83-
async with anyio.create_task_group() as tg: # pragma: no branch
84-
tg.start_soon(make_request, client)
77+
# Start the request in a task group we control
78+
async with anyio.create_task_group() as tg:
79+
80+
async def send_slow_request():
81+
with anyio.move_on_after(5):
82+
await client.session.send_request(
83+
types.CallToolRequest(
84+
params=types.CallToolRequestParams(name="slow_tool", arguments={}),
85+
),
86+
types.CallToolResult,
87+
)
88+
89+
tg.start_soon(send_slow_request)
8590

8691
# Wait for the request to be in-flight
87-
with anyio.fail_after(1): # Timeout after 1 second
92+
with anyio.fail_after(1):
8893
await ev_tool_called.wait()
8994

9095
# Send cancellation notification
@@ -93,9 +98,12 @@ async def make_request(client: Client):
9398
CancelledNotification(params=CancelledNotificationParams(request_id=request_id))
9499
)
95100

96-
# Give cancellation time to process
97-
with anyio.fail_after(1): # pragma: no branch
98-
await ev_cancelled.wait()
101+
# Verify the server handler was cancelled
102+
with anyio.fail_after(1):
103+
await ev_handler_cancelled.wait()
104+
105+
# Clean up: cancel the request task which is still waiting for a response
106+
tg.cancel_scope.cancel()
99107

100108

101109
@pytest.mark.anyio

0 commit comments

Comments
 (0)