fix: log expected tool failures as WARNING, drop tracebacks#4029
fix: log expected tool failures as WARNING, drop tracebacks#4029sergeykad wants to merge 1 commit intoPrefectHQ:mainfrom
Conversation
FastMCPError and Pydantic ValidationError are expected control flow, not server bugs. Log them as WARNING with the relevant payload and no traceback. Bare `except Exception` keeps logger.exception so real bugs still get a stack. Drop fastmcp.ValidationError from the except tuple: already caught by the FastMCPError branch above.
marvin-context-protocol
Bot
added
bug
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
ZLeventer
left a comment
ZLeventer
left a comment
There was a problem hiding this comment.
Makes sense — expected tool failures (like validation errors or user-facing error responses) shouldn't produce full tracebacks in the logs. WARNING level is appropriate since the tool did fail, but it's an expected failure path rather than a server error.
One thought: it might be worth distinguishing between "tool returned an error response" (WARNING, no traceback) vs "tool raised an unexpected exception" (ERROR, with traceback). If this PR already makes that distinction, looks good. If it suppresses tracebacks for all tool failures uniformly, unexpected bugs in tool implementations could become harder to debug.
|
I didn't open an issue for this. |
jlowin
left a comment
jlowin
left a comment
There was a problem hiding this comment.
Ok, for the future we typically expect every PR to have an associated issue to describe the work. #4036 is merged and includes a per-error log_level so users can downgrade to WARNING themselves. What's still useful is dropping exc_info to hide tracebacks and the unreachable ValidationError cleanup - would you want to rescope to that?
3 participants
Description
Tool dispatch in
src/fastmcp/server/server.pylogs every failure vialogger.exception, printing a full traceback to stderr. Three cases land in those branches, but only one is an actual bug:except FastMCPError: a tool deliberately raisedToolError(or similar) to return a structured error to the client. The exception message already carries that payload; the stack through fastmcp's dispatcher adds nothing.except (ValidationError, PydanticValidationError): the caller passed bad arguments.err.errors()already describes the failing field, expected type, and offending value. The stack walks Pydantic internals.except Exception: an unexpected error in tool code. This one benefits from a stack.On stdio deployments the tracebacks from the first two cases are written to the parent process's stderr on every malformed caller request or tool-raised error. For a long-running server with a misbehaving agent this floods the console.
This PR logs the first two categories as
WARNINGwith the relevant payload and drops the traceback. Theexcept Exceptionbranch is untouched. Pydantic URLs are stripped viainclude_url=False.Also drops
fastmcp.exceptions.ValidationErrorfrom the secondexcepttuple. It is aFastMCPErrorsubclass and is already caught by the branch above, so that entry was unreachable. Removing it lets the branch type-check as a singlePydanticValidationError.Closes #
Contribution type
Checklist
uv run prek run --all-filesand all checks pass