Skip to content

Commit 5787554

Browse files
committed
fix(mcp): handle CancelledError in MCPSessionManager.create_session
When an MCP server returns an HTTP error (e.g., 401, 403), the MCP SDK uses anyio cancel scopes internally, which raise CancelledError. Since CancelledError is a BaseException (not Exception) in Python 3.8+, the existing `except Exception` block does not catch it. This causes the error to propagate uncaught, leading to: - Application hangs - ASGI callable returned without completing response errors - Inability to handle MCP connection failures gracefully This fix explicitly catches asyncio.CancelledError and converts it to a ConnectionError with a descriptive message, allowing proper error handling and cleanup. Fixes #3708
1 parent a9b853f commit 5787554

File tree

2 files changed

+55
-0
lines changed

2 files changed

+55
-0
lines changed

src/google/adk/tools/mcp_tool/mcp_session_manager.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,25 @@ async def create_session(
363363
logger.debug('Created new session: %s', session_key)
364364
return session
365365

366+
except asyncio.CancelledError as e:
367+
# CancelledError can occur when the MCP server returns an HTTP error
368+
# (e.g., 401, 403). The MCP SDK uses anyio cancel scopes internally,
369+
# which raise CancelledError. Since CancelledError is a BaseException
370+
# (not Exception) in Python 3.8+, it must be caught explicitly.
371+
logger.warning(
372+
'MCP session creation cancelled (likely due to HTTP error): %s', e
373+
)
374+
try:
375+
await exit_stack.aclose()
376+
except Exception as exit_stack_error:
377+
logger.warning(
378+
'Error during cancelled session cleanup: %s', exit_stack_error
379+
)
380+
raise ConnectionError(
381+
f'MCP session creation cancelled (server may have returned HTTP'
382+
f' error): {e}'
383+
) from e
384+
366385
except Exception as e:
367386
# If session creation fails, clean up the exit stack
368387
if exit_stack:

tests/unittests/tools/mcp_tool/test_mcp_session_manager.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,42 @@ async def test_create_session_timeout(
290290
# Verify cleanup was called
291291
mock_exit_stack.aclose.assert_called_once()
292292

293+
@pytest.mark.asyncio
294+
@patch("google.adk.tools.mcp_tool.mcp_session_manager.stdio_client")
295+
@patch("google.adk.tools.mcp_tool.mcp_session_manager.AsyncExitStack")
296+
async def test_create_session_cancelled_error(
297+
self, mock_exit_stack_class, mock_stdio
298+
):
299+
"""Test session creation when CancelledError is raised (e.g., HTTP 403).
300+
301+
When an MCP server returns an HTTP error (e.g., 401, 403), the MCP SDK
302+
uses anyio cancel scopes internally, which raise CancelledError. This
303+
test verifies that CancelledError is caught and converted to a
304+
ConnectionError with proper cleanup.
305+
"""
306+
manager = MCPSessionManager(self.mock_stdio_connection_params)
307+
308+
mock_exit_stack = MockAsyncExitStack()
309+
310+
mock_exit_stack_class.return_value = mock_exit_stack
311+
mock_stdio.return_value = AsyncMock()
312+
313+
# Simulate CancelledError during session creation (e.g., HTTP 403)
314+
mock_exit_stack.enter_async_context.side_effect = asyncio.CancelledError(
315+
"Cancelled by cancel scope"
316+
)
317+
318+
# Expect ConnectionError due to CancelledError
319+
with pytest.raises(
320+
ConnectionError, match="MCP session creation cancelled"
321+
):
322+
await manager.create_session()
323+
324+
# Verify session was not added to pool
325+
assert not manager._sessions
326+
# Verify cleanup was called
327+
mock_exit_stack.aclose.assert_called_once()
328+
293329
@pytest.mark.asyncio
294330
async def test_close_success(self):
295331
"""Test successful cleanup of all sessions."""

0 commit comments

Comments
 (0)