From a7f52ab12a8937a14117b2dc15eeaaad887cc409 Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Tue, 18 Aug 2026 05:12:43 +0800 Subject: [PATCH 1/3] Python: fix(mcp): name the real error when a cancel scope masks MCP init failures When the MCP client stack cancels (e.g. an HTTP 401 from the server during session creation), anyio surfaces a bare CancelledError and the wrap sites reported 'Cancelled via cancel scope ...' as the failure. Unwrap single-member exception groups and __cause__/__context__ chains when composing the ToolException message so the real error is named. Genuine caller-driven cancellations still propagate unchanged and bare cancellations keep their own message. Fixes #7699 --- python/packages/core/agent_framework/_mcp.py | 35 +++++++++++-- python/packages/core/tests/core/test_mcp.py | 53 ++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 54cdaa1d87..43c626cb45 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -418,6 +418,31 @@ def _should_propagate_cancelled_error(ex: BaseException) -> bool: return task is not None and task.cancelling() > 0 +def _describe_error(ex: BaseException) -> str: + """Return the most specific message in *ex*'s chain, unmasking cancel scopes. + + anyio cancel scopes and task groups surface internal failures as a bare + ``CancelledError`` ("Cancelled via cancel scope ...") or a single-member + ``ExceptionGroup``; the real error (e.g. an HTTP 401 from the MCP server) + sits in ``__cause__``/``__context__`` or the group's single leaf. Follow + those links so the reported message names the actual failure. A genuine + bare cancellation keeps its own message. + """ + current = ex + for _ in range(10): # pathological chains only; normally 0-2 hops + inner = getattr(current, "exceptions", None) # ExceptionGroup leaf + if inner is not None and len(inner) == 1: + current = inner[0] + continue + cause = current.__cause__ or current.__context__ + if cause is None or cause is current: + break + if isinstance(current, asyncio.CancelledError) and isinstance(cause, asyncio.CancelledError): + break # cancel-of-cancel carries no information + current = cause + return str(current) or repr(current) + + # region: MCP Plugin @@ -1336,9 +1361,9 @@ async def _connect_on_owner(self, *, reset: bool = False, load_configured: bool raise command = getattr(self, "command", None) if command: - error_msg = f"Failed to start MCP server '{command}': {ex}" + error_msg = f"Failed to start MCP server '{command}': {_describe_error(ex)}" else: - error_msg = f"Failed to connect to MCP server: {ex}" + error_msg = f"Failed to connect to MCP server: {_describe_error(ex)}" # CancelledError is a BaseException (not Exception) on Python >= 3.8, so # inner_exception=None and ToolException.__init__ won't log exc_info. if isinstance(ex, asyncio.CancelledError): @@ -1376,7 +1401,7 @@ async def _connect_on_owner(self, *, reset: bool = False, load_configured: bool except (Exception, asyncio.CancelledError) as ex: if await self._close_and_check_cancelled(ex): raise - session_error_msg = f"Failed to create MCP session: {ex}" + session_error_msg = f"Failed to create MCP session: {_describe_error(ex)}" if isinstance(ex, asyncio.CancelledError): logger.debug(session_error_msg, exc_info=True) raise ToolException( @@ -1396,9 +1421,9 @@ async def _connect_on_owner(self, *, reset: bool = False, load_configured: bool if command: args_str = " ".join(getattr(self, "args", [])) full_command = f"{command} {args_str}".strip() - error_msg = f"MCP server '{full_command}' failed to initialize: {ex}" + error_msg = f"MCP server '{full_command}' failed to initialize: {_describe_error(ex)}" else: - error_msg = f"MCP server failed to initialize: {ex}" + error_msg = f"MCP server failed to initialize: {_describe_error(ex)}" if isinstance(ex, asyncio.CancelledError): logger.debug(error_msg, exc_info=True) raise ToolException(error_msg, inner_exception=ex if isinstance(ex, Exception) else None) from ex diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 4c27e596f8..1001f3d073 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -32,6 +32,7 @@ from agent_framework._mcp import ( MCPTool, _build_prefixed_mcp_name, + _describe_error, _get_input_model_from_mcp_prompt, _normalize_additional_tool_argument_names, _normalize_mcp_name, @@ -3736,6 +3737,58 @@ async def test_connect_cancelled_error_during_session_creation_includes_exceptio assert "cancel scope detail" in str(exc_info.value) +# Tests for _describe_error helper (cancel-scope / exception-group unmasking) + + +def test_describe_error_keeps_plain_exception_message(): + assert _describe_error(RuntimeError("boom")) == "boom" + + +@pytest.mark.skipif(sys.version_info < (3, 11), reason="ExceptionGroup is Python >= 3.11") +def test_describe_error_unwraps_single_member_exception_group(): + real = RuntimeError("401 Client Error: Unauthorized") + group = ExceptionGroup("unhandled errors in a TaskGroup", [real]) # noqa: F821 -- gated to 3.11+ by the skipif above + assert _describe_error(group) == "401 Client Error: Unauthorized" + + +def test_describe_error_unmasks_cancel_scope_via_context(): + real = RuntimeError("401 Client Error: Unauthorized") + masked = asyncio.CancelledError("Cancelled via cancel scope") + masked.__context__ = real + assert _describe_error(masked) == "401 Client Error: Unauthorized" + + +def test_describe_error_keeps_bare_cancellation(): + assert _describe_error(asyncio.CancelledError("Cancelled via cancel scope")) == "Cancelled via cancel scope" + + +async def test_connect_cancelled_error_unmasks_inner_auth_failure(): + """A 401 swallowed by the MCP client's cancel scope must be named in the ToolException.""" + tool = MCPStreamableHTTPTool(name="test", url="http://example.com") + + mock_transport = (Mock(), Mock()) + mock_context_manager = Mock() + mock_context_manager.__aenter__ = AsyncMock(return_value=mock_transport) + mock_context_manager.__aexit__ = AsyncMock(return_value=None) + tool.get_mcp_client = Mock(return_value=mock_context_manager) # type: ignore[method-assign] + + real = RuntimeError("401 Client Error: Unauthorized") + masked = asyncio.CancelledError("Cancelled via cancel scope") + masked.__context__ = real + + with patch("mcp.client.session.ClientSession") as mock_session_class: + mock_session_class.return_value.__aenter__ = AsyncMock(side_effect=masked) + mock_session_class.return_value.__aexit__ = AsyncMock(return_value=None) + + with pytest.raises(ToolException) as exc_info: + await tool.connect() + + message = str(exc_info.value) + assert "Failed to create MCP session" in message + assert "401 Client Error: Unauthorized" in message + assert "Cancelled via cancel scope" not in message + + async def test_connect_cancelled_error_during_session_creation_logs_with_exc_info(): """Test that CancelledError from session creation is logged with exc_info=True.""" tool = MCPStreamableHTTPTool(name="test", url="http://example.com") From 9d6c1f98cbbaa9fc49af59ec096833e5483c9eaf Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Wed, 19 Aug 2026 17:08:51 +0800 Subject: [PATCH 2/3] Python: fix: name the close-time failure when MCP initialize cancels bare --- python/packages/core/agent_framework/_mcp.py | 85 ++++++++++++++------ python/packages/core/tests/core/test_mcp.py | 33 ++++++++ 2 files changed, 94 insertions(+), 24 deletions(-) diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 43c626cb45..45ec704e75 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -418,10 +418,13 @@ def _should_propagate_cancelled_error(ex: BaseException) -> bool: return task is not None and task.cancelling() > 0 +_MAX_ERROR_UNWRAP_HOPS = 10 # pathological chains only; normally 0-2 hops + + def _describe_error(ex: BaseException) -> str: """Return the most specific message in *ex*'s chain, unmasking cancel scopes. - anyio cancel scopes and task groups surface internal failures as a bare + AnyIO cancel scopes and task groups surface internal failures as a bare ``CancelledError`` ("Cancelled via cancel scope ...") or a single-member ``ExceptionGroup``; the real error (e.g. an HTTP 401 from the MCP server) sits in ``__cause__``/``__context__`` or the group's single leaf. Follow @@ -429,8 +432,14 @@ def _describe_error(ex: BaseException) -> str: bare cancellation keeps its own message. """ current = ex - for _ in range(10): # pathological chains only; normally 0-2 hops - inner = getattr(current, "exceptions", None) # ExceptionGroup leaf + for _ in range(_MAX_ERROR_UNWRAP_HOPS): + # Group membership is gated on the type name (not the attribute) so a + # non-group exception that happens to carry .exceptions is not unwrapped. + inner = ( + getattr(current, "exceptions", None) + if type(current).__name__ in ("ExceptionGroup", "BaseExceptionGroup") + else None + ) if inner is not None and len(inner) == 1: current = inner[0] continue @@ -443,6 +452,18 @@ def _describe_error(ex: BaseException) -> str: return str(current) or repr(current) +def _describe_with_cleanup(ex: BaseException, cleanup_error: BaseException | None) -> str: + """Describe *ex*, preferring the cleanup failure when *ex* is a bare cancellation. + + A bare CancelledError from the task group carries no detail of its own; the + real failure (e.g. an HTTP 401 from the MCP server) can surface only when + the exit stack is closed, so the close's leaf is the better message then. + """ + if isinstance(ex, asyncio.CancelledError) and cleanup_error is not None: + return _describe_error(cleanup_error) + return _describe_error(ex) + + # region: MCP Plugin @@ -1261,8 +1282,13 @@ async def _run_on_lifecycle_owner( await queue.put((action, reset, load_configured, future)) await future - async def _safe_close_exit_stack(self) -> None: - """Safely close the exit stack, handling unexpected cleanup failures.""" + async def _safe_close_exit_stack(self) -> BaseException | None: + """Safely close the exit stack, handling unexpected cleanup failures. + + Returns the swallowed cleanup failure, if any. A caller that only saw a + bare cancellation from the task group can still name the real cause the + close surfaced (e.g. the HTTP status error of a rejected handshake). + """ try: await self._exit_stack.aclose() except RuntimeError as e: @@ -1273,26 +1299,33 @@ async def _safe_close_exit_stack(self) -> None: "This indicates MCP lifecycle ownership was lost. Error: %s", e, ) - else: - raise - except asyncio.CancelledError: + return e + raise + except asyncio.CancelledError as e: logger.warning("Could not cleanly close MCP exit stack because the lifecycle owner task was cancelled.") + return e except Exception as e: if type(e).__name__ == "ExceptionGroup": logger.warning("Could not cleanly close MCP exit stack due to cleanup error group. Error: %s", e) - else: - raise + return e + raise + return None - async def _close_and_check_cancelled(self, ex: BaseException) -> bool: - """Close the exit stack and return True if *ex* is a genuine task cancellation. + async def _close_and_check_cancelled(self, ex: BaseException) -> tuple[bool, BaseException | None]: + """Close the exit stack and report whether *ex* is a genuine task cancellation. - Callers should immediately re-raise when this returns True:: + Returns ``(should_reraise, cleanup_error)``. Callers should immediately + re-raise when the first element is True:: - if await self._close_and_check_cancelled(ex): + cancelled, cleanup_error = await self._close_and_check_cancelled(ex) + if cancelled: raise + + The second element carries the failure swallowed while closing, so an + error path holding only a bare cancellation can still describe it. """ - await self._safe_close_exit_stack() - return _should_propagate_cancelled_error(ex) + cleanup_error = await self._safe_close_exit_stack() + return _should_propagate_cancelled_error(ex), cleanup_error def _reset_session_state(self) -> None: self._server_capabilities = None @@ -1357,13 +1390,14 @@ async def _connect_on_owner(self, *, reset: bool = False, load_configured: bool # instead of wrapping it in ToolException. On Python < 3.11, task.cancelling() # is unavailable so MCP-internal CancelledErrors cannot be distinguished from # caller-driven cancellation; they are wrapped as ToolException in that case. - if await self._close_and_check_cancelled(ex): + cancelled, cleanup_error = await self._close_and_check_cancelled(ex) + if cancelled: raise command = getattr(self, "command", None) if command: - error_msg = f"Failed to start MCP server '{command}': {_describe_error(ex)}" + error_msg = f"Failed to start MCP server '{command}': {_describe_with_cleanup(ex, cleanup_error)}" else: - error_msg = f"Failed to connect to MCP server: {_describe_error(ex)}" + error_msg = f"Failed to connect to MCP server: {_describe_with_cleanup(ex, cleanup_error)}" # CancelledError is a BaseException (not Exception) on Python >= 3.8, so # inner_exception=None and ToolException.__init__ won't log exc_info. if isinstance(ex, asyncio.CancelledError): @@ -1399,9 +1433,10 @@ async def _connect_on_owner(self, *, reset: bool = False, load_configured: bool ) ) except (Exception, asyncio.CancelledError) as ex: - if await self._close_and_check_cancelled(ex): + cancelled, cleanup_error = await self._close_and_check_cancelled(ex) + if cancelled: raise - session_error_msg = f"Failed to create MCP session: {_describe_error(ex)}" + session_error_msg = f"Failed to create MCP session: {_describe_with_cleanup(ex, cleanup_error)}" if isinstance(ex, asyncio.CancelledError): logger.debug(session_error_msg, exc_info=True) raise ToolException( @@ -1414,16 +1449,18 @@ async def _connect_on_owner(self, *, reset: bool = False, load_configured: bool init_span.set_attribute(OtelAttr.MCP_PROTOCOL_VERSION, initialize_result.protocolVersion) self._set_server_capabilities(getattr(initialize_result, "capabilities", None)) except (Exception, asyncio.CancelledError) as ex: - if await self._close_and_check_cancelled(ex): + cancelled, cleanup_error = await self._close_and_check_cancelled(ex) + if cancelled: raise # Provide context about initialization failure command = getattr(self, "command", None) if command: args_str = " ".join(getattr(self, "args", [])) full_command = f"{command} {args_str}".strip() - error_msg = f"MCP server '{full_command}' failed to initialize: {_describe_error(ex)}" + described = _describe_with_cleanup(ex, cleanup_error) + error_msg = f"MCP server '{full_command}' failed to initialize: {described}" else: - error_msg = f"MCP server failed to initialize: {_describe_error(ex)}" + error_msg = f"MCP server failed to initialize: {_describe_with_cleanup(ex, cleanup_error)}" if isinstance(ex, asyncio.CancelledError): logger.debug(error_msg, exc_info=True) raise ToolException(error_msg, inner_exception=ex if isinstance(ex, Exception) else None) from ex diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 1001f3d073..4491b28ef7 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -3789,6 +3789,39 @@ async def test_connect_cancelled_error_unmasks_inner_auth_failure(): assert "Cancelled via cancel scope" not in message +@pytest.mark.skipif(sys.version_info < (3, 11), reason="ExceptionGroup is Python >= 3.11") +async def test_connect_bare_cancel_names_cleanup_error_from_exit_stack(): + """The reported 401 path: initialize() raises a bare CancelledError and the + real HTTP failure only surfaces from the exit-stack close. The ToolException + must name that close-time error, not the cancellation.""" + tool = MCPStreamableHTTPTool(name="test", url="http://example.com") + + mock_transport = (Mock(), Mock()) + mock_context_manager = Mock() + mock_context_manager.__aenter__ = AsyncMock(return_value=mock_transport) + mock_context_manager.__aexit__ = AsyncMock(return_value=None) + tool.get_mcp_client = Mock(return_value=mock_context_manager) # type: ignore[method-assign] + + cleanup_group = ExceptionGroup( # noqa: F821 -- gated to 3.11+ by the skipif above + "unhandled errors in a TaskGroup", [RuntimeError("401 Client Error: Unauthorized")] + ) + + mock_session = Mock() + mock_session.initialize = AsyncMock(side_effect=asyncio.CancelledError("Cancelled via cancel scope")) + + with patch("mcp.client.session.ClientSession") as mock_session_class: + mock_session_class.return_value.__aenter__ = AsyncMock(return_value=mock_session) + mock_session_class.return_value.__aexit__ = AsyncMock(side_effect=cleanup_group) + + with pytest.raises(ToolException) as exc_info: + await tool.connect() + + message = str(exc_info.value) + assert "MCP server failed to initialize" in message + assert "401 Client Error: Unauthorized" in message + assert "Cancelled via cancel scope" not in message + + async def test_connect_cancelled_error_during_session_creation_logs_with_exc_info(): """Test that CancelledError from session creation is logged with exc_info=True.""" tool = MCPStreamableHTTPTool(name="test", url="http://example.com") From 06987cba62c9f0817297a61303dd8f45cab68cff Mon Sep 17 00:00:00 2001 From: Yufeng He <40085740+he-yufeng@users.noreply.github.com> Date: Fri, 21 Aug 2026 20:31:25 +0800 Subject: [PATCH 3/3] Python: test: resolve ExceptionGroup compatibly --- python/packages/core/tests/core/test_mcp.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 4491b28ef7..256cab1cc6 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -3746,8 +3746,14 @@ def test_describe_error_keeps_plain_exception_message(): @pytest.mark.skipif(sys.version_info < (3, 11), reason="ExceptionGroup is Python >= 3.11") def test_describe_error_unwraps_single_member_exception_group(): + import builtins + + exception_group_type = getattr(builtins, "ExceptionGroup", None) + if exception_group_type is None: + pytest.skip("ExceptionGroup is not available on this Python version") + real = RuntimeError("401 Client Error: Unauthorized") - group = ExceptionGroup("unhandled errors in a TaskGroup", [real]) # noqa: F821 -- gated to 3.11+ by the skipif above + group = exception_group_type("unhandled errors in a TaskGroup", [real]) assert _describe_error(group) == "401 Client Error: Unauthorized" @@ -3794,6 +3800,12 @@ async def test_connect_bare_cancel_names_cleanup_error_from_exit_stack(): """The reported 401 path: initialize() raises a bare CancelledError and the real HTTP failure only surfaces from the exit-stack close. The ToolException must name that close-time error, not the cancellation.""" + import builtins + + exception_group_type = getattr(builtins, "ExceptionGroup", None) + if exception_group_type is None: + pytest.skip("ExceptionGroup is not available on this Python version") + tool = MCPStreamableHTTPTool(name="test", url="http://example.com") mock_transport = (Mock(), Mock()) @@ -3802,7 +3814,7 @@ async def test_connect_bare_cancel_names_cleanup_error_from_exit_stack(): mock_context_manager.__aexit__ = AsyncMock(return_value=None) tool.get_mcp_client = Mock(return_value=mock_context_manager) # type: ignore[method-assign] - cleanup_group = ExceptionGroup( # noqa: F821 -- gated to 3.11+ by the skipif above + cleanup_group = exception_group_type( "unhandled errors in a TaskGroup", [RuntimeError("401 Client Error: Unauthorized")] )