fix(server): populate structuredContent.adcp_error on MCP error results#534
Closed
fix(server): populate structuredContent.adcp_error on MCP error results#534
Conversation
When an ADCPError is raised from a handler, return a CallToolResult directly
instead of raising ToolError. Returning bypasses FastMCP's _make_error_result
path, which drops structuredContent, so both isError=true and
structuredContent.adcp_error.code are now propagated to the wire.
Adds _extract_adcp_error_fields() to translate.py to extract the structured
{code, message, recovery, field?, details?} dict from all ADCPError variants
(ADCPError, ADCPTaskError, Error Pydantic model, DecisioningAdcpError).
Text fallback in content[] is preserved for backward compatibility.
Closes #509
https://claude.ai/code/session_01V4QPiByXeHu2p18TsxjWDF
…r, fix mypy - Add suggestion field to _extract_adcp_error_fields() for all branches (prevents silent drop on MCP→A2A proxy round-trips) - Narrow except Exception → except ImportError for optional decisioning import - Add pyproject.toml mypy override for adcp.decisioning (optional component) https://claude.ai/code/session_01V4QPiByXeHu2p18TsxjWDF
Contributor
Author
|
Superseded by #525 (feat(server): MCP error responses populate structuredContent.adcp_error), already merged to main. |
Contributor
Author
|
Acknowledged — closing the loop here. #525 is already merged, so this draft is superseded. Generated by Claude Code |
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.
Summary
ADCPErroris raised from a handler, return aCallToolResultdirectly instead of raisingToolError. Returning bypasses FastMCP's_make_error_resultpath (which dropsstructuredContent), soisError=trueandstructuredContent.adcp_error.codeare now both populated on the wire._extract_adcp_error_fields()toadcp.server.translate— extracts{code, message, recovery, field?, details?, suggestion?}from allADCPErrorvariants (ADCPError,ADCPTaskError,ErrorPydantic model, optionaladcp.decisioning.types.AdcpError).content[]is preserved for backward compatibility (older clients that parse the text form still work unchanged).Closes #509
Why this approach
FastMCP's
_make_error_result(called when aToolErroris raised) dropsstructuredContent. The MCP lowlevel server has a bypass:isinstance(results, types.CallToolResult)at line 540 short-circuits toreturn types.ServerResult(results)and preserves the result unchanged. Returning aCallToolResultfromfntakes this path; raisingToolErrordoes not.The
convert_resultgate betweenfnreturn and the lowlevel server is safe: FastMCP createsRootModel[dict[str, Any]]fordict[str, Any]-typed tools, which callsmodel_validate(result.structuredContent)on the returnedCallToolResult— any dict passes, so the structured error envelope is never rejected.Test plan
TestExtractAdcpErrorFields(11 tests) — all exception type branches, field/details/suggestion presence/absencetest_adcp_error_returns_structured_call_tool_result—ADCPTaskErrorfrom handler →CallToolResultwithstructuredContent["adcp_error"]["code"]and text fallbacktest_adcp_error_text_fallback_preserved—ADCPAuthenticationErrortext fallback coexists with structured contentruff check src/— cleanmypy src/adcp/— cleanpytest tests/ --ignore=tests/integration --ignore=tests/conformance— 1987 passed, 15 skipped, 1 xfailedPre-PR reviews
Code review: LGTM with two fixes applied — narrowed
except Exception→except ImportErroron optional decisioning import; addedsuggestionfield toDecisioningAdcpErrorbranch to prevent silent drop on proxy round-trips. Both addressed in the refine commit.Protocol review: Confirms
structuredContent.adcp_errornested shape is correct per the AdCP spec's JSON-pointer convention (/adcp_error/code).recoveryenum values (terminal/transient/correctable) confirmed correct. Non-breaking addition under MCP spec.https://claude.ai/code/session_01V4QPiByXeHu2p18TsxjWDF
Generated by Claude Code