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

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

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 $mcp_error_message and $mcp_error_type scalars read $exception_list[0], so on mcp 2.1 every unexpected tool failure reported the same masked string and the failures view lost the reason.

The scalars now read the entry behind the dispatch wrapper, which exceptions_from_error_tuple already records from __cause__. The wrapper is matched by type name and message prefix because each SDK major ships its own ToolError class. A wrapper without a chained cause is kept as is, and the $exception sibling still carries the full chain.

This broke CI without a repo change: the rolling exclude-newer = "7 days" quarantine made mcp 2.1.0 (published Aug 24) eligible on Aug 31 at 19:04 UTC, between two runs of the same commit. The unpinned mcp>=2,<3 leg resolved 2.0.0 in the morning and 2.1.0 in the evening. mcp 2.1.1 ages in on Sep 1; the fix is verified against 2.0.0, 2.1.0, and 2.1.1.

Generated-By: PostHog Desktop
Task-Id: ec0cc27f-fc40-4994-b153-2bfe74de1edf

* fix(mcp): gate the unwrap on SDK module and traverse nested wrappers

Two review findings on the dispatch-wrapper unwrap, both reproduced before fixing.

A tool that invokes a failing tool is wrapped once per dispatch, so stepping past a single entry landed on the still-masked inner wrapper. The unwrap now walks every consecutive wrapper to the first real exception; a nested-server test asserts both the inner and the outer event report the root cause.

Matching by type name and message prefix alone also unwrapped an application's own exception that happened to be named ToolError with a matching prefix, replacing the message the application chose to surface. The match now additionally requires the entry's recorded module to come from an SDK namespace (mcp., fastmcp.), verified against mcp 1.28.1, mcp 2.0.0/2.1.0/2.1.1, and standalone fastmcp. The unit tests use the real per-major ToolError classes, and a new test pins that a same-named application exception is kept.

Generated-By: PostHog Desktop
Task-Id: ec0cc27f-fc40-4994-b153-2bfe74de1edf

Author: bernatixer

.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 —