Skip to content

Harden client tool result error handling#3778

Merged
jlowin merged 1 commit intoPrefectHQ:mainfrom
aimable100:fix/client-tool-error-handling
Apr 8, 2026
Merged

Harden client tool result error handling#3778
jlowin merged 1 commit intoPrefectHQ:mainfrom
aimable100:fix/client-tool-error-handling

Conversation

@aimable100
Copy link
Copy Markdown
Contributor

@aimable100 aimable100 commented Apr 7, 2026 •
edited
Loading

Description

_parse_call_tool_result crashes on error results with empty or non-text content, and parses structuredContent on error results through the success validation pipeline. These surface when connecting to non-FastMCP servers that don't produce TextContent on errors.

Bugs fixed:

  • Crash on empty error content. isError=True with no content blocks raises IndexError. MCP does not mandate text on errors, so third-party servers can legally return empty content.
  • Crash on non-text error content. If the first content block is not TextContent, the cast() call raises AttributeError.
  • Structured content parsed on error results. isError=True results with structuredContent fall through to the success parsing pipeline, attempting schema validation against a diagnostic payload.

Changes:

  • Error content extraction is guarded with isinstance and falls back to a tool-name-scoped message.
  • The structuredContent branch requires not result.isError.
  • except Exception narrowed to (ValidationError, KeyError, TypeError, ValueError) so unexpected exceptions propagate instead of being logged and dropped.
# Before: crashes if server returns empty content on error
result = await client.call_tool("tool_on_foreign_server", {})
# IndexError: list index out of range

# After: clean ToolError with a meaningful message
result = await client.call_tool("tool_on_foreign_server", {})
# ToolError: Tool 'tool_on_foreign_server' returned an error

Contribution type

  • Bug fix (simple, well-scoped fix for a clearly broken behavior)
  • Documentation improvement
  • Enhancement (maintainers typically implement enhancements — see CONTRIBUTING.md)

Checklist

  • This PR addresses an existing issue (or fixes a self-evident bug)
  • I have read CONTRIBUTING.md
  • I have added tests that cover my changes
  • I have run uv run prek run --all-files and all checks pass
  • I have self-reviewed my changes
  • If I used an LLM, it followed the repo's contributing conventions (not generic output)

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. client Related to the FastMCP client SDK or client-side functionality. labels Apr 7, 2026
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3f8cadc9b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/fastmcp/client/mixins/tools.py Outdated
@aimable100 aimable100 force-pushed the fix/client-tool-error-handling branch 2 times, most recently from c22eaa2 to 2655726 Compare April 7, 2026 06:54
jlowin
jlowin requested changes Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@jlowin jlowin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The empty content and non-text content guards are reasonable — that cast + blind index is genuinely fragile, and guarding it is the right call.

However, please revert the except Exception narrowing back to except Exception. That catch-all is intentional: the structured content parsing pipeline involves schema lookup, type coercion, and Pydantic validation, and we don't want any failure there to crash the tool call. Narrowing to a hand-picked list of exceptions is just predicting what might go wrong — the whole point is to handle what we didn't predict.

Also, for future reference — we generally expect bug fix PRs to reference an existing issue or provide a concrete reproduction (which server produces these edge cases?). The fixes here are defensible from reading the code, so I'm not going to block on that, but it helps us prioritize.

@aimable100
Copy link
Copy Markdown
Contributor Author

aimable100 commented Apr 8, 2026

Makes sense on except Exception, reverting that.
On reproduction: the empty/non-text content guards are defensive against third-party servers as MCP doesn't mandate TextContent on errors. The structuredContent case is reproducible with stock FastMCP: any middleware returning isError=True with structuredContent from on_call_tool triggers it. The test in the PR covers it directly.
Noted on the process for future PRs.

@aimable100 aimable100 force-pushed the fix/client-tool-error-handling branch from 2655726 to 40be7d6 Compare April 8, 2026 04:50
jlowin
jlowin approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@jlowin jlowin left a comment

Choose a reason for hiding this comment

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

Thank you!

@jlowin jlowin merged commit 556fd8f into PrefectHQ:main Apr 8, 2026
aimable100 added a commit to tenuo-ai/tenuo that referenced this pull request Apr 9, 2026
@aimable100
fix(mcp): bump fastmcp>=3.2.1, fix docs and middleware meta stripping
7437c79 
- Bump fastmcp dep to >=3.2.1 (includes PrefectHQ/fastmcp#3778 client
  call_tool error handling for empty/non-text error content)
- Add python_version>=3.10 marker on temporalio dep
- Fix docs/mcp.md: add timestamp arg to warrant.sign() examples,
  correct LangChain adapter pattern to use guard_tools(), fix
  call_protected_tool -> call_tool, clarify extraction result
- Fix fastmcp_middleware: strip tenuo from context-resolved _meta
  when params.meta is None
@aimable100 aimable100 mentioned this pull request Apr 9, 2026
Merged
aimable100 added a commit to tenuo-ai/tenuo that referenced this pull request Apr 9, 2026
@aimable100
fix(mcp): bump fastmcp>=3.2.1, fix docs and middleware meta stripping (…
3129596 
…#341)

Extracts useful changes from the now-stale
`mcp/fastmcp-middleware-client-errors` branch that weren't part of #339
or #340.

- Bump `fastmcp` dep to `>=3.2.1` (includes upstream [client call_tool
error handling](PrefectHQ/fastmcp#3778))
- Add `python_version>=3.10` marker on `temporalio` dep
- Fix `docs/mcp.md`: add timestamp arg to `warrant.sign()` examples,
correct LangChain adapter pattern to use `guard_tools()`, fix
`call_protected_tool` → `call_tool`
- Fix `fastmcp_middleware.py`: strip `tenuo` from context-resolved
`_meta` when `params.meta` is None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@jlowin jlowin jlowin approved these changes

Assignees

No one assigned

Labels

bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. client Related to the FastMCP client SDK or client-side functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aimable100 @jlowin