Log MCPServer handler exceptions by kind and keep crash details off the wire - #3314
Log MCPServer handler exceptions by kind and keep crash details off the wire#3314maxisbey wants to merge 8 commits into
Conversation
📚 Documentation preview
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/mcp/server/mcpserver/resources/types.py— FileResource.read (and DirectoryResource.read at line 247) now raise UnexpectedResourceError but their docstrings were not given theRaises:section that this same PR added to FunctionResource.read and ResourceTemplate.create_resource.Extended reasoning...
AGENTS.md (Code Quality) requires: "When a public API raises exceptions a caller would reasonably catch, document them in a
Raises:section." A caller reading FileResource.read's docstring ("Read the file content.") has no way to know a missing/unreadable file now surfaces as UnexpectedResourceError with the FileNotFoundError/PermissionError in cause, inconsistent with the sibling resource types updated in the same change.Verification: nit — The claim is factually true. This PR changed FileResource.read (src/mcp/server/mcpserver/resources/types.py:198-199:
raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc) and DirectoryResource.read (lines 252-253, same raise) to raise UnexpectedResourceError, but their docstrings remain "Read the file content." (line 193) and "Read the directory listing." (lin
There was a problem hiding this comment.
Beyond the inline findings, this run also checked the new validation ladder from 6fd5e05 for a deliberate ToolError raised inside a custom argument validator or default_factory: pydantic propagates it raw, so it lands in the except Exception arm and is classified as a crash — but the wire result (prefix, message, is_error=True) is identical to main, so the only effect is the ERROR-vs-INFO log level for a pattern the docstrings don't sanction, and it was ruled out as a finding.
Extended reasoning...
The commits pushed since the prior review (6fd5e05 in particular) address the earlier red-flag finding at tools/base.py:151 — non-ValidationError exceptions from validators are now wrapped as UnexpectedToolError instead of escaping raw. This run's two remaining findings (a Raises-docstring inaccuracy in server.py and a dead arguments argument at the call_fn_with_arg_validation call site) are posted inline and are quality-level, not correctness blockers. The one new candidate investigated this run — a deliberate ToolError from a custom validator being reclassified as a crash — was verified against both HEAD and the base commit: the client-visible text and is_error result are byte-identical, so the difference is confined to the new log-level taxonomy for an out-of-contract raising site. The hunt exited at max_rounds, so approval is off the table regardless; this note only records what else was examined.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
I re-reviewed the latest push and found no new bugs; the points raised in earlier rounds (the prompts/base.py syntax error, the tools/base.py validation ladder, and the two doc inconsistencies) are addressed in the current code, but given this PR reworks the error-handling taxonomy across tools, resources, and prompts, a maintainer review is still worthwhile.
What was reviewed this run:
- The hoisted argument-validation ladder in tools/base.py, including a ToolError/ResourceError raised from a field validator or default_factory (now lands on the explicit crash rung, judged acceptable).
- A ResourceNotFoundError escaping a nested ctx.read_resource() inside a resource handler being forwarded as the outer read's -32602 — pre-existing behavior, not introduced here.
- The corrected uri-templates tip and troubleshooting log-level wording against the code paths they describe.
Extended reasoning...
Overview
This PR reworks the anticipated-vs-crash error taxonomy in the mcpserver layer: src/mcp/server/mcpserver/tools/base.py (argument validation hoisted out of the execution try with a four-rung ladder: ValidationError becomes an anticipated ToolError, MCPError re-raises, everything else becomes UnexpectedToolError), resources/templates.py and resources/types.py (UnexpectedResourceError wrapping), prompts/base.py, exceptions.py, server.py handler logging, plus matching docs (docs/servers/handling-errors.md, docs/servers/uri-templates.md, docs/troubleshooting.md, docs/handlers/logging.md) and extensive test updates in tests/interaction/ and tests/server/mcpserver/.
Security risks
No injection, auth, or crypto surface is touched. The change is security-relevant in one narrow sense: it decides which exception messages reach clients verbatim versus being sanitized to a generic -32603. The current code keeps deliberate ToolError/ResourceError messages flowing to clients (as before) and sanitizes unexpected crashes, which is the safer direction. I found no path where a previously sanitized message becomes exposed.
Level of scrutiny
High. This is a behavioral contract change on a released 2.x line (per AGENTS.md, observable-behavior changes are explicit maintainer decisions), and it drew three prior review rounds from this system with red findings — including a genuine SyntaxError in prompts/base.py:213 on an earlier push. I verified from the current checkout that the syntax error is fixed, the validation ladder now has the MCPError and catch-all rungs my earlier comment said were missing, and both doc pages I flagged now state the log-level and -32603 behavior consistently with the code. One earlier finding (template-parameter ValidationError from validate_call classified as UnexpectedResourceError in templates.py:247-248) remains as-is in code; the author resolved that thread, which reads as a deliberate classification choice rather than an oversight, and I am not re-litigating it — but it is exactly the kind of judgment call a maintainer should confirm.
Other factors
The bug hunt exited on dry_streak with zero findings on this version, and the three candidates investigated this run were all ruled out (two were refinements of the already-addressed validation-ladder concern; the third is pre-existing behavior). Test coverage is thorough — the interaction suite and server tests assert the exact log levels, messages, and error codes for each rung. A maintainer (Kludex) has already engaged on the PR. Approval is not appropriate given the breaking-behavior scope and the author-self-resolved design threads, so a defer acknowledging the fixes and recording what was checked is the right outcome.
maxisbey
changed the title
There was a problem hiding this comment.
All reported issues were addressed across 28 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
3 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/troubleshooting.md">
<violation number="1" location="docs/troubleshooting.md:95">
P2: When a tool raises `ResourceError`, `Tool.run` preserves its message instead of producing the bare crash result. Exclude anticipated `ResourceError` failures from this crash definition while retaining output-schema failures.</violation>
</file>
<file name="src/mcp/server/mcpserver/exceptions.py">
<violation number="1" location="src/mcp/server/mcpserver/exceptions.py:67">
P2: When a tool reads a resource or invokes a nested tool that crashes, `UnexpectedToolError.__cause__` is the nested wrapper, not the original exception. Document the cause chain so callers inspect the underlying failure correctly.</violation>
</file>
<file name="src/mcp/server/mcpserver/server.py">
<violation number="1" location="src/mcp/server/mcpserver/server.py:524">
P3: When a tool or resolver raises `MCPError`, `call_tool` re-raises it, and `UnexpectedResourceError` is converted to `UnexpectedToolError`; neither behavior matches this new Raises block. Clarify these exceptions so callers do not implement the wrong public exception handling.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| The fix is in your client: **check `result.is_error`**. A `try/except` around `call_tool` catches none of these, because there is nothing to catch. This is deliberate, and it is the single most useful thing on this page to internalise: the *model* chose the call, so the model gets the message and a chance to try again. **[Handling errors](servers/handling-errors.md)** is the whole story, including the `MCPError` path that *does* raise. | ||
|
|
||
| The bare form, `Error executing tool <name>` with no message, means the tool **crashed**: something other than `ToolError` was raised while running it (or its return value failed the output schema), and that exception's text is kept off the wire. The traceback is in the **server's log** at `ERROR`, as `Tool '<name>' raised an unexpected exception`. |
There was a problem hiding this comment.
P2: When a tool raises ResourceError, Tool.run preserves its message instead of producing the bare crash result. Exclude anticipated ResourceError failures from this crash definition while retaining output-schema failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/troubleshooting.md, line 95:
<comment>When a tool raises `ResourceError`, `Tool.run` preserves its message instead of producing the bare crash result. Exclude anticipated `ResourceError` failures from this crash definition while retaining output-schema failures.</comment>
<file context>
@@ -92,7 +92,7 @@ result.structured_content # None
The fix is in your client: **check `result.is_error`**. A `try/except` around `call_tool` catches none of these, because there is nothing to catch. This is deliberate, and it is the single most useful thing on this page to internalise: the *model* chose the call, so the model gets the message and a chance to try again. **[Handling errors](servers/handling-errors.md)** is the whole story, including the `MCPError` path that *does* raise.
-The bare form, `Error executing tool <name>` with no message, means the tool **crashed**: it raised something other than `ToolError`, and the exception's text is kept off the wire. The traceback is in the **server's log** at `ERROR`, as `Tool '<name>' raised an unexpected exception`.
+The bare form, `Error executing tool <name>` with no message, means the tool **crashed**: something other than `ToolError` was raised while running it (or its return value failed the output schema), and that exception's text is kept off the wire. The traceback is in the **server's log** at `ERROR`, as `Tool '<name>' raised an unexpected exception`.
## `TypeError: The @tool decorator was used incorrectly. Did you forget to call it? Use @tool() instead of @tool`
</file context>
| return value that fails output conversion. You never raise it. The message is | ||
| only `Error executing tool <name>` (followed by the same for a nested tool or | ||
| resource that crashed), so nothing from the original reaches the client. | ||
| `__cause__` is the original exception, which the server logs with its |
There was a problem hiding this comment.
P2: When a tool reads a resource or invokes a nested tool that crashes, UnexpectedToolError.__cause__ is the nested wrapper, not the original exception. Document the cause chain so callers inspect the underlying failure correctly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/mcpserver/exceptions.py, line 67:
<comment>When a tool reads a resource or invokes a nested tool that crashes, `UnexpectedToolError.__cause__` is the nested wrapper, not the original exception. Document the cause chain so callers inspect the underlying failure correctly.</comment>
<file context>
@@ -61,8 +62,9 @@ class UnexpectedToolError(ToolError):
- client. `__cause__` is the original exception, which the server logs with its
+ only `Error executing tool <name>` (followed by the same for a nested tool or
+ resource that crashed), so nothing from the original reaches the client.
+ `__cause__` is the original exception, which the server logs with its
traceback before returning the `is_error=True` result. Catch it around
`MCPServer.call_tool()` to tell a crash from a deliberate `ToolError`.
</file context>
| tool (or a resolver) raises `ToolError` or `ResourceError`. | ||
| UnexpectedToolError: If the tool (or a resolver) raises anything else, or | ||
| its return value fails output conversion. `__cause__` is the original | ||
| exception. |
There was a problem hiding this comment.
P3: When a tool or resolver raises MCPError, call_tool re-raises it, and UnexpectedResourceError is converted to UnexpectedToolError; neither behavior matches this new Raises block. Clarify these exceptions so callers do not implement the wrong public exception handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/mcpserver/server.py, line 524:
<comment>When a tool or resolver raises `MCPError`, `call_tool` re-raises it, and `UnexpectedResourceError` is converted to `UnexpectedToolError`; neither behavior matches this new Raises block. Clarify these exceptions so callers do not implement the wrong public exception handling.</comment>
<file context>
@@ -521,10 +521,10 @@ async def call_tool(
- UnexpectedToolError: If the tool (or a resolver) raises anything other than
- `ToolError` or `MCPError`, or its return value fails output conversion.
- `__cause__` is the original exception.
+ tool (or a resolver) raises `ToolError` or `ResourceError`.
+ UnexpectedToolError: If the tool (or a resolver) raises anything else, or
+ its return value fails output conversion. `__cause__` is the original
</file context>
| tool (or a resolver) raises `ToolError` or `ResourceError`. | |
| UnexpectedToolError: If the tool (or a resolver) raises anything else, or | |
| its return value fails output conversion. `__cause__` is the original | |
| exception. | |
| tool (or a resolver) raises anticipated `ToolError` or `ResourceError`. | |
| UnexpectedToolError: If argument validation, the tool, or a resolver raises an | |
| unexpected exception (including `UnexpectedResourceError`), or its return | |
| value fails output conversion. `MCPError` is re-raised unchanged. |
There was a problem hiding this comment.
Beyond the inline finding, this pass also examined and ruled out: the direct ctx.elicit() path vs the Resolve(Elicit) resolver's new ValueError→ToolError conversion; a deliberate ToolError raised inside a custom argument validator or default_factory (the validation-phase except Exception → UnexpectedToolError in Tool.run is documented in-code as intentionally treating those as crashes); and Prompt.render dropping the : {e} detail from its rendering-failure message.
Extended reasoning...
This run produced one confirmed inline finding (the contents-conversion loop in _handle_read_resource at src/mcp/server/mcpserver/server.py:462 sits outside the new ResourceError-classifying try), so approval is not appropriate. The body is limited to the once-per-PR ruled-out note covering only candidates newly investigated this run: the direct ctx.elicit() classification question adjacent to the resolve.py fix, the ToolError-from-validator reclassification in the hoisted validation ladder (tools/base.py:149-159, where the code comment states a validator exception is deliberately a crash), and the prompt-render message detail in prompts/base.py. None of these repeats my previously posted comments (which covered the Sample/ListRoots legacy path, the completion-result construction, and the argument-rejection log formatting — the latest commit addressed the last two).
| raise MCPError(code=code, message=str(err), data={"uri": str(params.uri)}) | ||
| if isinstance(results, InputRequiredResult): | ||
| return results | ||
| contents: list[TextResourceContents | BlobResourceContents] = [] |
There was a problem hiding this comment.
🟣 Pre-existing, in code this diff rewrote: _handle_read_resource's contents-conversion loop runs outside the new ResourceError-classifying try (lines 451-459), so a custom Resource subclass whose read() returns a non-str/bytes value raises pydantic ValidationError from TextResourceContents(...) after the except block; the dispatcher's shared ladder maps ValidationError to -32602 "Invalid request parameters" with no server-side log record. This is the same result-construction-outside-the-try silent-failure the author fixed for the completion handler (commit 3b94285 moved CompleteResult inside its try), left unfixed for resources/read.
Extended reasoning...
An author registers a hand-written resource, e.g. mcp.add_resource(StatsResource(uri="stats://all", name="stats")) whose async def read(self) returns {"count": 3} — a natural mistake since decorated @ mcp.resource functions get automatic JSON dumping via FunctionResource. read_resource() succeeds (nothing raised), then TextResourceContents(uri=..., text={"count": 3}, ...) at server.py:474-481 raises ValidationError outside the try. On both dispatch paths (jsonrpc_dispatcher.handler_exception_to_error_data lines 100-101, runner.py on_request line 793) ValidationError is mapped straight to JSON-RPC -32602 "Invalid request parameters" with empty data and is never logged. Every read of that URI tells the client its request was malformed (the request was fine), and the server log contains neither "Resource ... raised an unexpected exception" nor any other record at any level — the author follows docs/servers/handling-errors.md, greps the log for the documented ERROR record, finds nothing, and has no way to discover the real cause, which is exactly the unlogged-failure class th
Verification: pre-existing — the defective lines predate this diff, but the diff rewrote the error classification directly above them in the same function, so it is flaggable. The failure chain is real and reachable: (1) a hand-written Resource subclass registered via add_resource() whose read() returns e.g. a dict raises nothing — MCPServer.read_resource (src/mcp/server/mcpserver/server.py:590-591) wraps it in
A crashing tool used to leave no server-side trace: _handle_call_tool
turned the exception into an is_error result before the dispatcher
boundary could log it, so a KeyError('id') reached the model as "'id'"
and its traceback existed nowhere. Resources logged once and prompts
twice. Tool.run also re-wrapped a deliberate ToolError, so nothing
downstream could tell an anticipated failure from a crash.
Tool.run now validates arguments first (a schema rejection is a plain
ToolError chained to the ValidationError) and runs the body under an
except ladder that keeps the distinction in the type: a deliberate
ToolError stays a ToolError, anything else becomes the new
UnexpectedToolError. Both keep the "Error executing tool X: " text, so
results are byte-identical. Resources get the matching
UnexpectedResourceError, raised by whichever layer first sees the
foreign exception so __cause__ is always the original.
_log_handler_exception in server.py is the one place tools and
resources are logged: INFO without a traceback for ToolError and
ResourceError (deliberate, unknown name, bad arguments, not found),
ERROR with the traceback for anything else. get_prompt stops logging,
leaving the dispatcher boundary's record as the only one.
ResourceError raised from a static resource now passes through to the
client as it already did from a template.
Log at the two handler sites directly instead of through a shared helper: the tool site checks for ToolError, the resource site only has to ask whether it caught an UnexpectedResourceError. Drop the three transport-matrix logging tests and their requirement ids from the interaction suite, which is for wire behaviour; the same properties are covered next to MCPServer in test_server.py. Shorten the logging docs to a pointer, reword the handling-errors section plainly, and drop the recap bullet and prompt caveats.
A custom argument validator that raises something other than ValidationError escaped Tool.run unwrapped, losing the "Error executing tool" prefix and the UnexpectedToolError type. It is now wrapped as a crash, and an MCPError raised there still passes through. A ResourceError (usually ResourceNotFoundError from ctx.read_resource) that escapes a tool body is now classified like a ToolError, since it is the same anticipated outcome resources/read logs at INFO. An UnexpectedResourceError escaping a tool stays a crash. MCPServer.read_resource is now the single place a resource crash is wrapped (plus create_resource for templates), so the built-in Resource types let the original exception propagate to direct callers. Also: trimmed raise-site comments in favour of the exception docstrings, reworded the ToolError and ResourceError docstrings, documented the FunctionResource/FileResource.read change in migration.md, corrected the uri-templates tip and example, and pinned the new cases in tests (including a wire test for ResourceNotFoundError from a static resource).
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
The applied suggestion dropped the closing quote along with the interpolated exception text, so prompts/base.py no longer parsed. With the message now just "Error rendering prompt <name>", the legacy-path interaction test snapshots that instead of matching the pydantic prefix.
Keep the one-word correction to the SEP-2164 sentence (static resources now pass ResourceNotFoundError through too), remove the added clause about FunctionResource.read()/FileResource.read(). No-Verification-Needed: docs-only change
A tool that crashed used to send the exception's own text to the client as "Error executing tool <name>: <str(exc)>". That text can describe server internals (or, for an output-schema failure, echo the tool's return value), so a crash now reads just "Error executing tool <name>". ToolError, ResourceError, and argument-validation messages still reach the model unchanged, since those are the anticipated failures it can act on. Closes the tool half of the leak that resources already avoided and that prompts stopped doing earlier in this branch. Related tidy-ups in the same direction: - a crashing @mcp.completion() handler is logged once and answered with -32603 "Error completing argument <name>" instead of str(exc) - the legacy resolver path reports a malformed elicitation answer as a ToolError, matching what the input_required path already did - the INFO line for rejected arguments names the fields, not the values Docs now teach ToolError as the way to talk to the model and describe a plain exception as a crash the model sees generically; examples that relied on ValueError text reaching the client raise ToolError instead.
…sult, add call_fn - Log rejected tool arguments with %r: pydantic's error locations can include caller-supplied dict keys, which must not break onto new log lines. - Build CompleteResult inside the completion adapter's try, so a handler returning the wrong type is logged as a crash and answered with the generic -32603 rather than "Invalid request parameters". - On the legacy resolver path, a malformed ElicitResult from a non-conformant client no longer has its pydantic text repeated back. - Add FuncMetadata.call_fn() for calling with already-validated arguments and use it from Tool.run; call_fn_with_arg_validation() becomes a deprecated wrapper (MCPDeprecationWarning, removal in 3.0). - Docstring and docs wording: MCPError carve-outs, nested crash message, ResourceError in the imports and resource paragraph, the exact MCPDeprecationWarning path a traceback prints.
3b94285 to
02a2b92
Compare
|
|
||
| * **Sync functions run on a worker thread.** A `def` tool (or resource, prompt, or resolver) no longer blocks the event loop; the trade is that its body no longer runs *on* the event-loop thread, which matters to thread-affine code. `async def` handlers are untouched. **[Migration Guide](migration.md#sync-handler-functions-now-run-on-a-worker-thread)**. | ||
| * **`MCPError` (v1's `McpError`) raised inside a tool is a protocol error now.** The model never sees it. Every other exception still becomes an `is_error=True` result the model can read and react to. **[Handling errors](servers/handling-errors.md)** is the split. | ||
| * **`MCPError` (v1's `McpError`) raised inside a tool is a protocol error now.** The model never sees it. Every other exception still becomes an `is_error=True` result, but only a `ToolError`'s message reaches the model: any other exception now reads `Error executing tool <name>`, with the traceback in your server log. **[Handling errors](servers/handling-errors.md)** is the split. |
There was a problem hiding this comment.
🟡 nit: The crash message-stripping this PR introduces (only a ToolError's message reaches the model; any other exception yields the bare 'Error executing tool ') was propagated to whats-new.md, handling-errors.md, migration.md, client/index.md and troubleshooting.md, but docs/get-started/testing.md:81-82 was missed and still teaches the old contract: 'An exception inside one of your tools ... becomes a normal result with is_error=True, and the model reads the message.' After this change the model reads the message only for ToolError; every other exception's text is withheld from content.
Extended reasoning...
A new user on the get-started Testing page (the primary onboarding path) follows its claim, raises a plain ValueError('No forecast for Paris') in a tool expecting the model to read the message and self-correct — exactly the pattern this page describes — and either ships a server whose model-facing errors are all the uninformative 'Error executing tool ', or writes the page's suggested test asserting the exception text appears in result.content and watches it fail with no explanation on the page they were following. The corrected story exists only on pages this PR edited; the page that tells users what a failing tool looks like in a test was left contradicting the shipped behavior.
Verification: nit — The behavior change is real and the cited doc page is genuinely stale. This PR rewrote src/mcp/server/mcpserver/tools/base.py so a non-ToolError exception is re-raised as UnexpectedToolError(f"Error executing tool {self.name}") from exc — the old code was raise ToolError(f"Error executing tool {self.name}: {e}") from e, which carried the exception text to the model. The new contract
| @deprecated( | ||
| "FuncMetadata.call_fn_with_arg_validation() is deprecated and will be removed in 3.0; " | ||
| "call validate_arguments() and then call_fn() instead.", | ||
| category=MCPDeprecationWarning, | ||
| ) |
There was a problem hiding this comment.
🟡 nit: New runtime deprecation of FuncMetadata.call_fn_with_arg_validation (emits mcp.MCPDeprecationWarning, "removed in 3.0") is not documented on docs/deprecated.md, which the docs elsewhere (docs/migration.md ~line 2739) present as "the full list" of MCPDeprecationWarning emitters; AGENTS.md requires the relevant docs page to be updated in the same PR as a user-visible API change.
Extended reasoning...
A 2.x user whose code calls meta.call_fn_with_arg_validation() upgrades and starts seeing mcp.MCPDeprecationWarning on every call (visible by default since the category subclasses UserWarning), or their test suite fails outright under the documented error::mcp.MCPDeprecationWarning / filterwarnings = ["error"] recipe. Following the docs, they open docs/deprecated.md — described in migration.md as having "the full list and each replacement" — and find no entry for this method, so the deprecation page contradicts the SDK and gives them no pointer to the validate_arguments()+call_fn() replacement (which only exists inside the warning text itself).
Verification: nit. The factual claims all check out. (1) The diff adds a new runtime deprecation at src/mcp/server/mcpserver/utilities/func_metadata.py:146-150: @ deprecated("FuncMetadata.call_fn_with_arg_validation() is deprecated and will be removed in 3.0; call validate_arguments() and then call_fn() instead.", category=MCPDeprecationWarning) — an SDK-level deprecation, visible by default since `MCPDepre
2 participants
MCPServer now treats an exception from a tool, resource, or prompt handler in one of two ways, decided by its type:
ToolError,ResourceError/ResourceNotFoundError, a schema rejection of the arguments, an unknown name): the message reaches the client as before, and the server writes oneINFOrecord with no traceback.Error executing tool <name>,Error reading resource <uri>,Error rendering prompt <name>), and the server writes oneERRORrecord with the traceback. Nothing from the exception's text goes on the wire, and nothing is logged twice.Fixes #3266. Fixes #698.
Motivation and Context
On
mainthe three primitives each hand-roll their ownexceptladder and each made a different choice:is_error=True,"Error executing tool X: {e}"— the raw exception textERROR+ traceback, once-32603 "Error reading resource {uri}"(text withheld)ERROR+ traceback, twicecode=0with the raw text; modern:Internal server errorTwo problems in that table. A crashing tool leaves no trace on the server (#3266): for a
KeyError('id')the model reads'id'and the traceback exists nowhere. And a crashing tool or prompt sendsstr(exc)to the client (#698), which can describe server internals; for an output-schema failure it echoes the tool's return value.Rather than an eighth site-specific patch (#3267 / #3271 / #2198), the exception's type now carries "was this anticipated?" to one decision point per primitive:
Tool.runvalidates arguments first (a schema rejection is a plainToolErrorchained to theValidationError; a validator that raises anything else is a crash). The body then runs under an except ladder:ToolErrororResourceError(from the tool, a resolver, orctx.read_resource()) is re-raised asToolErrorwith its message; anything else becomes the newUnexpectedToolError(ToolError)whose message is onlyError executing tool <name>and whose__cause__is the original._handle_call_tool/_handle_read_resourcelog at the point the failure becomes a response:INFOfor the anticipated types (rejected arguments log field names, not values),logger.exceptionfor theUnexpected*wrappers.UnexpectedResourceError(ResourceError), raised inMCPServer.read_resource(andResourceTemplate.create_resource), so__cause__is always the original. Built-inResource.read()implementations no longer wrap.logger.exceptioninget_prompt(the dispatcher boundary's record is the only one), andPrompt.renderno longer interpolates the exception text into its message.@mcp.completion()gets the same treatment: a crash is oneERRORrecord and-32603 "Error completing argument <name>".FuncMetadata.call_fn()is the new "call with already-validated arguments" stepTool.runuses;call_fn_with_arg_validation()remains as a thin wrapper, deprecated withMCPDeprecationWarningfor removal in 3.0.Why level, not "log everything at ERROR": level is the one filter operators get for free and what Sentry/Datadog integrations key on. External FastMCP shipped
logger.exceptionfor every tool failure and walked it back over PrefectHQ/fastmcp#4036, #4029, #4392 once deliberateToolErrors and model typos flooded error monitoring. #2422 and #2346 are the same signal here.Why withhold crash text: it's the call already made for resources (#1957) and prompts, it's what #698 / #2386 ask for, and it's the default in every framework surveyed (Starlette/uvicorn, Flask, Django, gRPC-java, the C# SDK). Model self-correction is preserved because the two channels the model can act on,
ToolErrorand argument-validation text, still pass through.Client-visible changes
ToolError/ResourceError/MCPError→contentisError executing tool <name>(was…: <str(exc)>). Same for a crashing resolver, a crashing validator, and an output-schema failure.Prompt.renderfailure on the legacy path →Error rendering prompt <name>(was…: <str(exc)>).@mcp.completion()crash →-32603 Error completing argument <name>(legacy path wascode=0, str(exc)).ResourceError/ResourceNotFoundErrorfrom a static resource now pass through (-32602/ your message) as they already did from a template. Also visible one level up when a tool reads such a resource viactx.read_resource().Not in here
code=0, str(e)for any other unmapped handler exception on 2025-era transports, lowlevelServerincluded) is unchanged; it has its own TODO /protocol:error:internal-errordivergence and a wider blast radius.validate_call, which fuses validation with the call. Not a regression. Follow-up.Error executing tool X:prefix for a deliberateToolError(feat(mcpserver): let ToolError carry content for is_error results #2984 territory).MCPServer's defaultRichHandlerrenders a crash as 100+ stderr lines at 80 columns under a stdio host; tool crashes now join resources/prompts there. Separate conversation.How Has This Been Tested?
tests/server/mcpserver/test_server.py: level, message, traceback identity and wire result per class — crash,ToolError,ToolErrorsubclass, bad arguments (INFO names fields;__cause__is theValidationError), validator crash vs validatorMCPError,ValidationErrorinside the body / output-schema failure (crashes), unknown tool,MCPError(no record), resolverToolErrorvs crash,ResourceNotFoundErrorvs resource crash escaping a tool, a tool that recovers from a missing resource (nothing logged), static / template / custom-subclass resource crash, staticResourceNotFoundError, deliberateResourceError, completion crash vsMCPError, prompt crash logged once, nested tool crash, and the directcall_tool()/read_resource()type and__cause__contracts.ResourceNotFoundError→-32602.tests/docs_src/*: every rewritten docs claim is exercised.log_level="WARNING"leaves only the crash records../scripts/test: 100% coverage,strict-no-cover, pyright, pre-commit clean.Breaking Changes
The client-visible changes listed above.
docs/servers/handling-errors.mdnow teachesToolErroras the way to hand the model a message; code that raised a plain exception expecting the model to read its text should switch toToolError. The new exception types subclass the existing ones, soexcept ToolError/except ResourceErrorand the documentedRaises:contracts keep working. Softer differences:FunctionResource.read()/FileResource.read()called directly raise the original exception instead of aValueError.MCPServer.read_resource()/get_prompt()no longer log by themselves.FuncMetadata.call_fn_with_arg_validation()warns (MCPDeprecationWarning) and is slated for 3.0. Log message wording changed.Types of changes
Checklist
Additional context
Supersedes #3267, #3271, and #2198 (thank you all — the diagnoses were right; this moves the fix to where all three primitives share it). Related: #2153, #2386, #2422.
AI Disclaimer