diff --git a/CHANGELOG.md b/CHANGELOG.md index 08f0b30..e66d5c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,8 @@ adheres to [Semantic Versioning](https://semver.org/). `ToolError` with a generic "Error executing tool ". MCPg's own errors (`MCPgError` subclasses, e.g. an unconfigured NL→SQL provider that names the env var to set) are re-raised as `ToolError` by the call wrapper, and the audit log records their text; the heavy-diagnostics gate raises `ToolError`. Unexpected exceptions keep the SDK's - generic message. + generic message. The wrapper reads the `MCPgError` from the exception's `__cause__` or, failing that, its `__context__` + (CodeRabbit on #356). ## [0.8.3] - 2026-10-14 diff --git a/src/mcpg/server.py b/src/mcpg/server.py index c3f9512..a164e16 100644 --- a/src/mcpg/server.py +++ b/src/mcpg/server.py @@ -211,7 +211,7 @@ async def call_tool( # MCPgError messages are written for the client ("provider # 'openai' is not configured ...; set OPENAI_API_KEY"), so they # go back as a ToolError, and the audit log gets the real text. - domain_error = exc if isinstance(exc, MCPgError) else exc.__cause__ + domain_error = exc if isinstance(exc, MCPgError) else (exc.__cause__ or exc.__context__) if isinstance(domain_error, MCPgError): tool_exc = ToolError(str(domain_error)) audit.record(audit.AuditEvent(tool=name, arguments=arguments, status="error", error=str(tool_exc))) diff --git a/tests/unit/test_server.py b/tests/unit/test_server.py index 27a780d..53d4edb 100644 --- a/tests/unit/test_server.py +++ b/tests/unit/test_server.py @@ -511,3 +511,34 @@ def test_dynamic_session_intent_middleware_absent_by_default() -> None: server = create_server(settings) assert not any(isinstance(m, DynamicSessionIntentMiddleware) for m in server._lowlevel_server.middleware) + + +async def test_call_tool_surfaces_mcpg_error_found_only_in_exception_context() -> None: + """An MCPgError reached through ``__context__`` (an implicit raise inside an + except block, no ``from``) is surfaced like one reached through ``__cause__``: + the client gets its message as a ToolError, not the SDK's generic text.""" + from mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError + + from mcpg.errors import MCPgError + + server = create_server(_SETTINGS) + + async def boom(self: Any, name: str, arguments: dict[str, Any], context: Any = None) -> Any: + try: + raise MCPgError("provider 'x' is not configured; set X_API_KEY") + except MCPgError: + raise UnexpectedToolError(f"Error executing tool {name}") # noqa: B904 - context only + + events: list[Any] = [] + with ( + patch.object(MCPServer, "call_tool", boom), + patch.object(server_mod.audit, "record", side_effect=events.append), + pytest.raises(ToolError) as exc_info, + ): + await server.call_tool("some_tool", {}) + assert "is not configured" in str(exc_info.value) + assert "Error executing tool" not in str(exc_info.value) + # the audit record carries the domain message, not the SDK's generic text + assert [e.status for e in events] == ["error"] + assert "is not configured" in events[0].error + assert "Error executing tool" not in events[0].error