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
This commit is contained in:
liuhao1024 2026-05-04 07:56:05 +08:00 committed by Teknium
parent f481395d4c
commit f9b4b8af34
2 changed files with 107 additions and 6 deletions

View file

@ -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

View file

@ -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)