From f9b4b8af3410e13ba002129e25c3f212568ee031 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Mon, 4 May 2026 07:56:05 +0800 Subject: [PATCH] fix(mcp): include exception type in error messages when str(exc) is empty Some exception classes (e.g. anyio.ClosedResourceError) are raised without a message argument, so str(exc) returns an empty string. The existing error format f'{type(exc).__name__}: {exc}' would produce messages like 'MCP call failed: ClosedResourceError: ' with nothing after the colon. Add _exc_str() helper that falls back to repr(exc) when str(exc) is empty, and apply it to all 6 MCP error formatting sites (5 tool/prompt/resource handlers + 1 sampling handler). Fixes #19417 --- tests/tools/test_mcp_empty_error_message.py | 89 +++++++++++++++++++++ tools/mcp_tool.py | 24 ++++-- 2 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 tests/tools/test_mcp_empty_error_message.py diff --git a/tests/tools/test_mcp_empty_error_message.py b/tests/tools/test_mcp_empty_error_message.py new file mode 100644 index 0000000000..6c04089f67 --- /dev/null +++ b/tests/tools/test_mcp_empty_error_message.py @@ -0,0 +1,89 @@ +"""Regression tests for MCP error messages when str(exc) is empty. + +Issue #19417: ClosedResourceError (and similar exceptions raised without a +message argument) produced ``MCP call failed: ClosedResourceError: `` with +nothing after the colon, making debugging impossible. + +Fix: ``_exc_str()`` falls back to ``repr(exc)`` when ``str(exc)`` is empty. +""" + +import json +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from tools.mcp_tool import _exc_str, _sanitize_error + + +# --------------------------------------------------------------------------- +# _exc_str unit tests +# --------------------------------------------------------------------------- + + +class _EmptyMessageError(Exception): + """Exception whose __str__ returns empty string (like anyio.ClosedResourceError).""" + + def __str__(self): + return "" + + +class _NormalError(Exception): + pass + + +def test_exc_str_returns_str_when_nonempty(): + exc = _NormalError("something broke") + assert _exc_str(exc) == "something broke" + + +def test_exc_str_falls_back_to_repr_when_str_empty(): + exc = _EmptyMessageError() + result = _exc_str(exc) + assert result != "" + assert "_EmptyMessageError" in result + + +def test_exc_str_falls_back_to_repr_for_whitespace_only(): + """str(exc) that is only whitespace should also trigger the repr fallback.""" + exc = Exception(" ") + result = _exc_str(exc) + # After strip(), the text is empty, so repr is used + assert result.strip() != "" + + +def test_exc_str_handles_closedresource_like_exception(): + """Simulate anyio.ClosedResourceError which has no message.""" + # Replicate the real anyio.ClosedResourceError behavior + exc = type("ClosedResourceError", (Exception,), {"__str__": lambda self: ""})() + result = _exc_str(exc) + assert "ClosedResourceError" in result + assert result != "" + + +# --------------------------------------------------------------------------- +# Integration: error message format in _sanitize_error +# --------------------------------------------------------------------------- + + +def test_error_message_not_empty_when_exc_has_no_message(): + """The formatted error string should always contain the exception class name.""" + exc = _EmptyMessageError() + error_msg = _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + ) + assert "ClosedResourceError" not in error_msg or "_EmptyMessageError" in error_msg + # The key invariant: the message must not end with ": " + assert not error_msg.endswith(": ") + # And it must contain the exception type name + assert "_EmptyMessageError" in error_msg + + +def test_error_message_preserves_normal_exception_text(): + """Normal exceptions should still show their message text.""" + exc = _NormalError("connection refused") + error_msg = _sanitize_error( + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" + ) + assert "connection refused" in error_msg + assert "_NormalError" in error_msg diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 5c45f1c4f2..e1c8ef393e 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -312,6 +312,18 @@ def _sanitize_error(text: str) -> str: return _CREDENTIAL_PATTERN.sub("[REDACTED]", text) +def _exc_str(exc: BaseException) -> str: + """Return a non-empty human-readable string for *exc*. + + Some exception classes (e.g. ``anyio.ClosedResourceError``) are raised + without a message argument, so ``str(exc)`` is ``""``. This helper + falls back to ``repr(exc)`` so that error messages shown to the user + and logged to disk always carry *some* diagnostic information. + """ + text = str(exc).strip() + return text if text else repr(exc) + + # --------------------------------------------------------------------------- # MCP tool description content scanning # --------------------------------------------------------------------------- @@ -831,7 +843,7 @@ class SamplingHandler: except Exception as exc: self.metrics["errors"] += 1 return self._error( - f"Sampling LLM call failed: {_sanitize_error(str(exc))}" + f"Sampling LLM call failed: {_sanitize_error(_exc_str(exc))}" ) # Guard against empty choices (content filtering, provider errors) @@ -2174,7 +2186,7 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): ) return json.dumps({ "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {exc}" + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" ) }, ensure_ascii=False) @@ -2232,7 +2244,7 @@ def _make_list_resources_handler(server_name: str, tool_timeout: float): ) return json.dumps({ "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {exc}" + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" ) }, ensure_ascii=False) @@ -2292,7 +2304,7 @@ def _make_read_resource_handler(server_name: str, tool_timeout: float): ) return json.dumps({ "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {exc}" + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" ) }, ensure_ascii=False) @@ -2355,7 +2367,7 @@ def _make_list_prompts_handler(server_name: str, tool_timeout: float): ) return json.dumps({ "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {exc}" + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" ) }, ensure_ascii=False) @@ -2426,7 +2438,7 @@ def _make_get_prompt_handler(server_name: str, tool_timeout: float): ) return json.dumps({ "error": _sanitize_error( - f"MCP call failed: {type(exc).__name__}: {exc}" + f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}" ) }, ensure_ascii=False)