Skip to content

fix: check the cause of the tool error#2674

Merged
jlowin merged 1 commit intoPrefectHQ:mainfrom
rjolaverria:fix/error-handling-middleware-transform-cause
Dec 22, 2025
Merged

fix: check the cause of the tool error#2674
jlowin merged 1 commit intoPrefectHQ:mainfrom
rjolaverria:fix/error-handling-middleware-transform-cause

Conversation

@rjolaverria
Copy link
Copy Markdown
Contributor

Description

This PR updates ErrorHandlingMiddleware._transform_error to check the Exception.__cause__ when ToolErrors are raised. The server re-raises Exceptions as ToolErrors when executing the tools; the middleware was not transforming the errors correctly since its expecting the __cause__ of the tool errors.
Contributors Checklist

Review Checklist

  • I have self-reviewed my changes
  • My Pull Request is ready for review

@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. labels Dec 22, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 22, 2025

Walkthrough

The error handling middleware was modified to inspect the underlying exception cause when determining the error type for MCP transformation. The change examines error.__cause__ if present before falling back to the direct error type. This enables the error mapping logic to use the original exception type when exceptions are wrapped with raise ... from ... syntax, potentially producing more accurate MCP error classifications based on the root cause rather than the wrapper exception.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: check the cause of the tool error' accurately describes the main code change—inspecting Exception.cause in error transformation logic.
Description check ✅ Passed The description clearly explains the issue, solution, and includes all required checklist items marked complete with proper context about the fix.
Linked Issues check ✅ Passed The PR fixes issue #2673 by modifying _transform_error to check Exception.cause, which enables proper MCP error mapping for wrapped exceptions rather than always returning generic internal errors.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to fix the error handling middleware's exception transformation logic, with no extraneous modifications outside the stated objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/fastmcp/server/middleware/error_handling.py (1)

90-90: LGTM! The fix correctly addresses the error transformation issue.

The change properly inspects error.__cause__ to determine the error type for classification, which resolves the issue where wrapped exceptions (e.g., ValueError wrapped in ToolError) were being misclassified. This enables the middleware to map the underlying exception to the appropriate MCP error code.

Optional consideration for future enhancement:

The error messages still use str(error) (lines 94, 98, 102, 107, 111) rather than str(error.__cause__). This means when classifying based on the cause, the displayed message comes from the wrapper exception. This may be intentional to preserve tool context, but it's worth verifying this is the desired behavior.

Additionally, the implementation only inspects one level of __cause__. If there are deeper exception chains, the root cause won't be reached. For the current use case this appears sufficient.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 748d25e and df18f28.

⛔ Files ignored due to path filters (1)
  • tests/server/middleware/test_error_handling.py is excluded by none and included by none
📒 Files selected for processing (1)
  • src/fastmcp/server/middleware/error_handling.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bare except - be specific with exception types

Files:

  • src/fastmcp/server/middleware/error_handling.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Run tests: Python 3.10 on windows-latest
  • GitHub Check: Run tests: Python 3.13 on ubuntu-latest
  • GitHub Check: Run tests with lowest-direct dependencies
  • GitHub Check: Run tests: Python 3.10 on ubuntu-latest

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!

@jlowin jlowin merged commit 820b69a into PrefectHQ:main Dec 22, 2025
11 checks passed
@rjolaverria rjolaverria deleted the fix/error-handling-middleware-transform-cause branch December 22, 2025 16:49
jlowin pushed a commit that referenced this pull request Dec 23, 2025
jlowin pushed a commit that referenced this pull request Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ErrorHandlingMiddleware if not transforming errors to MCP

2 participants