From d4e14a49a7155eebdae65a9bdfa63ef61ffcf4a1 Mon Sep 17 00:00:00 2001 From: Luka Crnkovic-Friis Date: Wed, 16 Jul 2025 12:31:46 +0200 Subject: [PATCH 1/6] server: skip duplicate response on CancelledError When a tool call is cancelled, RequestResponder.cancel() sends an error response. Previously, _handle_request would try to send another error response, causing "Request already responded to" assertion. This fix catches CancelledError and returns early, preventing the duplicate response. Fixes #1152 --- src/mcp/server/lowlevel/server.py | 7 ++ tests/server/test_cancel_handling.py | 161 +++++++++++++++++++++++++++ 2 files changed, 168 insertions(+) create mode 100644 tests/server/test_cancel_handling.py diff --git a/src/mcp/server/lowlevel/server.py b/src/mcp/server/lowlevel/server.py index 562de31b7..db17ebe46 100644 --- a/src/mcp/server/lowlevel/server.py +++ b/src/mcp/server/lowlevel/server.py @@ -67,6 +67,7 @@ async def main(): from __future__ import annotations as _annotations +import asyncio import contextvars import json import logging @@ -647,6 +648,12 @@ async def _handle_request( response = await handler(req) except McpError as err: response = err.error + except asyncio.CancelledError: + logger.info( + "Request %s cancelled - duplicate response suppressed", + message.request_id, + ) + return except Exception as err: if raise_exceptions: raise err diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py new file mode 100644 index 000000000..580da1e1a --- /dev/null +++ b/tests/server/test_cancel_handling.py @@ -0,0 +1,161 @@ +"""Test that cancelled requests don't cause double responses.""" + +import asyncio +from unittest.mock import MagicMock + +import pytest + +import mcp.types as types +from mcp.server.lowlevel.server import Server +from mcp.types import PingRequest, ServerResult + + +# Shared mock class +class MockRequestResponder: + def __init__(self): + self.request_id = "test-123" + self._responded = False + self.request_meta = {} + self.message_metadata = None + + async def send(self, response): + if self._responded: + raise AssertionError(f"Request {self.request_id} already responded to") + self._responded = True + + async def respond(self, response): + await self.send(response) + + def cancel(self): + """Simulate the cancel() method sending an error response.""" + asyncio.create_task(self.send(ServerResult( + error=types.ErrorData( + code=-32800, + message="Request cancelled" + ) + ))) + + +@pytest.mark.asyncio +async def test_cancelled_request_no_double_response(): + """Verify server handles cancelled requests without double response.""" + + # Create a server instance + server = Server("test-server") + + # Track if multiple responses are attempted + response_count = 0 + + # Override the send method to track calls + mock_message = MockRequestResponder() + original_send = mock_message.send + + async def tracked_send(response): + nonlocal response_count + response_count += 1 + await original_send(response) + + mock_message.send = tracked_send + + # Create a slow handler that will be cancelled + async def slow_handler(req): + await asyncio.sleep(10) + return types.ServerResult(types.EmptyResult()) + + # Use PingRequest as it's a valid request type + server.request_handlers[types.PingRequest] = slow_handler + + # Create mock message and session + mock_req = PingRequest(method="ping", params={}) + mock_session = MagicMock() + mock_context = None + + # Start the request + handle_task = asyncio.create_task( + server._handle_request( + mock_message, + mock_req, + mock_session, + mock_context, + raise_exceptions=False + ) + ) + + # Give it time to start + await asyncio.sleep(0.1) + + # Simulate cancellation + mock_message.cancel() + handle_task.cancel() + + # Wait for cancellation to propagate + try: + await handle_task + except asyncio.CancelledError: + pass + + # Give time for any duplicate response attempts + await asyncio.sleep(0.1) + + # Should only have one response (from cancel()) + assert response_count == 1, f"Expected 1 response, got {response_count}" + + +@pytest.mark.asyncio +async def test_server_remains_functional_after_cancel(): + """Verify server can handle new requests after a cancellation.""" + + server = Server("test-server") + + # Add handlers + async def slow_handler(req): + await asyncio.sleep(5) + return types.ServerResult(types.EmptyResult()) + + async def fast_handler(req): + return types.ServerResult(types.EmptyResult()) + + # Override ping handler for our test + server.request_handlers[types.PingRequest] = slow_handler + + # First request (will be cancelled) + mock_message1 = MockRequestResponder() + mock_req1 = PingRequest(method="ping", params={}) + + handle_task = asyncio.create_task( + server._handle_request( + mock_message1, + mock_req1, + MagicMock(), + None, + raise_exceptions=False + ) + ) + + await asyncio.sleep(0.1) + mock_message1.cancel() + handle_task.cancel() + + try: + await handle_task + except asyncio.CancelledError: + pass + + # Change handler to fast one + server.request_handlers[types.PingRequest] = fast_handler + + # Second request (should work normally) + mock_message2 = MockRequestResponder() + mock_req2 = PingRequest(method="ping", params={}) + + # This should complete successfully + await server._handle_request( + mock_message2, + mock_req2, + MagicMock(), + None, + raise_exceptions=False + ) + + # Server handled the second request successfully + assert mock_message2._responded \ No newline at end of file From 36805b6dd4e878702304d13e5cf0a75a3410201c Mon Sep 17 00:00:00 2001 From: Luka Crnkovic-Friis Date: Wed, 16 Jul 2025 12:34:34 +0200 Subject: [PATCH 2/6] Apply ruff formatting to test file --- tests/server/test_cancel_handling.py | 93 +++++++++++----------------- 1 file changed, 35 insertions(+), 58 deletions(-) diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py index 580da1e1a..a9865d32d 100644 --- a/tests/server/test_cancel_handling.py +++ b/tests/server/test_cancel_handling.py @@ -17,145 +17,122 @@ def __init__(self): self._responded = False self.request_meta = {} self.message_metadata = None - + async def send(self, response): if self._responded: raise AssertionError(f"Request {self.request_id} already responded to") self._responded = True - + async def respond(self, response): await self.send(response) - + def cancel(self): """Simulate the cancel() method sending an error response.""" - asyncio.create_task(self.send(ServerResult( - error=types.ErrorData( - code=-32800, - message="Request cancelled" - ) - ))) + asyncio.create_task(self.send(ServerResult(error=types.ErrorData(code=-32800, message="Request cancelled")))) @pytest.mark.asyncio async def test_cancelled_request_no_double_response(): """Verify server handles cancelled requests without double response.""" - + # Create a server instance server = Server("test-server") - + # Track if multiple responses are attempted response_count = 0 - + # Override the send method to track calls mock_message = MockRequestResponder() original_send = mock_message.send - + async def tracked_send(response): nonlocal response_count response_count += 1 await original_send(response) - + mock_message.send = tracked_send - + # Create a slow handler that will be cancelled async def slow_handler(req): await asyncio.sleep(10) return types.ServerResult(types.EmptyResult()) - + # Use PingRequest as it's a valid request type server.request_handlers[types.PingRequest] = slow_handler - + # Create mock message and session mock_req = PingRequest(method="ping", params={}) mock_session = MagicMock() mock_context = None - + # Start the request handle_task = asyncio.create_task( - server._handle_request( - mock_message, - mock_req, - mock_session, - mock_context, - raise_exceptions=False - ) + server._handle_request(mock_message, mock_req, mock_session, mock_context, raise_exceptions=False) ) - + # Give it time to start await asyncio.sleep(0.1) - + # Simulate cancellation mock_message.cancel() handle_task.cancel() - + # Wait for cancellation to propagate try: await handle_task except asyncio.CancelledError: pass - + # Give time for any duplicate response attempts await asyncio.sleep(0.1) - + # Should only have one response (from cancel()) assert response_count == 1, f"Expected 1 response, got {response_count}" -@pytest.mark.asyncio +@pytest.mark.asyncio async def test_server_remains_functional_after_cancel(): """Verify server can handle new requests after a cancellation.""" - + server = Server("test-server") - + # Add handlers async def slow_handler(req): await asyncio.sleep(5) return types.ServerResult(types.EmptyResult()) - + async def fast_handler(req): return types.ServerResult(types.EmptyResult()) - + # Override ping handler for our test server.request_handlers[types.PingRequest] = slow_handler - + # First request (will be cancelled) mock_message1 = MockRequestResponder() mock_req1 = PingRequest(method="ping", params={}) - + handle_task = asyncio.create_task( - server._handle_request( - mock_message1, - mock_req1, - MagicMock(), - None, - raise_exceptions=False - ) + server._handle_request(mock_message1, mock_req1, MagicMock(), None, raise_exceptions=False) ) - + await asyncio.sleep(0.1) mock_message1.cancel() handle_task.cancel() - + try: await handle_task except asyncio.CancelledError: pass - + # Change handler to fast one server.request_handlers[types.PingRequest] = fast_handler - + # Second request (should work normally) mock_message2 = MockRequestResponder() mock_req2 = PingRequest(method="ping", params={}) - + # This should complete successfully - await server._handle_request( - mock_message2, - mock_req2, - MagicMock(), - None, - raise_exceptions=False - ) - + await server._handle_request(mock_message2, mock_req2, MagicMock(), None, raise_exceptions=False) + # Server handled the second request successfully - assert mock_message2._responded \ No newline at end of file + assert mock_message2._responded From 452dfb9488d495ab20873b65b3cce2abc7150afd Mon Sep 17 00:00:00 2001 From: Luka Crnkovic-Friis Date: Wed, 16 Jul 2025 12:35:49 +0200 Subject: [PATCH 3/6] Use pytest.mark.anyio instead of asyncio --- tests/server/test_cancel_handling.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py index a9865d32d..857f15ec1 100644 --- a/tests/server/test_cancel_handling.py +++ b/tests/server/test_cancel_handling.py @@ -31,7 +31,7 @@ def cancel(self): asyncio.create_task(self.send(ServerResult(error=types.ErrorData(code=-32800, message="Request cancelled")))) -@pytest.mark.asyncio +@pytest.mark.anyio async def test_cancelled_request_no_double_response(): """Verify server handles cancelled requests without double response.""" @@ -90,7 +90,7 @@ async def slow_handler(req): assert response_count == 1, f"Expected 1 response, got {response_count}" -@pytest.mark.asyncio +@pytest.mark.anyio async def test_server_remains_functional_after_cancel(): """Verify server can handle new requests after a cancellation.""" From 58e1c6685c2a673bc7087a696f15977817f54152 Mon Sep 17 00:00:00 2001 From: Luka Crnkovic-Friis Date: Wed, 16 Jul 2025 12:37:50 +0200 Subject: [PATCH 4/6] Add type ignore comments for test mocks --- tests/server/test_cancel_handling.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py index 857f15ec1..ecbe279d2 100644 --- a/tests/server/test_cancel_handling.py +++ b/tests/server/test_cancel_handling.py @@ -28,7 +28,7 @@ async def respond(self, response): def cancel(self): """Simulate the cancel() method sending an error response.""" - asyncio.create_task(self.send(ServerResult(error=types.ErrorData(code=-32800, message="Request cancelled")))) + asyncio.create_task(self.send(types.ErrorData(code=-32800, message="Request cancelled"))) @pytest.mark.anyio @@ -67,7 +67,7 @@ async def slow_handler(req): # Start the request handle_task = asyncio.create_task( - server._handle_request(mock_message, mock_req, mock_session, mock_context, raise_exceptions=False) + server._handle_request(mock_message, mock_req, mock_session, mock_context, raise_exceptions=False) # type: ignore ) # Give it time to start @@ -112,7 +112,7 @@ async def fast_handler(req): mock_req1 = PingRequest(method="ping", params={}) handle_task = asyncio.create_task( - server._handle_request(mock_message1, mock_req1, MagicMock(), None, raise_exceptions=False) + server._handle_request(mock_message1, mock_req1, MagicMock(), None, raise_exceptions=False) # type: ignore ) await asyncio.sleep(0.1) @@ -132,7 +132,7 @@ async def fast_handler(req): mock_req2 = PingRequest(method="ping", params={}) # This should complete successfully - await server._handle_request(mock_message2, mock_req2, MagicMock(), None, raise_exceptions=False) + await server._handle_request(mock_message2, mock_req2, MagicMock(), None, raise_exceptions=False) # type: ignore # Server handled the second request successfully assert mock_message2._responded From 81f3e36912ad7cb6fdb729a8055aeb0f43a13961 Mon Sep 17 00:00:00 2001 From: Luka Crnkovic-Friis Date: Wed, 16 Jul 2025 12:44:05 +0200 Subject: [PATCH 5/6] Remove unused ServerResult import --- tests/server/test_cancel_handling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py index ecbe279d2..af003744f 100644 --- a/tests/server/test_cancel_handling.py +++ b/tests/server/test_cancel_handling.py @@ -7,7 +7,7 @@ import mcp.types as types from mcp.server.lowlevel.server import Server -from mcp.types import PingRequest, ServerResult +from mcp.types import PingRequest # Shared mock class From 81bd1910cc32f18fb65bab035b98136f7e5973b3 Mon Sep 17 00:00:00 2001 From: Luka Crnkovic-Friis Date: Wed, 16 Jul 2025 12:45:14 +0200 Subject: [PATCH 6/6] Remove empty params dict from PingRequest --- tests/server/test_cancel_handling.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/server/test_cancel_handling.py b/tests/server/test_cancel_handling.py index af003744f..fb8c955d4 100644 --- a/tests/server/test_cancel_handling.py +++ b/tests/server/test_cancel_handling.py @@ -61,7 +61,7 @@ async def slow_handler(req): server.request_handlers[types.PingRequest] = slow_handler # Create mock message and session - mock_req = PingRequest(method="ping", params={}) + mock_req = PingRequest(method="ping") mock_session = MagicMock() mock_context = None @@ -109,7 +109,7 @@ async def fast_handler(req): # First request (will be cancelled) mock_message1 = MockRequestResponder() - mock_req1 = PingRequest(method="ping", params={}) + mock_req1 = PingRequest(method="ping") handle_task = asyncio.create_task( server._handle_request(mock_message1, mock_req1, MagicMock(), None, raise_exceptions=False) # type: ignore @@ -129,7 +129,7 @@ async def fast_handler(req): # Second request (should work normally) mock_message2 = MockRequestResponder() - mock_req2 = PingRequest(method="ping", params={}) + mock_req2 = PingRequest(method="ping") # This should complete successfully await server._handle_request(mock_message2, mock_req2, MagicMock(), None, raise_exceptions=False) # type: ignore