fix(server): hide unexpected tool exception details

Author: go165

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

@@ -4,16 +4,20 @@
 from functools import cached_property
 from typing import TYPE_CHECKING, Any
 
+import pydantic_core
 from pydantic import BaseModel, Field
 
 from mcp.server.mcpserver.exceptions import ToolError
 from mcp.server.mcpserver.utilities.context_injection import find_context_parameter
 from mcp.server.mcpserver.utilities.func_metadata import FuncMetadata, func_metadata
+from mcp.server.mcpserver.utilities.logging import get_logger
 from mcp.shared._callable_inspection import is_async_callable
 from mcp.shared.exceptions import UrlElicitationRequiredError
 from mcp.shared.tool_name_validation import validate_and_warn_tool_name
 from mcp.types import Icon, ToolAnnotations
 
+logger = get_logger(__name__)
+
 if TYPE_CHECKING:
     from mcp.server.context import LifespanContextT, RequestT
     from mcp.server.mcpserver.context import Context
@@ -115,5 +119,10 @@ async def run(
             # Re-raise UrlElicitationRequiredError so it can be properly handled
             # as an MCP error response with code -32042
             raise
-        except Exception as e:
+        except ToolError:
+            raise
+        except pydantic_core.ValidationError as e:
             raise ToolError(f"Error executing tool {self.name}: {e}") from e
+        except Exception as e:
+            logger.exception("Unexpected error executing tool %s", self.name)
+            raise ToolError(f"Error executing tool {self.name}: unexpected internal error") from e

tests/client/test_list_roots_callback.py

@@ -44,4 +44,4 @@ async def test_list_roots(context: Context, message: str):
         result = await client.call_tool("test_list_roots", {"message": "test message"})
         assert result.is_error is True
         assert isinstance(result.content[0], TextContent)
-        assert result.content[0].text == "Error executing tool test_list_roots: List roots not supported"
+        assert result.content[0].text == "Error executing tool test_list_roots: unexpected internal error"

tests/client/test_sampling_callback.py

@@ -54,7 +54,7 @@ async def test_sampling_tool(message: str, ctx: Context) -> bool:
         result = await client.call_tool("test_sampling", {"message": "Test message for sampling"})
         assert result.is_error is True
         assert isinstance(result.content[0], TextContent)
-        assert result.content[0].text == "Error executing tool test_sampling: Sampling not supported"
+        assert result.content[0].text == "Error executing tool test_sampling: unexpected internal error"
 
 
 @pytest.mark.anyio

tests/interaction/_requirements.py

@@ -677,8 +677,8 @@ def __post_init__(self) -> None:
     "mcpserver:tool:handler-throws": Requirement(
         source="sdk",
         behavior=(
-            "An exception raised by a tool function (ToolError or otherwise) is caught and returned as a "
-            "tool result with isError true and the failure text in content; it does not become a JSON-RPC error."
+            "An exception raised by a tool function is caught and returned as a tool result with isError true; "
+            "explicit ToolError messages are returned to the client, while unexpected exception details are hidden."
         ),
     ),
     "mcpserver:tool:input-validation": Requirement(

tests/interaction/mcpserver/test_tools.py

@@ -81,19 +81,22 @@ async def test_call_tool_function_exception_becomes_error_result(connect: Connec
 
     The function's `-> str` annotation gives the tool a derived output schema, but the error
     result is built before any schema validation runs, so no validation failure is layered on
-    top of the original exception.
+    top of the original exception. Unexpected exception details are logged server-side and
+    hidden from the client-facing result.
     """
     mcp = MCPServer("errors")
 
     @mcp.tool()
     def explode() -> str:
-        raise ValueError("boom")
+        raise ValueError("secret token leaked")
 
     async with connect(mcp) as client:
         result = await client.call_tool("explode", {})
 
     assert result == snapshot(
-        CallToolResult(content=[TextContent(text="Error executing tool explode: boom")], is_error=True)
+        CallToolResult(
+            content=[TextContent(text="Error executing tool explode: unexpected internal error")], is_error=True
+        )
     )
 
 
@@ -109,9 +112,7 @@ def flux() -> str:
     async with connect(mcp) as client:
         result = await client.call_tool("flux", {})
 
-    assert result == snapshot(
-        CallToolResult(content=[TextContent(text="Error executing tool flux: flux capacitor offline")], is_error=True)
-    )
+    assert result == snapshot(CallToolResult(content=[TextContent(text="flux capacitor offline")], is_error=True))
 
 
 @requirement("mcpserver:tool:unknown-name")

tests/server/mcpserver/test_server.py

@@ -265,7 +265,8 @@ async def test_tool_exception_handling(self):
             assert len(result.content) == 1
             content = result.content[0]
             assert isinstance(content, TextContent)
-            assert "Test error" in content.text
+            assert content.text == "Error executing tool error_tool_fn: unexpected internal error"
+            assert "Test error" not in content.text
             assert result.is_error is True
 
     async def test_tool_error_handling(self):
@@ -276,19 +277,21 @@ async def test_tool_error_handling(self):
             assert len(result.content) == 1
             content = result.content[0]
             assert isinstance(content, TextContent)
-            assert "Test error" in content.text
+            assert content.text == "Error executing tool error_tool_fn: unexpected internal error"
+            assert "Test error" not in content.text
             assert result.is_error is True
 
     async def test_tool_error_details(self):
-        """Test that exception details are properly formatted in the response"""
+        """Test that unexpected exception details are hidden in the response."""
         mcp = MCPServer()
         mcp.add_tool(error_tool_fn)
         async with Client(mcp) as client:
             result = await client.call_tool("error_tool_fn", {})
             content = result.content[0]
             assert isinstance(content, TextContent)
             assert isinstance(content.text, str)
-            assert "Test error" in content.text
+            assert content.text == "Error executing tool error_tool_fn: unexpected internal error"
+            assert "Test error" not in content.text
             assert result.is_error is True
 
     async def test_tool_return_value_conversion(self):

tests/server/mcpserver/test_tool_manager.py

@@ -383,18 +383,34 @@ async def async_tool(x: int, ctx: Context) -> str:
         assert result == "42"
 
     @pytest.mark.anyio
-    async def test_context_error_handling(self):
-        """Test error handling when context injection fails."""
+    async def test_unexpected_error_hides_internal_details(self):
+        """Test error handling does not expose unexpected exception details."""
 
         def tool_with_context(x: int, ctx: Context) -> str:
-            raise ValueError("Test error")
+            raise ValueError("secret token leaked")
 
         manager = ToolManager()
         manager.add_tool(tool_with_context)
 
-        with pytest.raises(ToolError, match="Error executing tool tool_with_context"):
+        with pytest.raises(ToolError) as exc_info:
             await manager.call_tool("tool_with_context", {"x": 42}, context=Context())
 
+        assert str(exc_info.value) == "Error executing tool tool_with_context: unexpected internal error"
+        assert "secret token leaked" not in str(exc_info.value)
+
+    @pytest.mark.anyio
+    async def test_tool_error_preserves_message(self):
+        """Test explicit ToolError messages remain visible to clients."""
+
+        def tool_with_expected_error() -> str:
+            raise ToolError("safe client-facing error")
+
+        manager = ToolManager()
+        manager.add_tool(tool_with_expected_error)
+
+        with pytest.raises(ToolError, match="safe client-facing error"):
+            await manager.call_tool("tool_with_expected_error", {}, context=Context())
+
 
 class TestToolAnnotations:
     def test_tool_annotations(self):

tests/server/mcpserver/test_url_elicitation_error_throw.py

@@ -92,7 +92,7 @@ async def multi_auth(ctx: Context) -> str:
 
 @pytest.mark.anyio
 async def test_normal_exceptions_still_return_error_result():
-    """Test that normal exceptions still return CallToolResult with is_error=True."""
+    """Test that normal exceptions still return CallToolResult without exposing details."""
     mcp = MCPServer(name="NormalErrorServer")
 
     @mcp.tool(description="A tool that raises a normal exception")
@@ -105,4 +105,5 @@ async def failing_tool(ctx: Context) -> str:
         assert result.is_error is True
         assert len(result.content) == 1
         assert isinstance(result.content[0], types.TextContent)
-        assert "Something went wrong" in result.content[0].text
+        assert result.content[0].text == "Error executing tool failing_tool: unexpected internal error"
+        assert "Something went wrong" not in result.content[0].text