fix(mcp): report the exception a tool raised when the SDK masks it

.sampo/changesets/mcp-unwrap-masked-tool-errors.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+MCP tool failures now report the exception the tool actually raised on `$mcp_error_message` and `$mcp_error_type`, stepping past the SDK's dispatch `ToolError` wrapper. mcp 2.1 masks the original message out of that wrapper, which left the failures view with only `Error executing tool <name>`. The `$exception` sibling still carries the full chain.

posthog/mcp/_posthog_events.py

@@ -157,24 +157,64 @@ def _add_common_properties(event: Event, properties: Dict[str, Any]) -> None:
         properties["$set"] = {**identify_actor_data}
 
 
+_TOOL_DISPATCH_WRAPPERS = ("ToolError", "UnexpectedToolError")
+
+# Where the SDKs define their dispatch wrappers: mcp.server.fastmcp.exceptions
+# (mcp 1.x), mcp.server.mcpserver.exceptions (mcp 2.x), fastmcp.exceptions
+# (standalone fastmcp). An application's own exception carries its own module,
+# so a matching name alone must not unwrap it.
+_SDK_MODULE_PREFIXES = ("mcp.", "fastmcp.")
+
+
+def _is_dispatch_wrapper(entry: Any) -> bool:
+    if not isinstance(entry, dict):
+        return False
+    return (
+        entry.get("type") in _TOOL_DISPATCH_WRAPPERS
+        and str(entry.get("module") or "").startswith(_SDK_MODULE_PREFIXES)
+        and str(entry.get("value", "")).startswith("Error executing tool")
+    )
+
+
+def _primary_exception(error: Any) -> Dict[str, Any]:
+    """Pick the ``$exception_list`` entry that carries the failure reason.
+
+    The MCP SDK's tool dispatch re-raises whatever a tool raised as a
+    ``ToolError`` whose message starts ``Error executing tool <name>``, and
+    mcp >= 2.1 masks the original text out of that message entirely, keeping
+    it only on ``__cause__`` — the next entry of the chain here. The wrapper
+    says nothing the event's tool name does not already say, so the scalars
+    step past every consecutive wrapper (a tool invoking a failing tool is
+    wrapped once per dispatch) to the first real exception.
+    """
+    if not isinstance(error, dict):
+        return {}
+    exception_list = error.get("$exception_list")
+    if not isinstance(exception_list, list) or not exception_list:
+        return {}
+    index = 0
+    while (
+        index + 1 < len(exception_list)
+        and isinstance(exception_list[index + 1], dict)
+        and _is_dispatch_wrapper(exception_list[index])
+    ):
+        index += 1
+    entry = exception_list[index]
+    return entry if isinstance(entry, dict) else {}
+
+
 def _add_error_details(event: Event, properties: Dict[str, Any]) -> None:
     """Surface the failure reason on the primary event itself.
 
     Without these the dashboard has to join to the ``$exception`` sibling to
     know *why* a call failed — and that sibling can be switched off with
     ``enable_exception_autocapture``, or never emitted when no error value was
     passed. Both values are read off the ``$exception_list`` the sibling would
-    carry, so the two always agree; the message is already bounded to
+    carry — the sibling keeps the full chain while the scalars carry the entry
+    ``_primary_exception`` picks; the message is already bounded to
     ``_MAX_ERROR_MESSAGE_LENGTH`` because truncation runs before this mapping.
     """
-    first: Dict[str, Any] = {}
-    error = event.get("error")
-    if isinstance(error, dict):
-        exception_list = error.get("$exception_list")
-        if isinstance(exception_list, list) and exception_list:
-            candidate = exception_list[0]
-            if isinstance(candidate, dict):
-                first = candidate
+    first = _primary_exception(event.get("error"))
 
     # An explicit coarse category (e.g. "validation", "timeout") beats the
     # thrown type; a custom dispatcher can pass one that means something to the

posthog/test/mcp/test_error_properties.py

@@ -23,6 +23,23 @@ def make_client(**kwargs):
     return client, captured
 
 
+class ToolError(Exception):
+    """An application's own ToolError. It shares the SDK wrapper's name but not
+    its module, so the unwrap must leave it alone. Module-level so the recorded
+    type is the bare name and not a ``<locals>`` path."""
+
+
+def _sdk_tool_error() -> type:
+    """The real dispatch-wrapper class for the installed SDK major."""
+    from posthog.test.mcp._helpers import MCP_MAJOR
+
+    if MCP_MAJOR >= 2:
+        from mcp.server.mcpserver.exceptions import ToolError as SDKToolError
+    else:
+        from mcp.server.fastmcp.exceptions import ToolError as SDKToolError
+    return SDKToolError
+
+
 async def test_failed_call_carries_message_and_type():
     client, captured = make_client()
     client.capture_tool_call("add", is_error=True, error=ValueError("bad input"))
@@ -153,6 +170,102 @@ def boom() -> str:
     assert "explode" in props[P.ERROR_MESSAGE]
 
 
+async def test_the_sdk_dispatch_wrapper_is_unwrapped_to_its_cause():
+    """mcp >= 2.1 masks an unexpected tool exception to ``Error executing tool
+    <name>`` and keeps the original only on ``__cause__``. The scalars must
+    carry that original; the ``$exception`` sibling keeps the full chain."""
+    client, captured = make_client()
+    sdk_tool_error = _sdk_tool_error()
+
+    try:
+        try:
+            raise ValueError("explode")
+        except ValueError as original:
+            raise sdk_tool_error("Error executing tool boom") from original
+    except sdk_tool_error as wrapper:
+        client.capture_tool_call("boom", is_error=True, error=wrapper)
+    await _flush()
+
+    props = _events(captured, "$mcp_tool_call")[0]["properties"]
+    assert props[P.ERROR_MESSAGE] == "explode"
+    assert props[P.ERROR_TYPE] == "ValueError"
+    sibling = _events(captured, "$exception")[0]["properties"]["$exception_list"]
+    assert sibling[0]["type"] == "ToolError"
+
+
+async def test_a_dispatch_wrapper_without_a_cause_is_kept():
+    """With no chained cause the wrapper's own message is all there is."""
+    client, captured = make_client()
+
+    client.capture_tool_call(
+        "boom", is_error=True, error=_sdk_tool_error()("Error executing tool boom")
+    )
+    await _flush()
+
+    props = _events(captured, "$mcp_tool_call")[0]["properties"]
+    assert props[P.ERROR_MESSAGE] == "Error executing tool boom"
+    assert props[P.ERROR_TYPE] == "ToolError"
+
+
+async def test_an_application_error_sharing_the_wrapper_name_is_kept():
+    """``capture_tool_call`` accepts arbitrary exceptions. The wrapper is
+    matched by SDK module, not just name, so an application's own ToolError
+    keeps the message and type the application chose to surface."""
+    client, captured = make_client()
+
+    try:
+        try:
+            raise ValueError("inner detail")
+        except ValueError as original:
+            raise ToolError("Error executing tool application task") from original
+    except ToolError as wrapper:
+        client.capture_tool_call("task", is_error=True, error=wrapper)
+    await _flush()
+
+    props = _events(captured, "$mcp_tool_call")[0]["properties"]
+    assert props[P.ERROR_MESSAGE] == "Error executing tool application task"
+    assert props[P.ERROR_TYPE] == "ToolError"
+
+
+async def test_nested_dispatch_wrappers_unwrap_to_the_root_cause():
+    """An outer tool that invokes a failing inner tool gets wrapped twice —
+    once per dispatch — so the scalars must step past every wrapper, not just
+    the first, on both the inner and the outer event."""
+    from posthog.test.mcp._helpers import MCP_MAJOR, FakeClient
+
+    if MCP_MAJOR >= 2:
+        from mcp.server.mcpserver import MCPServer as Server
+    else:
+        from mcp.server.fastmcp import FastMCP as Server
+
+    from posthog.mcp import instrument
+
+    server = Server("nested-e2e")
+
+    @server.tool()
+    def inner() -> str:
+        raise ValueError("root failure")
+
+    @server.tool()
+    async def outer() -> str:
+        return await server._tool_manager.call_tool("inner", {})
+
+    client = FakeClient()
+    instrument(server, client)
+
+    try:
+        await server._tool_manager.call_tool("outer", {})
+    except Exception:
+        pass
+    await _flush()
+
+    calls = _events(client, "$mcp_tool_call")
+    assert len(calls) == 2
+    for call in calls:
+        assert call["properties"][P.IS_ERROR] is True
+        assert "root failure" in call["properties"][P.ERROR_MESSAGE]
+
+
 async def test_a_secret_in_the_message_is_redacted_on_both_surfaces():
     """An exception message is free text a server wrote, so it can carry the
     credential that caused the failure. It must be redacted before it leaves —