FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(mcpserver): let ToolError carry content for is_error results by RaidLZ · Pull Request #2984 · modelcontextprotocol/python-sdk · GitHub

/ python-sdk Public

feat(mcpserver): let ToolError carry content for is_error results - #2984

Open
RaidLZ wants to merge 1 commit into
modelcontextprotocol:mainfrom
RaidLZ:fix/348-toolerror-content
ConversationCommits (1)ChecksFiles changed
Open

feat(mcpserver): let ToolError carry content for is_error results#2984
RaidLZ wants to merge 1 commit into
modelcontextprotocol:mainfrom
RaidLZ:fix/348-toolerror-content

Conversation

RaidLZ commented Jun 26, 2026

Copy link
Copy Markdown

Summary

Closes #348.

Tool authors currently have no way to return a CallToolResult with is_error=True that carries non-text content — raising an exception only surfaces the message as text. This implements the approach @Kludex suggested in the issue: give ToolError an optional content field that is translated to the error result internally.

What changed

  • ToolError accepts an optional keyword content: list[ContentBlock] | None. When set, it becomes the CallToolResult.content; otherwise behavior is unchanged (the message is returned as text).
  • The tool layer (Tool.run) preserves content when it wraps a raised ToolError, so the content survives to the result.
  • _handle_call_tool returns the attached content for an errored result when present.

A plain ToolError("...") behaves exactly as before — this is purely additive and non-breaking.

@mcp.tool()
def render() -> str:
    raise ToolError(
        "rendering failed",
        content=[ImageContent(type="image", data=..., mime_type="image/png")],
    )
# -> CallToolResult(content=[ImageContent(...)], is_error=True)

Tests / checks

  • Added a test covering an image-bearing ToolError.
  • Existing tool-error behavior (including the snapshot test) is unchanged.
  • ./scripts/test passes at 100% coverage; pyright and ruff are clean.

Disclosure

Developed with AI assistance. I've reviewed the change and can explain every line.

RaidLZ mentioned this pull request Jun 26, 2026
Open
cubic-dev-ai Bot reviewed Jun 26, 2026
View reviewed changes

cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No issues found across 4 files

Re-trigger cubic

feat(mcpserver): let ToolError carry content for is_error results
9b6b9af 
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
RaidLZ force-pushed the fix/348-toolerror-content branch from 723770d to 9b6b9af Compare June 26, 2026 08:14

itxaiohanglover commented Jun 26, 2026

Copy link
Copy Markdown

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 reviewed Aug 9, 2026
View reviewed changes

pwdh2026 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 to ImageContent by pydantic.
  • content=[] → empty content + is_error=True.

One new sharp edge (non-blocking)

  • ToolError(content=["not-a-content-block"]) raises a pydantic ValidationError while constructing CallToolResult; it escapes the error handler and surfaces to the client as an ExceptionGroup wrapping MCPError: Invalid request parameters instead of a clean is_error result. 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 in ToolError.__init__ (e.g. TypeAdapter[ContentBlock]) or catching ValidationError in _handle_call_tool and falling back to the message text; a regression test would help.

Design note (non-blocking)

  • When content is 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 a TextContent with the message so the failure reason survives.

Housekeeping (non-blocking)

  • The base is ~6 weeks behind main (main moved to the mcp_types namespace and added httpx2/cryptography); mergeable_state is clean, but a rebase is advisable before merging.
  • content: list[Any] | None could use a TYPE_CHECKING-imported ContentBlock for precision.

Verdict: approve — the change is additive, backward compatible, and behaves as intended for valid input.

This was referenced Aug 11, 2026
Closed
Merged
niklasgruener mentioned this pull request Aug 20, 2026
Draft
9 tasks
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments
+1 more reviewer
pwdh2026 pwdh2026 left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No way to set isError=True for arbitrary tool result content

3 participants

Back | FazBrowse Home | New Git URL