Repository navigation
Conversation
Tool authors can now raise ToolError(content=[...]) to return a CallToolResult with is_error=True that carries arbitrary content (e.g. an image or embedded resource) instead of only the error message as text. A plain ToolError behaves exactly as before. content is typed as list[Any] rather than list[ContentBlock] because exceptions.py is imported during mcp package initialization, before mcp.types is importable - referencing that type would create a circular import. Closes modelcontextprotocol#348
723770d to
9b6b9af
Compare
|
Good enhancement! Letting ToolError carry structured content (images, embedded resources) is useful for rich error responses. The circular import avoidance note is a smart documentation touch. |
pwdh2026
left a comment
There was a problem hiding this comment.
Verified locally on Python 3.12 (Windows) against PR head 9b6b9af.
Tests
tests/server/mcpserver/tools/test_base.py: 4 passed (incl. the new image-bearing ToolError test).tests/server/mcpserver/: 341 passed, 1 skipped — no regressions.
Behavior probes (PR branch)
ToolError("msg")→ text +is_error=True, prefix preserved (backward compatible).ToolError(content=[TextContent/ImageContent])→ content preserved +is_error=True, works for both async and sync tools.content=[dict]is coerced toImageContentby pydantic.content=[]→ empty content +is_error=True.
One new sharp edge (non-blocking)
ToolError(content=["not-a-content-block"])raises a pydanticValidationErrorwhile constructingCallToolResult; it escapes the error handler and surfaces to the client as anExceptionGroupwrappingMCPError: Invalid request parametersinstead of a cleanis_errorresult. On current main (no content support) the same call degrades gracefully to a text error, so this crash is introduced by the new API surface. Consider validating content items inToolError.__init__(e.g.TypeAdapter[ContentBlock]) or catchingValidationErrorin_handle_call_tooland falling back to the message text; a regression test would help.
Design note (non-blocking)
- When
contentis provided, the error message (including the "Error executing tool " prefix) is dropped entirely — the client/model sees only the content (e.g. a bare image with no explanation). This is documented in the new docstring, but consider prepending aTextContentwith the message so the failure reason survives.
Housekeeping (non-blocking)
- The base is ~6 weeks behind main (main moved to the
mcp_typesnamespace and addedhttpx2/cryptography);mergeable_stateis clean, but a rebase is advisable before merging. content: list[Any] | Nonecould use aTYPE_CHECKING-importedContentBlockfor precision.
Verdict: approve — the change is additive, backward compatible, and behaves as intended for valid input.
|
Thanks for the PR, and sorry it sat here without a proper review. We're closing most of the open PR backlog. v2 is out and changed a lot of the SDK, so many older PRs no longer apply as written, and we're a small team that realistically doesn't have the capacity to work through the rest. If this still matters to you on v2, the most useful thing you can do is open an issue (or comment on the existing one) with your use case and a repro. Hearing why it matters to you is what we use to decide what to prioritise. |
Summary
Closes #348.
Tool authors currently have no way to return a
CallToolResultwithis_error=Truethat carries non-text content — raising an exception only surfaces the message as text. This implements the approach @Kludex suggested in the issue: giveToolErroran optionalcontentfield that is translated to the error result internally.What changed
ToolErroraccepts an optional keywordcontent: list[ContentBlock] | None. When set, it becomes theCallToolResult.content; otherwise behavior is unchanged (the message is returned as text).Tool.run) preservescontentwhen it wraps a raisedToolError, so the content survives to the result._handle_call_toolreturns the attached content for an errored result when present.A plain
ToolError("...")behaves exactly as before — this is purely additive and non-breaking.Tests / checks
ToolError../scripts/testpasses at 100% coverage;pyrightandruffare clean.Disclosure
Developed with AI assistance. I've reviewed the change and can explain every line.