Repository navigation
fix(mcp): report the exception behind a FastMCP isError tool result - #1226
lucasheriques wants to merge 9 commits into
Conversation
…exception Standalone FastMCP converts a raised tool exception into an isError result before our wrapped handler returns, so the failure is reported as a generic "Error" with no frames (posthog-python#1223). Fails on fastmcp 3/mcp 1 and fastmcp 4/mcp 2.
Standalone FastMCP converts a raised tool exception into CallToolResult(isError=True) inside the tools/call handler we wrap, so every failure was captured from the result text: $mcp_error_type "Error", one generic $exception_list entry, no frames (posthog-python#1223). instrument() now wraps the server's public call_tool, which still raises (ToolError chained from the tool's exception). The wrapper hands the outermost call's outcome to the enclosing handler through a context variable, and the handler records it next to the result. fastmcp's "Error calling tool" ToolError joins the dispatch wrappers the scalars step past, so $mcp_error_type names the tool's own exception. Tested: - test_fastmcp_tool_errors.py fails before, passes after, on fastmcp 3.2 + mcp 1.30 and fastmcp 4.1.8 + mcp 2.3 - pytest posthog/test/mcp: 694 passed (mcp 1.x), 575 passed (mcp 2.x) - issue repro: $mcp_error_type RuntimeError, chain ToolError -> RuntimeError -> ConnectionError with frames - ruff format/check, mypy clean Not included: $mcp_error_status. The taxonomy defines it as the upstream status of PostHog's own server calls and no SDK sets it today; setting it from the SDK is a cross-SDK contract change.
posthog-python Compliance ReportDate: 2026-11-12T13:52:31.482364+00:00 ✅ All Tests Passed!270/270 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_V4 Tests✅ 149/149 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
SDK performance: posthog-pythonBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
Pre-PR gate findings on the previous commit: - The error slot was shared across servers. A tool that called another instrumented FastMCP's call_tool and handled its failure got that exception on its own successful event, whenever its own server's call_tool was not wrapped (a raw low-level server, or a FastMCP instrumented only through _mcp_server). The slot now names the server whose handler opened it, and only that server's call_tool writes to it. - The "Error calling tool" wrapper match also unwrapped a tool's own ToolError worded that way. It now requires fastmcp's quoted tool name. Tested: both repros added to test_fastmcp_tool_errors.py; pytest posthog/test/mcp 697 passed (mcp 1.30 + fastmcp 3.2), 578 passed (mcp 2.3 + fastmcp 4.1.8); ruff and mypy clean.
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 81b3170: 2 Must fix, 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
|
|
||
|
|
||
| def wrap_call_tool(server: Any) -> None: | ||
| original = server.call_tool |
There was a problem hiding this comment.
P1 · Preserve instrumentation for FastMCP 2.x
On standalone FastMCP 2.x (reproduced on 2.14.0), FastMCP exposes _call_tool and _call_tool_middleware, not call_tool. This access raises AttributeError before instrument() installs any handlers, so it returns _NoopAnalytics and captures no MCP events on a previously supported stack. Guard this hook or wrap the corresponding 2.x method.
There was a problem hiding this comment.
Fixed in f5aea70. FastMCP 2.x dispatches tools/call through _call_tool_middleware, so that's the method wrapped there, and call_tool on 3.x and later. Before the fix, 63 MCP tests failed on fastmcp 2.14.7 + mcp 1.30. Now all 689 pass, and 2.x also reports the raised exception (the issue's repro gives RuntimeError with the full chain). I added a v1-fastmcp2 leg to the tests-mcp matrix so CI covers 2.x from now on.
| allow_self_reported_model=self.allow_self_reported_model, | ||
| analytics_owned_parameters=self.analytics_owned_parameters, | ||
| result=result, | ||
| error=error, |
There was a problem hiding this comment.
P1 · Sanitize source context before capturing recovered exceptions
On failed standalone FastMCP calls, record_result now forwards exceptions with source snippets in their stack frames. _sanitize_exception_values scrubs only exception value, leaving pre_context, context_line, and post_context untouched. URL passwords and PostHog tokens in those lines reach $exception verbatim, violating the MCP privacy contract; sanitize frame strings before capture.
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
+1, on fastmcp 3.2.0 a phx_ token written in the raise line is redacted in value but shows up as-is in that frame's context_line. Base sent no frames here.
There was a problem hiding this comment.
Fixed in 34fbaeb. The sanitizer now runs pre_context, context_line and post_context of every frame through sanitize_captured_value, the same pass value gets. That also covers the official SDK path, which already sent frames. test_secrets_in_frame_source_lines_are_redacted puts a database URL with a password on the line before the raise. It fails without the change and passes with it, on fastmcp 2.14.7, 3.2 and 4.1.8.
| if raised is not None and raised.server is server: | ||
| raised.error = error |
There was a problem hiding this comment.
P2 · Prevent background calls from overwriting the settled exception
A same-server background call_tool inherits the parent's mutable RaisedToolError. If the parent raises and the child finishes during the handler's awaited registered_tool_exists, _settle overwrites the client's failure: an outer RuntimeError is reported as the child's ValueError. Freeze the outcome on context exit or restrict settlement to the outer call.
There was a problem hiding this comment.
Fixed in f5aea70. The slot now closes when the handler's dispatch returns, so a background task can no longer write to it afterwards. I couldn't reproduce the race: a tool that spawned a failing background call_tool and then raised kept its own RuntimeError across a 0-40 event-loop tick sweep on fastmcp 3.2 and 4.1.8. So there's no test for it. Closing the slot on exit makes the window impossible by construction.
ioannisj
left a comment
ioannisj
left a comment
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
The FastMCP 2.x and frame source context threads both reproduce for me on this head, so I'd want those two fixed before this merges. On the description: it opens with Fixes #1223, but the issue also asks for $mcp_error_status and that part isn't included here, so merging closes the issue with the status request still open and nothing tracking it. I think leaving it out is the right call, the spec only says MAY and posthog-js doesn't set it either. Worth switching to Part of #1223 so the issue stays open for that discussion?
|
|
||
|
|
||
| def wrap_call_tool(server: Any) -> None: | ||
| original = server.call_tool |
| allow_self_reported_model=self.allow_self_reported_model, | ||
| analytics_owned_parameters=self.analytics_owned_parameters, | ||
| result=result, | ||
| error=error, |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
+1, on fastmcp 3.2.0 a phx_ token written in the raise line is redacted in value but shows up as-is in that frame's context_line. Base sent no frames here.
Review on 81b3170 (PostHog Review, confirmed by ioannisj): - FastMCP 2.x has no public call_tool, so wrapping it raised AttributeError and instrument() fell back to no-op analytics: no MCP events at all on a supported stack. 2.x dispatches tools/call through _call_tool_middleware, so that is the seam wrapped there. 2.x now also reports the raised exception. - A background task holding the slot could settle it after the handler's dispatch returned. The slot now closes when the handler leaves it. - CI gains a v1-fastmcp2 leg (mcp 1.x + fastmcp 2.x), which would have caught the first point. Tested: pytest posthog/test/mcp passes on fastmcp 2.14.7 + mcp 1.30, fastmcp 3.2 + mcp 1.30, fastmcp 4.1.8 + mcp 2.3 (at 81b3170, 63 tests failed on fastmcp 2.14.7; at ae8f0bd all passed). The background race did not reproduce in a 0-40 tick sweep on 3.2 or 4.1.8, so it has no test; the close-on-exit makes it impossible by construction.
The MCP sanitizer scrubbed each exception's value but not its frames' pre_context, context_line and post_context. A token written in the source around a raise reached $exception verbatim. This was already true on the official SDK path; #1223's fix made it reachable on standalone FastMCP too, where frames were never sent before. Those three keys now pass through the same sanitize_captured_value as the value. Tested: test_secrets_in_frame_source_lines_are_redacted fails without the sanitizer change and passes with it; full MCP suites pass on fastmcp 2.14.7, 3.2 and 4.1.8.
| raised.open = False | ||
| _current.reset(token) |
There was a problem hiding this comment.
P2 · Release recovered exceptions after recording the outcome
When a failing tool starts a long-lived background task, that task inherits the mutable RaisedToolError. Setting open=False leaves error attached, retaining its traceback and all tool-frame locals after the request finishes. Repeated failures retain request payloads for the background tasks' lifetimes; clear the shared exception after recording the outcome.
There was a problem hiding this comment.
Fixed in 52fc421. The slot a background task can reach is now emptied on exit, and the handler reads the outcome from a separate object only it holds. test_a_background_task_does_not_keep_the_failure_alive holds a weakref to a tool local while the tool's background task is still pending. It fails before and passes after, on fastmcp 2.14.7, 3.2 and 4.1.8.
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-adding the stamphog label gives the same result unless the changed files change.
stamphog can't auto-review this pull request because two gates refused it. The deny-list gate flagged .github/workflows/ci.yml as infra/CI-CD (it adds a new v1-fastmcp2 matrix leg). The tier gate classified the change as T2-never: it is a cross-cutting fix spanning 10 files and 319 lines, including the instrumentation, sanitizer, and event-building code under posthog/mcp/.
A human reviewer needs to look at this one. To make that easier, you could move the CI workflow change into its own pull request, so the posthog/mcp changes are smaller and no longer touch CI config. Either way, please ask a maintainer to review it.
- posthog[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✗ | matches: infra_cicd |
| size | ✓ | 154L, 8F substantive, 319L/10F incl. docs/generated/snapshots — within ceiling |
| tier | ✗ | classified as T2-never: T2-never (319L, 10F, cross-cutting, fix) |
| stamphog 2.7.0 | .stamphog/policy.yml @ unknown · reviewed head 34fbaeb |
…ck alive PostHog Review on 34fbaeb: a background task started by a failing tool copies the request's context, and with it the slot holding the raised exception. Closing the slot left the exception attached, so its traceback and every frame's locals lived as long as the task. The slot the context can reach is now emptied on exit; the handler reads the outcome from a separate object only it holds. Tested: test_a_background_task_does_not_keep_the_failure_alive holds a weakref to a tool local while the tool's background task is still pending. It fails before and passes after on fastmcp 2.14.7, 3.2 and 4.1.8. The test turns logging off, because pytest's log capture keeps fastmcp's error records and their exc_info would retain the traceback by itself. Full MCP suites: 690 / 699 / 580 passed.
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 52fc421: 1 Must fix.
This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
| return { | ||
| **frame, | ||
| **{ | ||
| key: sanitize_captured_value(frame[key]) |
There was a problem hiding this comment.
P1 · Redact sensitive members embedded in captured source lines
When a failed FastMCP tool has a source line containing {"password": "hunter2"}, _sanitize_frame sends the credential unchanged: sanitize_captured_value redacts dictionary keys but does not recognize sensitive members embedded in strings. Newly captured source therefore violates the MCP payload privacy contract; redact embedded sensitive members before sending frame context.
There was a problem hiding this comment.
Fixed in d8a7a55. The root cause was wider than frames: posthog-python never implemented the spec's JSON-member pass, so {"password": "hunter2"} also passed through response text and error messages. I ported posthog-go's and posthog-rs's pattern into _redact_credentials, so every captured string gets it, frame source included. posthog-go's JSON-member test table is ported as a parameterized test, plus a source-line case: 13 redaction cases failed before and all pass after. The frame test now has a JSON credential next to "account": "reader", which must survive. Full MCP suites pass on fastmcp 2.14.7, 3.2 and 4.1.8.
dustinbyrne
left a comment
dustinbyrne
left a comment
There was a problem hiding this comment.
Native FastMCP exception recovery and the latest traceback-lifetime correction are verified. The earlier compatibility, URL/token source-redaction, parent-outcome and retention cases now pass.
One credential-redaction gap remains in newly captured source context: sensitive JSON-member values can be transmitted unchanged. The independently executed public-client/HTTP regression fails on reviewed head 52fc4216becdfa410b5615d87ab3de4fc22278f8; reproduction and requested correction are in the inline finding.
AI-assisted review.
| return { | ||
| **frame, | ||
| **{ | ||
| key: sanitize_captured_value(frame[key]) |
There was a problem hiding this comment.
1. [P1] Redact sensitive JSON members in captured source context
IMPORTANT · REQUIRED · Exposes/amplifies an existing sanitizer omission
Location: posthog/mcp/_sanitization.py:709, RIGHT.
A normal failed FastMCP tool can now transmit a password from nearby source text even when its exception message contains no credential. With readable source and default exception autocapture, a line such as credentials = {"password": "hunter2"} appears unchanged in $exception frame pre_context; the expected value is "[redacted]".
_sanitize_frame() delegates source strings to sanitize_captured_value(). Sensitive-key redaction handles dictionary objects, while its string path applies token/entropy/URL passes but not sensitive JSON-member redaction. The pinned MCP Payload privacy requirement explicitly requires that redaction inside any string, including this member shape. The rationale mentions structured responses, but does not limit the rule to them.
This executed repository-style regression uses synthetic data:
import json
from fastmcp import Client, FastMCP
from posthog.mcp import MCPAnalyticsOptions, instrument
from posthog.test.mcp._helpers import FakeClient, events_named, flush_background
async def test_sensitive_json_members_in_frame_source_are_redacted():
server = FastMCP("json-source-regression")
@server.tool
def fail() -> str:
credentials = {"password": "hunter2"} # Synthetic fixture.
raise RuntimeError("synthetic failure")
sink = FakeClient()
instrument(server, sink, MCPAnalyticsOptions())
async with Client(server) as client:
result = await client.call_tool("fail", {}, raise_on_error=False)
await flush_background()
assert result.is_error
[exception] = events_named(sink, "$exception")
assert "hunter2" not in json.dumps(exception["properties"]["$exception_list"])I separately verified actual HTTP transmission through public Client(server), instrument(server, real_posthog_client), the SDK queue and /batch/ transport, mock-service inspection, flush and shutdown. That fixture also confirms the scalar error message has no password and ordinary source text ("account": "reader") remains present.
Equivalent Python 3.11.13 / MCP 1.28.1 / FastMCP 3.2.0 comparisons:
| Observation | Merge base ae8f0bd2 |
Previous 34fbaeb2 |
Reviewed 52fc4216 |
|---|---|---|---|
| Application frames | 0 | 1 | 1 |
| Password literal transmitted in source | No | Yes | Yes |
| HTTP privacy assertion | Pass, exit 0 | Fail, exit 1 | Fail, exit 1 |
| Pytest regression above | Pass | Fail | Fail |
Both current failures are privacy assertions, not setup/import failures. The HTTP assertion reports Sensitive JSON member leaked in captured source context.
The generic string omission predates the latest memory fix, but the original PR newly emits these source frames on this standalone path; identical base calls send frameless generic errors and do not transmit the literal. This demonstrates credential-shaped source transmission, not an actual customer-secret incident, arbitrary-local-variable capture or deployed backend disclosure. Disabling the exception sibling or repairing it in before_send can mitigate exposure, but does not satisfy the default privacy contract.
Consider adding the required embedded-member redaction to the string path used by source context and retaining this regression and its benign-source control. Preserve ordinary source text and frame shapes. No proposed correction was applied or tested.
There was a problem hiding this comment.
Fixed in d8a7a55. The root cause was wider than frames: posthog-python never implemented the spec's JSON-member pass, so {"password": "hunter2"} also passed through response text and error messages. I ported posthog-go's and posthog-rs's pattern into _redact_credentials, so every captured string gets it, frame source included. posthog-go's JSON-member test table is ported as a parameterized test, plus a source-line case: 13 redaction cases failed before and all pass after. The frame test now has a JSON credential next to "account": "reader", which must survive. Full MCP suites pass on fastmcp 2.14.7, 3.2 and 4.1.8.
Review on 52fc421 (PostHog Review, dustinbyrne): a failed FastMCP tool whose source has `credentials = {"password": "hunter2"}` sent the password in its frame's source context. The cause is wider than frames: the MCP Payload privacy spec requires redacting, inside any captured string, the value of a JSON member whose key is on the credential list, and posthog-python never implemented it. posthog-go and posthog-rs do. This ports their pattern into _redact_credentials, so it covers response text, error messages and frame source alike. Tested: posthog-go's TestSanitizeStringRedactsSensitiveJSONMembers table ported as a parameterized test, plus a source-line case; 13 redaction cases failed before, all 17 pass after. The frame test now also carries a JSON credential next to a benign member that must survive. Full MCP suites pass: 716 (fastmcp 3.2 + mcp 1.30), 597 (fastmcp 4.1.8 + mcp 2.3), 707 (fastmcp 2.14.7 + mcp 1.30). Ruff and mypy clean.
There was a problem hiding this comment.
PostHog Review · Standard · reviewed d8a7a55: 1 Must fix.
This is review round 4 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
| + r"|\[(?:\s*(?:" | ||
| + _JSON_SCALAR | ||
| + r"|null)\s*,?)*\s*(?:\]|$))" |
There was a problem hiding this comment.
P1 · Require separators between numeric array elements
A captured string containing "note":[ followed by 30 digits and ! makes sanitize_captured_value run for over three seconds. The optional comma lets the repeated numeric alternative partition those digits exponentially when matching fails. Sanitization blocks the MCP event loop, allowing request-driven denial of service; require separators and eliminate ambiguous repetition.
There was a problem hiding this comment.
Fixed in 9c1cb4f. The array part made every comma optional. Go and Rust run this pattern on linear-time engines, but Python's re backtracks, so a run of digits split into array items in exponentially many ways. Every item after the first now needs its comma, plus an optional trailing one so an array cut off mid-list still matches. That leaves each input exactly one parse. test_json_member_redaction_runs_in_linear_time_on_malformed_arrays timed out at 5 s on the digit inputs before and passes after. I also probed ten adversarial 100k-character shapes (open arrays, lone quotes, unclosed strings, escapes, number characters), and each finishes in under 10 ms.
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Review on d8a7a55 (PostHog Review, veria-ai): the array part of the JSON-member pattern made every comma optional. Go and Rust run that pattern on linear-time engines; Python's re backtracks, so a caller's `{"note":[` + 40 digits + `!` split the digits into array items in exponentially many ways and blocked the event loop. Every item after the first now needs its comma, with an optional trailing one so an array cut off mid-list still matches, which leaves each input one parse. Tested: test_json_member_redaction_runs_in_linear_time_on_malformed_arrays timed out at 5 s on two of three inputs before and passes after. Ten adversarial 100k-character shapes (open arrays, lone quotes, unclosed strings, escapes, number characters) each finish under 10 ms. All JSON-member cases still pass. Full MCP suites: 719 / 600 / 710 passed on fastmcp 3.2 / 4.1.8 / 2.14.7.
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 9c1cb4f: 1 Must fix.
This is review round 5 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
| r"\[\s*(?:" + _JSON_ITEM + r"(?:,\s*" + _JSON_ITEM + r")*(?:,\s*)?)?(?:\]|$)" | ||
| ) | ||
| _JSON_MEMBER_PATTERN = re.compile( | ||
| r'("((?:[^"\\\n]|\\.)*)"\s*:\s*)(?:' + _JSON_SCALAR + "|" + _JSON_ARRAY + ")" |
There was a problem hiding this comment.
P1 · Scan escaped quotes without repeatedly rescanning the remaining text
A captured string containing 16,000 backslash–quote pairs (32 KB) takes about 2.4 seconds through process_mcp_event. _JSON_MEMBER_PATTERN retries at every escaped quote and rescans the remaining suffix, producing quadratic work. Request-controlled arguments therefore block the MCP event loop before truncation; scan strings once without restarting inside escaped sequences.
There was a problem hiding this comment.
Fixed in f9832cc. This is the second performance finding in the same regex, so I replaced it rather than patching it again. Go and Rust run the pattern on linear-time engines, but Python's re restarts at every quote and rescans the rest of the text: 16,000 escaped quote pairs took about 3 s here, and 64,000 took 49 s. The new scanner tries the same key starts but caches where each string body ends and what follows each closing quote, so overlapping candidates share their work. Ten adversarial inputs of 100k to 200k characters, including both of the ones reported here, each finish in under 60 ms. A differential test keeps the reference pattern as an oracle and checks the scanner against it on 30,000 random texts, so the redaction rules stay the same as in Go and Rust. A one-off run of 200,000 longer texts found no mismatch.
…ry quote Review on 9c1cb4f (PostHog Review, veria-ai): with no member to match, the JSON-member regex restarted at every quote and rescanned the rest of the text, so 16,000 escaped quote pairs took about 3 s and 64,000 took 49 s on the server's event loop. This is the second performance finding in that regex: Go and Rust run it on linear-time engines, and Python's re does not. It is replaced by a scanner that tries the same key starts but caches where a string body ends and what follows each closing quote, so overlapping candidates share their work. Tested: the linear-time test gains escaped-quote and JSON-encoded-quote inputs at 32,000 pairs, which timed out at 5 s before and pass after; ten adversarial 100k-200k-character shapes each finish under 60 ms. A differential test keeps the reference pattern as an oracle and checks the scanner against it on 30,000 random texts (a one-off run of 200,000 longer ones found no mismatch). Full MCP suites: 722 / 603 / 713 passed on fastmcp 3.2 / 4.1.8 / 2.14.7; ruff and mypy clean.
💡 Motivation and Context
Part of #1223. On a standalone FastMCP server, every failed tool call reached PostHog as
$mcp_error_type: "Error"with one synthetic$exception_listentry and no stack frames, so MCP failure views and error tracking could not tell one failure from another.FastMCP's
tools/callhandler turns whatever its publiccall_toolraised intoCallToolResult(isError=True)and keeps only the message text. Our wrapper sits around that handler, so it only ever saw the text.instrument()now wraps the server's tool dispatch, which still raises (ToolErrorchained from the tool's exception). That'scall_toolon fastmcp 3 and later, and_call_tool_middlewareon fastmcp 2, which has no publiccall_tool. The wrapper hands the outermost call's outcome to the enclosing handler through a context variable, and the handler records the exception next to the result. The slot names the server whose handler opened it, so a tool that calls another server'scall_tooland handles the failure never gets that exception on its own event. fastmcp'sError calling tool ...wrapper joins the dispatch wrappers the scalars already step past, so$mcp_error_typenames the tool's own exception, the same as on the official SDK path.$mcp_error_typeErrorRuntimeError$exception_listError(generic, 0 frames)ToolError->RuntimeError->ConnectionError, each with framesFrames now leave the process on this path, so the MCP sanitizer also redacts each frame's
pre_context,context_lineandpost_context, not just the exceptionvalue. That also closes the same gap on the official SDK path, which already sent frames.With
mask_error_details=True, the client still sees the masked text, but PostHog now gets the real exception and message. Masking controls what the MCP client sees. The official SDK path already sends the raised exception to PostHog, and error tracking needs it.Not included:
$mcp_error_status, which the issue also asks for. The taxonomy defines it as the upstream status of PostHog's own server calls, and no SDK sets it today. Setting it from the SDKs is a cross-SDK contract change, so it gets its own discussion. #1223 stays open for it.💚 How did you test it?
posthog/test/mcp/test_fastmcp_tool_errors.pylands in its own commit first and fails on both stacks ('Error' == 'RuntimeError'), then passes with the fix. Cases: a chained exception withmask_error_detailsoff and on, a tool raisingToolErroritself (including one worded like fastmcp's wrapper), and a tool that handles a failedcall_tooland succeeds. That last case covers a failure from the same server, from another server, and on a server instrumented only through_mcp_server, and it must report no error.test_secrets_in_frame_source_lines_are_redactedfails without the sanitizer change and passes with it.pytest posthog/test/mcp: 698 passed on mcp 1.30 + fastmcp 3.2, 579 on mcp 2.3 + fastmcp 4.1.8, and 689 on mcp 1.30 + fastmcp 2.14.7. CI gains av1-fastmcp2leg for the last combination; review caught that this PR had broken fastmcp 2.x, and nothing in CI ran it.$mcp_error_type RuntimeErrorand the three-entry chain with frames.ruff format --check,ruff check,mypy | mypy-baseline filterclean.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Opus) reproduced the issue on both fastmcp majors, traced the conversion to fastmcp's
_call_tool_mcp, and wrote the fix test-first. The issue suggested registering a FastMCP middleware; wrappingcall_toolwas chosen instead because it sits outside every middleware no matter when the application adds its own, and needs no ordering against them. A pre-merge adversarial review (Opus and Sonnet subagents) found the cross-server leak and the over-broad wrapper match. Both are fixed in the last commit, with tests.