From a58db4aeb215decd8917cc5fdc4078d2b4abf4c8 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Wed, 11 Nov 2026 13:19:50 -0300 Subject: [PATCH 1/3] fix(mcp): report the raised exception on mcp 1.x low-level isError results On mcp 1.x the low-level @server.call_tool() decorator catches a tool's raise and returns CallToolResult(isError=True) before our handler sees it, so $mcp_tool_call had $mcp_error_type "Error", no frames and no $mcp_error_status (#1223). The decorator builds that result through server._make_error_result inside its except block, so an instance wrapper reads the live exception there and settles it into the same raised_tool_error slot standalone FastMCP uses. Validation failures also pass through that method inside an except block; only a message equal to str(exception) is the tool's own raise. mcp 2.x low-level needs no change: raw handlers raise through to our wrapper, and a scratch check showed RuntimeError, both frames and status 503. Tested: parameterized test in test_lowlevel.py (plain and wrapped 503, plain ValueError, input-validation guard). Red before on the three raising rows. Removing the message guard turns the validation row into ValidationError. pytest posthog/test/mcp: mcp 1.30 727 passed / 11 skipped; mcp 2.3 bare 551 passed / 55 skipped. CI-exact mypy: only the base branch's _exceptions.py:65. --- .../changesets/mcp-lowlevel-raised-error.md | 5 ++ posthog/mcp/_instrument_lowlevel.py | 7 +- posthog/mcp/_raised_tool_error.py | 30 ++++++-- posthog/test/mcp/test_lowlevel.py | 68 +++++++++++++++---- 4 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 .sampo/changesets/mcp-lowlevel-raised-error.md diff --git a/.sampo/changesets/mcp-lowlevel-raised-error.md b/.sampo/changesets/mcp-lowlevel-raised-error.md new file mode 100644 index 000000000..571b0a34e --- /dev/null +++ b/.sampo/changesets/mcp-lowlevel-raised-error.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +`posthog.mcp`: on mcp 1.x, a tool registered with the low-level `Server`'s `@server.call_tool()` decorator that raises now reports the exception it raised, with its cause chain, stack frames and `$mcp_error_status`, and `$mcp_error_type` names its class. Before, the decorator converted the exception into an error result first, so the failure was a generic `Error` with no frames and no status. diff --git a/posthog/mcp/_instrument_lowlevel.py b/posthog/mcp/_instrument_lowlevel.py index 8d6cf4393..ace769fd3 100644 --- a/posthog/mcp/_instrument_lowlevel.py +++ b/posthog/mcp/_instrument_lowlevel.py @@ -54,7 +54,7 @@ from ._model_parameters import request_meta_from_context from ._model_parameters import can_inject_model_parameter, is_capture_model_enabled from ._output_instructions import mirror_instructions_into_structured_content -from ._raised_tool_error import raised_tool_error +from ._raised_tool_error import raised_tool_error, wrap_error_result from .logger import log, warn from .tools import get_more_tools_result_text @@ -69,6 +69,7 @@ def instrument_low_level(server: Any, data: MCPAnalyticsData) -> None: """ data.server_name = getattr(server, "name", None) data.server_version = getattr(server, "version", None) + wrap_error_result(server) _wrap_call_tool(server, data, strip_injected=False) _wrap_list_tools(server, data, context_required=False) _wrap_resource_requests(server, data) @@ -283,7 +284,9 @@ async def handler(req: Any) -> Any: start = time.monotonic() try: - with raised_tool_error(high_level) as raised: + with raised_tool_error( + server if high_level is None else high_level + ) as raised: result = await original(req) except Exception as error: if await lifecycle.record_if_unknown_tool( diff --git a/posthog/mcp/_raised_tool_error.py b/posthog/mcp/_raised_tool_error.py index e694cf6fe..a77a7fd41 100644 --- a/posthog/mcp/_raised_tool_error.py +++ b/posthog/mcp/_raised_tool_error.py @@ -1,13 +1,15 @@ -"""Recover the exception behind a standalone FastMCP ``isError`` tool result. +"""Recover the exception behind an ``isError`` tool result. -FastMCP's ``tools/call`` handler, the one we wrap, turns whatever its tool -dispatch raised into ``CallToolResult(isError=True)``, keeping only the -message text. Wrapping that dispatch on the server instance sees the exception -first, and hands it to the enclosing handler through a context variable. +Standalone FastMCP's ``tools/call`` handler, and the mcp 1.x low-level +``@server.call_tool()`` decorator, turn whatever the tool raised into +``CallToolResult(isError=True)``, keeping only the message text. A wrapper on +the server instance sees the exception first, and hands it to the enclosing +handler through a context variable. """ from __future__ import annotations +import sys from contextlib import contextmanager from contextvars import ContextVar from dataclasses import dataclass @@ -60,6 +62,24 @@ async def dispatch(*args: Any, **kwargs: Any) -> Any: setattr(server, method, dispatch) +def wrap_error_result(server: Any) -> None: + original = getattr(server, "_make_error_result", None) + if original is None or getattr(original, _WRAPPED_FLAG, False): + return + + def make_error_result(error_message: str) -> Any: + # Input and output validation failures also land here from inside an + # except block, but with a prefixed message. Only the tool's own raise + # is passed through as its bare text. + error = sys.exc_info()[1] + if isinstance(error, Exception) and error_message == str(error): + _settle(server, error) + return original(error_message) + + setattr(make_error_result, _WRAPPED_FLAG, True) + setattr(server, "_make_error_result", make_error_result) + + def _settle(server: Any, error: Optional[BaseException]) -> None: # Another server's dispatch, reached from inside a tool, must not write # its outcome onto this call, nor may a background task that outlives it. diff --git a/posthog/test/mcp/test_lowlevel.py b/posthog/test/mcp/test_lowlevel.py index 62dbd1ed0..9106d53a3 100644 --- a/posthog/test/mcp/test_lowlevel.py +++ b/posthog/test/mcp/test_lowlevel.py @@ -2,6 +2,7 @@ import json +import httpx import pytest import mcp.types as mcp_types @@ -17,7 +18,25 @@ ) -def make_server(*, resource_error: bool = False, listing: str = "resources") -> Server: +def _boom(): + raise ValueError("boom") + + +def _upstream_503(): + request = httpx.Request("GET", "https://api.example.com/invoices/1") + httpx.Response(503, request=request).raise_for_status() + + +def _wrapped_upstream_503(): + try: + _upstream_503() + except httpx.HTTPStatusError as exc: + raise RuntimeError("invoice lookup failed") from exc + + +def make_server( + *, resource_error: bool = False, listing: str = "resources", fail=_boom +) -> Server: server = Server("test-lowlevel") @server.list_tools() @@ -38,7 +57,7 @@ async def list_tools(): async def call_tool(name, arguments): if name == "echo": return [mcp_types.TextContent(type="text", text=str(arguments.get("msg")))] - raise ValueError("boom") + fail() async def list_resources(_request): if listing == "error": @@ -344,26 +363,45 @@ async def test_tool_call_success_captures_intent(): assert "context" not in props["$mcp_parameters"]["request"]["params"]["arguments"] -async def test_tool_call_error_captured_from_is_error_result(): - server = make_server() +@pytest.mark.parametrize( + "fail, name, error_type, chain, status", + [ + (_boom, "fail", "ValueError", ["ValueError"], None), + (_upstream_503, "fail", "HTTPStatusError", ["HTTPStatusError"], 503), + ( + _wrapped_upstream_503, + "fail", + "RuntimeError", + ["RuntimeError", "HTTPStatusError"], + 503, + ), + # Missing "msg" fails the SDK's input validation before the tool runs. + (_boom, "echo", "Error", [], None), + ], +) +async def test_raise_converted_to_is_error_result_carries_the_raised_exception( + fail, name, error_type, chain, status +): + server = make_server(fail=fail) client = FakeClient() instrument(server, client) + await server.request_handlers[mcp_types.ListToolsRequest](None) - handler = server.request_handlers[mcp_types.CallToolRequest] - # "fail" is unlisted -> no validation -> handler raises -> isError result - result = await handler( - _call_request("fail", {"context": "trying a tool that errors"}) + result = await server.request_handlers[mcp_types.CallToolRequest]( + _call_request(name, {"context": "trying a tool that errors"}) ) await _flush() assert result.root.isError is True - calls = _events(client, "$mcp_tool_call") - assert calls and calls[0]["properties"]["$mcp_is_error"] is True - exceptions = _events(client, "$exception") - assert ( - exceptions - and "boom" in exceptions[0]["properties"]["$exception_list"][0]["value"] - ) + [call] = _events(client, "$mcp_tool_call") + assert call["properties"]["$mcp_is_error"] is True + assert call["properties"]["$mcp_error_type"] == error_type + assert call["properties"].get("$mcp_error_status") == status + [exception] = _events(client, "$exception") + exception_list = exception["properties"]["$exception_list"] + raised = [e for e in exception_list if e["type"] in chain] + assert [e["type"] for e in raised] == chain + assert all(e["stacktrace"]["frames"] for e in raised) async def test_initialize_emitted_once(): From 08c4555b93d328ac234979cc6519561e36ce2721 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Wed, 11 Nov 2026 13:29:01 -0300 Subject: [PATCH 2/3] chore(mcp): scope the low-level raise recovery note and changeset to what it covers Pre-PR review: the message guard admits any exception the decorator's catch-all sees (tool body, list_tools cache refresh, result construction), not only the tool's own raise, and _make_error_result first ships in mcp 1.10. Comment and changeset now say so. Tested: pytest posthog/test/mcp on mcp 1.30, 727 passed. --- .sampo/changesets/mcp-lowlevel-raised-error.md | 2 +- posthog/mcp/_raised_tool_error.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.sampo/changesets/mcp-lowlevel-raised-error.md b/.sampo/changesets/mcp-lowlevel-raised-error.md index 571b0a34e..312638b87 100644 --- a/.sampo/changesets/mcp-lowlevel-raised-error.md +++ b/.sampo/changesets/mcp-lowlevel-raised-error.md @@ -2,4 +2,4 @@ pypi/posthog: patch --- -`posthog.mcp`: on mcp 1.x, a tool registered with the low-level `Server`'s `@server.call_tool()` decorator that raises now reports the exception it raised, with its cause chain, stack frames and `$mcp_error_status`, and `$mcp_error_type` names its class. Before, the decorator converted the exception into an error result first, so the failure was a generic `Error` with no frames and no status. +`posthog.mcp`: on mcp 1.x (1.10 and later), a tool registered with the low-level `Server`'s `@server.call_tool()` decorator that raises now reports the exception it raised, with its cause chain, stack frames and `$mcp_error_status`, and `$mcp_error_type` names its class. Before, the decorator converted the exception into an error result first, so the failure was a generic `Error` with no frames and no status. diff --git a/posthog/mcp/_raised_tool_error.py b/posthog/mcp/_raised_tool_error.py index a77a7fd41..20acef92b 100644 --- a/posthog/mcp/_raised_tool_error.py +++ b/posthog/mcp/_raised_tool_error.py @@ -69,8 +69,8 @@ def wrap_error_result(server: Any) -> None: def make_error_result(error_message: str) -> Any: # Input and output validation failures also land here from inside an - # except block, but with a prefixed message. Only the tool's own raise - # is passed through as its bare text. + # except block, but with a prefixed message. Only the catch-all around + # the tool call passes an exception's bare text. error = sys.exc_info()[1] if isinstance(error, Exception) and error_message == str(error): _settle(server, error) From c5481c3f207c0a27430225ad3d0cfb040fc4e3e6 Mon Sep 17 00:00:00 2001 From: Lucas Faria Date: Wed, 11 Nov 2026 16:14:04 -0300 Subject: [PATCH 3/3] fix(mcp): tell validation results apart before rendering the exception Review on #1231 (PostHog Review, dustinbyrne): the message guard called str(error) on jsonschema ValidationErrors, which renders the whole instance and schema. On a deeply nested payload that raised RecursionError and replaced the SDK's validation response; on a large one it blocked the loop. A validation result is now matched by the prefix plus the error's own short .message first, and any failure while inspecting the exception leaves the original result untouched. A tool's own raise worded like a validation error is still recovered. Tested: parameterized test gains a 400-deep payload row (red before: response text 'maximum recursion depth exceeded') and a prefix-worded ValueError row. pytest posthog/test/mcp: mcp 1.30 730 passed / 11 skipped; mcp 2.3 bare 551 passed / 55 skipped. CI-exact mypy clean. --- posthog/mcp/_raised_tool_error.py | 26 ++++++++++++++--- posthog/test/mcp/test_lowlevel.py | 46 +++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/posthog/mcp/_raised_tool_error.py b/posthog/mcp/_raised_tool_error.py index 20acef92b..6cb375f54 100644 --- a/posthog/mcp/_raised_tool_error.py +++ b/posthog/mcp/_raised_tool_error.py @@ -21,6 +21,10 @@ # `call_tool` and dispatches through `_call_tool_middleware`. _DISPATCH_METHODS = ("call_tool", "_call_tool_middleware") +# The prefixes the mcp 1.x `@server.call_tool()` decorator puts on jsonschema +# validation failures. +_VALIDATION_PREFIXES = ("Input validation error: ", "Output validation error: ") + @dataclass class RaisedToolError: @@ -68,11 +72,8 @@ def wrap_error_result(server: Any) -> None: return def make_error_result(error_message: str) -> Any: - # Input and output validation failures also land here from inside an - # except block, but with a prefixed message. Only the catch-all around - # the tool call passes an exception's bare text. error = sys.exc_info()[1] - if isinstance(error, Exception) and error_message == str(error): + if isinstance(error, Exception) and _carries_bare_text(error, error_message): _settle(server, error) return original(error_message) @@ -80,6 +81,23 @@ def make_error_result(error_message: str) -> Any: setattr(server, "_make_error_result", make_error_result) +def _carries_bare_text(error: Exception, error_message: str) -> bool: + # Input and output validation failures also land here from inside an + # except block, with the validator's short message behind a prefix. Only + # the catch-all around the tool call passes an exception's bare text. + # Rendering a validation error walks the whole instance and schema, so + # those results are told apart before `str(error)` runs. + try: + detail = getattr(error, "message", None) + if isinstance(detail, str) and any( + error_message == prefix + detail for prefix in _VALIDATION_PREFIXES + ): + return False + return error_message == str(error) + except Exception: + return False + + def _settle(server: Any, error: Optional[BaseException]) -> None: # Another server's dispatch, reached from inside a tool, must not write # its outcome onto this call, nor may a background task that outlives it. diff --git a/posthog/test/mcp/test_lowlevel.py b/posthog/test/mcp/test_lowlevel.py index 9106d53a3..8cbff6413 100644 --- a/posthog/test/mcp/test_lowlevel.py +++ b/posthog/test/mcp/test_lowlevel.py @@ -363,24 +363,55 @@ async def test_tool_call_success_captures_intent(): assert "context" not in props["$mcp_parameters"]["request"]["params"]["arguments"] +def _worded_like_validation(): + raise ValueError("Input validation error: quota exceeded") + + +def _nested(depth): + value = 0 + for _ in range(depth): + value = [value] + return value + + @pytest.mark.parametrize( - "fail, name, error_type, chain, status", + "fail, name, payload, error_type, chain, status, response", [ - (_boom, "fail", "ValueError", ["ValueError"], None), - (_upstream_503, "fail", "HTTPStatusError", ["HTTPStatusError"], 503), + (_boom, "fail", None, "ValueError", ["ValueError"], None, "boom"), + ( + _upstream_503, + "fail", + None, + "HTTPStatusError", + ["HTTPStatusError"], + 503, + "Server error '503 Service Unavailable'", + ), ( _wrapped_upstream_503, "fail", + None, "RuntimeError", ["RuntimeError", "HTTPStatusError"], 503, + "invoice lookup failed", + ), + ( + _worded_like_validation, + "fail", + None, + "ValueError", + ["ValueError"], + None, + "Input validation error: quota exceeded", ), # Missing "msg" fails the SDK's input validation before the tool runs. - (_boom, "echo", "Error", [], None), + (_boom, "echo", None, "Error", [], None, "Input validation error"), + (_boom, "echo", _nested(400), "Error", [], None, "Input validation error"), ], ) async def test_raise_converted_to_is_error_result_carries_the_raised_exception( - fail, name, error_type, chain, status + fail, name, payload, error_type, chain, status, response ): server = make_server(fail=fail) client = FakeClient() @@ -388,11 +419,14 @@ async def test_raise_converted_to_is_error_result_carries_the_raised_exception( await server.request_handlers[mcp_types.ListToolsRequest](None) result = await server.request_handlers[mcp_types.CallToolRequest]( - _call_request(name, {"context": "trying a tool that errors"}) + _call_request( + name, {"context": "trying a tool that errors", "payload": payload} + ) ) await _flush() assert result.root.isError is True + assert result.root.content[0].text.startswith(response) [call] = _events(client, "$mcp_tool_call") assert call["properties"]["$mcp_is_error"] is True assert call["properties"]["$mcp_error_type"] == error_type