Fix unhandled exceptions in OpenAPI POST tool calls#3133
Conversation
Harden body construction in _unflatten_arguments() to guard against empty content_schema dicts, and add a catch-all in OpenAPITool.run() so unexpected errors during request building produce actionable messages instead of generic "Internal Server Error". Closes #3113
WalkthroughDocumentation anchors were updated to reflect new source line numbers. In src/fastmcp/server/providers/openapi/components.py, OpenAPITool.run now wraps request construction in a try/except and raises a ValueError containing HTTP method, path, exception type, and message if construction fails; the subsequent request send/response handling remains separate. In src/fastmcp/utilities/openapi/director.py, request-body construction now checks that 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd3c3ccf4e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception as e: | ||
| raise ValueError( | ||
| f"Error building request for {self._route.method.upper()} " | ||
| f"{self._route.path}: {type(e).__name__}: {e}" |
There was a problem hiding this comment.
Limit catch-all to request-building failures
The new except Exception in OpenAPITool.run catches non-HTTPX errors from the entire method, not just request construction, so failures that happen after send() (for example response parsing or ToolResult validation) are now rethrown as Error building request ..., which is misleading and makes production debugging harder. Please scope this handler to the build step or use a message that matches all covered phases.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| ### `OpenAPIResource` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L222" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup> | ||
| ### `OpenAPIResource` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L228" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup> |
There was a problem hiding this comment.
Remove edits under auto-generated python-sdk docs
This change modifies docs/python-sdk/**, but the repository guideline in /workspace/fastmcp/AGENTS.md states "Never modify docs/python-sdk/** (auto-generated)." Committing these generated line-reference updates creates avoidable churn and risks stale docs diffs being overwritten by generation tooling.
Useful? React with 👍 / 👎.
The previous except Exception covered the entire try block including the HTTP send and response handling, producing misleading "Error building request" messages for post-send failures. Now request building has its own try/except and HTTP failures fall through to the specific httpx exception handlers.
|
hi @jlowin how can i test this change with 2.14. version can this be released in 2.14.6? |
|
This went out in 3.0 rc1 |
OpenAPITool.run()only catcheshttpx.*exceptions, so any failure during request building (parameter routing, body construction) escapes unhandled and surfaces to clients as a generic "Internal Server Error" with no diagnostic info. This particularly affects POST requests where the body construction code exercises more fragile paths.Two changes: harden
_unflatten_arguments()to guard against emptycontent_schemadicts and non-dict body schemas, and add a catch-all inrun()that wraps unexpected errors with the tool's method, path, and original error — so users see something like"Error building request for POST /items: KeyError: missing_param"instead of a silent 500.Closes #3113