From 69eda4055f5de137e3f8154e27fe3d23b8c2b676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=89=E7=94=B0=20=E8=A3=95=E7=B4=80?= Date: Fri, 5 Dec 2025 20:05:11 +0900 Subject: [PATCH 1/2] 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 --- .../adk/tools/mcp_tool/mcp_session_manager.py | 19 ++++++++++ .../mcp_tool/test_mcp_session_manager.py | 36 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/google/adk/tools/mcp_tool/mcp_session_manager.py b/src/google/adk/tools/mcp_tool/mcp_session_manager.py index c9c4c2ae66..18e90d7141 100644 --- a/src/google/adk/tools/mcp_tool/mcp_session_manager.py +++ b/src/google/adk/tools/mcp_tool/mcp_session_manager.py @@ -363,6 +363,25 @@ async def create_session( logger.debug('Created new session: %s', session_key) return session + except asyncio.CancelledError as e: + # CancelledError can occur when the 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+, it must be caught explicitly. + logger.warning( + 'MCP session creation cancelled (likely due to HTTP error): %s', e + ) + try: + await exit_stack.aclose() + except Exception as exit_stack_error: + logger.warning( + 'Error during cancelled session cleanup: %s', exit_stack_error + ) + raise ConnectionError( + f'MCP session creation cancelled (server may have returned HTTP' + f' error): {e}' + ) from e + except Exception as e: # If session creation fails, clean up the exit stack if exit_stack: diff --git a/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py b/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py index 74eabe9d4d..862df0af1f 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py @@ -290,6 +290,42 @@ async def test_create_session_timeout( # Verify cleanup was called mock_exit_stack.aclose.assert_called_once() + @pytest.mark.asyncio + @patch("google.adk.tools.mcp_tool.mcp_session_manager.stdio_client") + @patch("google.adk.tools.mcp_tool.mcp_session_manager.AsyncExitStack") + async def test_create_session_cancelled_error( + self, mock_exit_stack_class, mock_stdio + ): + """Test session creation when CancelledError is raised (e.g., HTTP 403). + + When an MCP server returns an HTTP error (e.g., 401, 403), the MCP SDK + uses anyio cancel scopes internally, which raise CancelledError. This + test verifies that CancelledError is caught and converted to a + ConnectionError with proper cleanup. + """ + manager = MCPSessionManager(self.mock_stdio_connection_params) + + mock_exit_stack = MockAsyncExitStack() + + mock_exit_stack_class.return_value = mock_exit_stack + mock_stdio.return_value = AsyncMock() + + # Simulate CancelledError during session creation (e.g., HTTP 403) + mock_exit_stack.enter_async_context.side_effect = asyncio.CancelledError( + "Cancelled by cancel scope" + ) + + # Expect ConnectionError due to CancelledError + with pytest.raises( + ConnectionError, match="MCP session creation cancelled" + ): + await manager.create_session() + + # Verify session was not added to pool + assert not manager._sessions + # Verify cleanup was called + mock_exit_stack.aclose.assert_called_once() + @pytest.mark.asyncio async def test_close_success(self): """Test successful cleanup of all sessions.""" From 3f23d33b835346eaa6c173fe887c5ca486878376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9C=89=E7=94=B0=20=E8=A3=95=E7=B4=80?= Date: Thu, 11 Dec 2025 05:48:32 +0900 Subject: [PATCH 2/2] style(mcp): apply autoformat to fix lint errors - Format mcp_session_manager.py (remove unnecessary f-string prefix) - Format test_mcp_session_manager.py (consolidate pytest.raises call) --- src/google/adk/tools/mcp_tool/mcp_session_manager.py | 2 +- tests/unittests/tools/mcp_tool/test_mcp_session_manager.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/google/adk/tools/mcp_tool/mcp_session_manager.py b/src/google/adk/tools/mcp_tool/mcp_session_manager.py index 18e90d7141..3238e55432 100644 --- a/src/google/adk/tools/mcp_tool/mcp_session_manager.py +++ b/src/google/adk/tools/mcp_tool/mcp_session_manager.py @@ -378,7 +378,7 @@ async def create_session( 'Error during cancelled session cleanup: %s', exit_stack_error ) raise ConnectionError( - f'MCP session creation cancelled (server may have returned HTTP' + 'MCP session creation cancelled (server may have returned HTTP' f' error): {e}' ) from e diff --git a/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py b/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py index 862df0af1f..1036945f0e 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py @@ -316,9 +316,7 @@ async def test_create_session_cancelled_error( ) # Expect ConnectionError due to CancelledError - with pytest.raises( - ConnectionError, match="MCP session creation cancelled" - ): + with pytest.raises(ConnectionError, match="MCP session creation cancelled"): await manager.create_session() # Verify session was not added to pool