Skip to content

Commit 1fb4a70

Browse files
committed
fix(server): hide unexpected tool exception details
1 parent cf110e3 commit 1fb4a70

4 files changed

Lines changed: 39 additions & 13 deletions

File tree

src/mcp/server/mcpserver/tools/base.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,20 @@
44
from functools import cached_property
55
from typing import TYPE_CHECKING, Any
66

7+
import pydantic_core
78
from pydantic import BaseModel, Field
89

910
from mcp.server.mcpserver.exceptions import ToolError
1011
from mcp.server.mcpserver.utilities.context_injection import find_context_parameter
1112
from mcp.server.mcpserver.utilities.func_metadata import FuncMetadata, func_metadata
13+
from mcp.server.mcpserver.utilities.logging import get_logger
1214
from mcp.shared._callable_inspection import is_async_callable
1315
from mcp.shared.exceptions import UrlElicitationRequiredError
1416
from mcp.shared.tool_name_validation import validate_and_warn_tool_name
1517
from mcp.types import Icon, ToolAnnotations
1618

19+
logger = get_logger(__name__)
20+
1721
if TYPE_CHECKING:
1822
from mcp.server.context import LifespanContextT, RequestT
1923
from mcp.server.mcpserver.context import Context
@@ -115,5 +119,10 @@ async def run(
115119
# Re-raise UrlElicitationRequiredError so it can be properly handled
116120
# as an MCP error response with code -32042
117121
raise
118-
except Exception as e:
122+
except ToolError:
123+
raise
124+
except pydantic_core.ValidationError as e:
119125
raise ToolError(f"Error executing tool {self.name}: {e}") from e
126+
except Exception as e:
127+
logger.exception("Unexpected error executing tool %s", self.name)
128+
raise ToolError(f"Error executing tool {self.name}: unexpected internal error") from e

tests/interaction/_requirements.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,8 @@ def __post_init__(self) -> None:
677677
"mcpserver:tool:handler-throws": Requirement(
678678
source="sdk",
679679
behavior=(
680-
"An exception raised by a tool function (ToolError or otherwise) is caught and returned as a "
681-
"tool result with isError true and the failure text in content; it does not become a JSON-RPC error."
680+
"An exception raised by a tool function is caught and returned as a tool result with isError true; "
681+
"explicit ToolError messages are returned to the client, while unexpected exception details are hidden."
682682
),
683683
),
684684
"mcpserver:tool:input-validation": Requirement(

tests/interaction/mcpserver/test_tools.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,22 @@ async def test_call_tool_function_exception_becomes_error_result(connect: Connec
8181
8282
The function's `-> str` annotation gives the tool a derived output schema, but the error
8383
result is built before any schema validation runs, so no validation failure is layered on
84-
top of the original exception.
84+
top of the original exception. Unexpected exception details are logged server-side and
85+
hidden from the client-facing result.
8586
"""
8687
mcp = MCPServer("errors")
8788

8889
@mcp.tool()
8990
def explode() -> str:
90-
raise ValueError("boom")
91+
raise ValueError("secret token leaked")
9192

9293
async with connect(mcp) as client:
9394
result = await client.call_tool("explode", {})
9495

9596
assert result == snapshot(
96-
CallToolResult(content=[TextContent(text="Error executing tool explode: boom")], is_error=True)
97+
CallToolResult(
98+
content=[TextContent(text="Error executing tool explode: unexpected internal error")], is_error=True
99+
)
97100
)
98101

99102

@@ -109,9 +112,7 @@ def flux() -> str:
109112
async with connect(mcp) as client:
110113
result = await client.call_tool("flux", {})
111114

112-
assert result == snapshot(
113-
CallToolResult(content=[TextContent(text="Error executing tool flux: flux capacitor offline")], is_error=True)
114-
)
115+
assert result == snapshot(CallToolResult(content=[TextContent(text="flux capacitor offline")], is_error=True))
115116

116117

117118
@requirement("mcpserver:tool:unknown-name")

tests/server/mcpserver/test_tool_manager.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,18 +383,34 @@ async def async_tool(x: int, ctx: Context) -> str:
383383
assert result == "42"
384384

385385
@pytest.mark.anyio
386-
async def test_context_error_handling(self):
387-
"""Test error handling when context injection fails."""
386+
async def test_unexpected_error_hides_internal_details(self):
387+
"""Test error handling does not expose unexpected exception details."""
388388

389389
def tool_with_context(x: int, ctx: Context) -> str:
390-
raise ValueError("Test error")
390+
raise ValueError("secret token leaked")
391391

392392
manager = ToolManager()
393393
manager.add_tool(tool_with_context)
394394

395-
with pytest.raises(ToolError, match="Error executing tool tool_with_context"):
395+
with pytest.raises(ToolError) as exc_info:
396396
await manager.call_tool("tool_with_context", {"x": 42}, context=Context())
397397

398+
assert str(exc_info.value) == "Error executing tool tool_with_context: unexpected internal error"
399+
assert "secret token leaked" not in str(exc_info.value)
400+
401+
@pytest.mark.anyio
402+
async def test_tool_error_preserves_message(self):
403+
"""Test explicit ToolError messages remain visible to clients."""
404+
405+
def tool_with_expected_error() -> str:
406+
raise ToolError("safe client-facing error")
407+
408+
manager = ToolManager()
409+
manager.add_tool(tool_with_expected_error)
410+
411+
with pytest.raises(ToolError, match="safe client-facing error"):
412+
await manager.call_tool("tool_with_expected_error", {}, context=Context())
413+
398414

399415
class TestToolAnnotations:
400416
def test_tool_annotations(self):

0 commit comments

Comments
 (0)