Repository navigation
feat(mcp): set $mcp_error_status from the raised exception's cause chain - #1229
Open
lucasheriques wants to merge 3 commits into
Open
lucasheriques wants to merge 3 commits into
lucasheriques wants to merge 3 commits into
Conversation
This was referenced Nov 11, 2026
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 68b8974: 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.
| CONVERSATION_ID = "$mcp_conversation_id" | ||
| DURATION_MS = "$mcp_duration_ms" | ||
| ERROR_MESSAGE = "$mcp_error_message" | ||
| ERROR_STATUS = "$mcp_error_status" |
Contributor
There was a problem hiding this comment.
P2 · Update the public API snapshot for ERROR_STATUS
Adding ERROR_STATUS changes the exported public API, but references/public_api_snapshot.txt lacks its entry. With the locked Griffe version, make public_api_check exits with this exact mismatch, failing the Public API snapshot CI job. AGENTS.md:11 requires snapshot updates for public API changes; regenerate and include the snapshot.
Contributor
Author
There was a problem hiding this comment.
Fixed in ddeb000. references/public_api_snapshot.txt now lists PostHogMCPAnalyticsProperty.ERROR_STATUS, and check_public_api.py reports it up to date.
Contributor
posthog-python Compliance ReportDate: 2026-11-11T20:01:12.266118+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
|
Contributor
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 |
lucasheriques
force-pushed
the
lucas/mcp-error-status
branch
from
November 11, 2026 16:20
68b8974 to
ddeb000
Compare
6 tasks done
dustinbyrne
approved these changes
Nov 11, 2026
lucasheriques
force-pushed
the
lucas/mcp-error-status
branch
from
November 11, 2026 19:15
ddeb000 to
9f60b4a
Compare
MCP Analytics groups failed calls by $mcp_error_status, but only PostHog's own server set it, so every posthog.mcp failure landed in "unknown". Rootly asked for it in #1223. Walk the cause chain inward (__cause__, else __context__ unless suppressed) and take the first int 100-599 from status_code, statusCode or status on the exception or its response, per the sdk-specs Failure capture rule. A caller's properties still win. Tested: pytest posthog/test/mcp (720 passed, 11 skipped); new parameterized test_error_status.py covers chain shapes and rejected values; FastMCP end-to-end row raises httpx.HTTPStatusError(503) under mask_error_details=True.
…0 links Pre-PR adversarial review found two gaps against the sdk-specs rule: a __cause__ left with __suppress_context__ False was skipped for __context__, and a chain whose links are built on read never ended. Walk the chain a traceback prints, capped at 50 links like $exception_list. Tested: pytest posthog/test/mcp (724 passed, 11 skipped); mypy baseline clean.
… the API snapshot CI on 68b8974 failed Mypy (var-annotated on `seen` in _exceptions.py) and the Public API snapshot check (the new PostHogMCPAnalyticsProperty.ERROR_STATUS constant), which PostHog Review also flagged. Tested: mypy | mypy-baseline filter clean; check_public_api.py up to date; pytest posthog/test/mcp 725 / 606 / 716 passed on fastmcp 3.2 + mcp 1.30, 4.1.8 + mcp 2.3, 2.14.7 + mcp 1.30.
lucasheriques
force-pushed
the
lucas/mcp-error-status
branch
from
November 11, 2026 19:56
9f60b4a to
3d2ad57
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
Part of #1223. MCP Analytics groups failed tool calls by
$mcp_error_status, but only PostHog's own MCP server sets it, so everyposthog.mcpfailure lands in "unknown". Rootly's tools raisehttpx.HTTPStatusError, often wrapped, and asked for the status from the chain.Stacked on #1226, which makes the raised exception available on standalone FastMCP. Review only the top three commits (
3fb9d4f,08b58f0,3d2ad57).A failed tool call now sets
$mcp_error_statusper the new sdk-specs rule (PostHog/sdk-specs#212). The SDK walks the chain a traceback prints, at most 50 links. On each exception and itsresponseit readsstatus_code,statusCode,status. The first int from 100 to 599 wins, and the outermost one wins. A status in the caller'spropertiesstill wins. The value is set on the automatic path and incapture_tool_call.Parity
Rule: PostHog/sdk-specs#212. Rows are the cases the new parameterized tests cover in both SDKs.
response.status_code/response.statushttpx.HTTPStatusError)Response)raise ... from){ cause })raiseinsideexcept)raise ... from Nonestatus_code,statusCode,statuson the errorresponsegetter that throwsisErrorresult with no thrown error$mcp_error_statusinpropertiesstatus$mcp_error_statusPaths where the raised error never reaches the SDK. On these the status and
$mcp_error_typeare both missing. Fixing them is about recovering the raised error, like #1226 did for FastMCP, and is tracked as a follow-up:Server, whose@server.call_tool()turns the raise into anisErrorresult before our wrapper sees it.McpServer.McpServerwhen the tool throws a value that is not anError.💚 How did you test it?
posthog/test/mcp/test_error_status.pyis new and parameterized over the chain shapes and rejected values in the table. It failed before the change and passes after.test_fastmcp_tool_errors.pygains Rootly's case. A standalone FastMCP tool withmask_error_details=TrueraisesRuntimeErrorfromhttpx.HTTPStatusError(503), and the event carries 503.pytest posthog/test/mcp: 724 passed, 11 skipped on mcp 1.30 + fastmcp 3. mcp 2.x was not run locally; CI covers it.ruff format --check,ruff check,mypy | mypy-baseline filterclean.__cause__left with__suppress_context__False was skipped, and that the walk had no bound. Both are fixed in the last commit, with tests. It also found the low-levelServergap above, which is out of scope.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Opus) drafted the spec rule, then wrote this test-first, with the @posthog/mcp change alongside it for parity.