Skip to content

Commit ab24c34

Browse files
committed
Address review: keep three anticipated failures out of the crash path, docs sweep
- pre_parse_json leaves a string alone when json.loads refuses it with something other than JSONDecodeError (over-long integer, deep nesting), so validation rejects it as a bad argument instead of it surfacing as a crash with a traceback per request. - convert_result skips output-schema validation for a returned CallToolResult(is_error=True); an error result has no structured content to check, and the author's message now reaches the client as written. - read_resource checks that Resource.read() returned str or bytes, so a mistyped custom resource is logged as a crash and answered with -32603 rather than "Invalid request parameters" with no log record. - Docs and examples that still said "raise any exception and the model reads it" now say ToolError; deprecated.md lists the deprecated FuncMetadata helper; docstrings spell out the MCPError carve-out and the nested-crash __cause__. - Two tests tightened: the prompt argument-validation test proves the body never ran, and the invalid-types check asserts on validate_arguments.
1 parent 02a2b92 commit ab24c34

14 files changed

Lines changed: 128 additions & 37 deletions

File tree

docs/deprecated.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Deprecated features
22

3-
The 2026-07-28 spec retires five things. The SDK still implements every one of them, and every one of them now carries a **deprecation warning**.
3+
The 2026-07-28 spec retires five things. The SDK still implements every one of them, and every one of them now carries a **deprecation warning**. One SDK helper is deprecated on its own account and is listed [at the end](#deprecated-sdk-helpers).
44

55
The table below names each deprecated feature, why it is going away, and the replacement to build on.
66

@@ -129,13 +129,22 @@ That is the whole API. There is no per-method switch, and you don't want one: th
129129
One line of pytest configuration, and a deprecated call can never sneak back into your
130130
codebase without failing a test.
131131

132+
## Deprecated SDK helpers
133+
134+
These are not spec changes, only SDK internals with a better replacement. They warn with the same `MCPDeprecationWarning` and will be removed in 3.0.
135+
136+
| Deprecated | What you do instead |
137+
|---|---|
138+
| `FuncMetadata.call_fn_with_arg_validation()` | `FuncMetadata.validate_arguments()` and then `FuncMetadata.call_fn()`. Only code that drives `FuncMetadata` directly (a custom `Tool` subclass, say) ever called it. |
139+
132140
## Recap
133141

134142
* The 2026-07-28 spec deprecates **roots**, server-initiated **sampling**, and protocol **logging** (all [SEP-2577](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2577)), restricts **progress** to server-to-client, and removes **`ping`**.
135143
* The replacement column points you onward: **[Multi-round-trip requests](handlers/multi-round-trip.md)** for sampling and roots, **[Logging](handlers/logging.md)** for logging, **[Progress](handlers/progress.md)** for progress. `ping` needs nothing at all.
136144
* Deprecated is advisory: no wire changes, everything keeps working against pre-2026 sessions, and you get a visible `MCPDeprecationWarning` (a `UserWarning`, so it is on by default).
137145
* Sampling and roots additionally need a back-channel that a 2026-07-28 session does not have. On a modern connection they warn and then they raise.
138146
* `warnings.filterwarnings("ignore", category=MCPDeprecationWarning)` silences the whole category; `"error::mcp.MCPDeprecationWarning"` in pytest turns it into a test failure.
147+
* One SDK helper, `FuncMetadata.call_fn_with_arg_validation()`, is deprecated separately for removal in 3.0.
139148
* New code should not be built on any of these.
140149

141150
Every other page in these docs teaches the current API.

docs/get-started/real-host.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Which means connecting to a host is one act: you tell it **the command that star
66

77
## One server, every host
88

9-
```python title="server.py" hl_lines="3 33-34"
9+
```python title="server.py" hl_lines="4 34-35"
1010
--8<-- "docs_src/real_host/tutorial001.py"
1111
```
1212

docs/get-started/testing.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ There you go! You can now extend your tests to cover more scenarios.
7979
Two different things can go wrong, and this flag only touches one of them.
8080

8181
An exception inside one of **your tools** is not a protocol failure. It becomes a normal result with
82-
`is_error=True`, and the model reads the message. `raise_exceptions` doesn't change that: with or
83-
without it, `call_tool` returns the same `is_error=True` result. There's a whole page on it:
82+
`is_error=True` (and if it was a `ToolError`, the model reads your message). `raise_exceptions` doesn't
83+
change that: with or without it, `call_tool` returns the same `is_error=True` result. There's a whole page on it:
8484
**[Handling errors](../servers/handling-errors.md)**.
8585

8686
A failure **outside** a tool body is different. On the connection `Client(mcp)` gives you, the

docs/migration.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -992,8 +992,8 @@ its behavior is unchanged.
992992
`MCPError` carries `ErrorData` and is the SDK's protocol-error type — raise it
993993
when the request itself should be rejected (missing client capability,
994994
elicitation required, invalid parameters). For tool *execution* failures the
995-
calling LLM should see and react to, raise any other exception or return
996-
`CallToolResult(is_error=True, ...)` directly; that path is unchanged.
995+
calling LLM should see and react to, raise `ToolError` or return
996+
`CallToolResult(is_error=True, ...)` directly.
997997

998998
The client sees this change too. `Client.call_tool()` and
999999
`ClientSession.call_tool()` raise on a JSON-RPC error response, so a tool that

docs/troubleshooting.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ result.structured_content # None
9292

9393
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.
9494

95-
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`.
95+
The bare form, `Error executing tool <name>` with no message, means the tool **crashed**: an exception it didn't anticipate escaped 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`.
9696

9797
## `TypeError: The @tool decorator was used incorrectly. Did you forget to call it? Use @tool() instead of @tool`
9898

docs_src/real_host/tutorial001.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from mcp.server import MCPServer
2+
from mcp.server.mcpserver.exceptions import ToolError
23

34
mcp = MCPServer("Bookshop")
45

@@ -20,7 +21,7 @@ def search_books(query: str) -> list[str]:
2021
def get_author(title: str) -> str:
2122
"""Look up the author of a book in the catalog."""
2223
if title not in CATALOG:
23-
raise ValueError(f"No book titled {title!r} in the catalog.")
24+
raise ToolError(f"No book titled {title!r} in the catalog.")
2425
return CATALOG[title]
2526

2627

examples/servers/everything-server/mcp_everything_server/server.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import click
1414
from mcp.server import ServerRequestContext
1515
from mcp.server.mcpserver import Context, MCPServer, RequestStateSecurity
16+
from mcp.server.mcpserver.exceptions import ToolError
1617
from mcp.server.mcpserver.prompts.base import Prompt, UserMessage
1718
from mcp.server.streamable_http import EventCallback, EventMessage, EventStore
1819
from mcp.shared.exceptions import MCPError
@@ -328,7 +329,7 @@ async def test_elicitation_sep1330_enums(ctx: Context) -> str:
328329
@mcp.tool()
329330
def test_error_handling() -> str:
330331
"""Tests error response handling"""
331-
raise RuntimeError("This tool intentionally returns an error for testing")
332+
raise ToolError("This tool intentionally returns an error for testing")
332333

333334

334335
@mcp.tool()

src/mcp/server/mcpserver/exceptions.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ class ResourceError(MCPServerError):
1111
Raise this from a resource or resource template handler for a failure you saw
1212
coming: the client receives a `-32603` protocol error carrying your message
1313
(`ResourceNotFoundError` below is the `-32602` variant), and the server logs it
14-
at INFO without a traceback. Any other exception is treated as a crash: the
15-
client gets a generic message naming only the URI, and the server logs the
16-
traceback at ERROR.
14+
at INFO without a traceback. Any other exception (bar `MCPError`, which is a
15+
protocol error) is treated as a crash: the client gets a generic message naming
16+
only the URI, and the server logs the traceback at ERROR.
1717
1818
The SDK raises it too, and `UnexpectedResourceError` subclasses it, so
1919
`except ResourceError` around `MCPServer.read_resource()` catches every read
@@ -45,11 +45,12 @@ class ToolError(MCPServerError):
4545
4646
Raise this from a tool (or a resolver) for a failure you saw coming: the
4747
call returns `is_error=True` with your message in `content` for the model to
48-
read, and the server logs it at INFO without a traceback. Any other exception
49-
(bar `MCPError`, which is a protocol error) is treated as a crash: the model
50-
sees only `Error executing tool <name>`, and the server logs the traceback at
51-
ERROR. A `ResourceError` that escapes the tool (say from `ctx.read_resource()`)
52-
counts as anticipated too.
48+
read, and the server logs it at INFO without a traceback. A `ResourceError`
49+
that escapes the tool (say from `ctx.read_resource()`) counts the same. Any
50+
other exception bar `MCPError` (a protocol error) is treated as a crash: the
51+
model sees only `Error executing tool <name>`, and the server logs the
52+
traceback at ERROR. Inside a pydantic validator, raise `ValueError` as pydantic
53+
expects; it arrives as an argument-validation failure, which is anticipated too.
5354
5455
The SDK raises it too, for an unknown tool name and for arguments that fail
5556
the input schema, and `UnexpectedToolError` subclasses it, so `except ToolError`
@@ -58,13 +59,14 @@ class ToolError(MCPServerError):
5859

5960

6061
class UnexpectedToolError(ToolError):
61-
"""A tool call failed with something other than `ToolError` or `MCPError`.
62+
"""A tool call failed with something other than `ToolError`, `ResourceError`, or `MCPError`.
6263
6364
The SDK raises this itself, around a crash in the tool (or a resolver) or a
6465
return value that fails output conversion. You never raise it. The message is
6566
only `Error executing tool <name>` (followed by the same for a nested tool or
6667
resource that crashed), so nothing from the original reaches the client.
67-
`__cause__` is the original exception, which the server logs with its
68+
`__cause__` is the original exception (or, for a nested tool or resource
69+
crash, its `Unexpected...Error` wrapper), which the server logs with its
6870
traceback before returning the `is_error=True` result. Catch it around
6971
`MCPServer.call_tool()` to tell a crash from a deliberate `ToolError`.
7072
"""

src/mcp/server/mcpserver/server.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import inspect
77
from collections.abc import AsyncIterator, Awaitable, Callable, Iterable, Mapping, Sequence
88
from contextlib import AbstractAsyncContextManager, asynccontextmanager
9-
from typing import Any, Generic, Literal, TypeVar, overload
9+
from typing import Any, Generic, Literal, TypeVar, cast, overload
1010

1111
import anyio
1212
import pydantic_core
@@ -524,7 +524,8 @@ async def call_tool(
524524
tool (or a resolver) raises `ToolError` or `ResourceError`.
525525
UnexpectedToolError: If the tool (or a resolver) raises anything else, or
526526
its return value fails output conversion. `__cause__` is the original
527-
exception.
527+
exception (or, for a nested tool or resource crash, its wrapper).
528+
MCPError: Raised by the tool or a resolver; passed through unchanged.
528529
"""
529530
if context is None:
530531
context = Context(mcp_server=self, subscriptions=self._subscriptions)
@@ -580,14 +581,18 @@ async def read_resource(
580581
UnexpectedResourceError: If reading the resource (or creating it from a
581582
template) raises anything other than `ResourceError` or `MCPError`.
582583
`__cause__` is the original exception.
584+
MCPError: Raised by the resource or template function; passed through unchanged.
583585
"""
584586
if context is None:
585587
context = Context(mcp_server=self, subscriptions=self._subscriptions)
586588
try:
587589
resource = await self._resource_manager.get_resource(uri, context)
588590
if isinstance(resource, InputRequiredResult):
589591
return resource
590-
content = await resource.read()
592+
# Checked at runtime because a Resource subclass may not honour the annotation.
593+
content = cast(object, await resource.read())
594+
if not isinstance(content, str | bytes):
595+
raise TypeError(f"Resource.read() must return str or bytes, not {type(content).__name__}")
591596
return [ReadResourceContents(content=content, mime_type=resource.mime_type, meta=resource.meta)]
592597
except (MCPError, ResourceError):
593598
raise

src/mcp/server/mcpserver/utilities/func_metadata.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ def convert_result(self, result: Any) -> CallToolResult | InputRequiredResult:
182182
# A schema published without a model (hand-built metadata) is advertised but not validated here.
183183
output_model = self.output_model if self.output_schema is not None else None
184184
if isinstance(result, CallToolResult):
185-
if output_model is not None:
185+
if output_model is not None and not result.is_error:
186186
self._output_adapter(output_model).validate_python(result.structured_content)
187187
return result
188188

@@ -234,8 +234,10 @@ def pre_parse_json(self, data: dict[str, Any]) -> dict[str, Any]:
234234
if isinstance(data_value, str) and field_info.annotation is not str:
235235
try:
236236
pre_parsed = json.loads(data_value)
237-
except json.JSONDecodeError:
238-
continue # Not JSON - skip
237+
except (ValueError, RecursionError):
238+
# Not JSON, or JSON the parser refuses (over-long integers, deep
239+
# nesting): leave the string for validation to accept or reject.
240+
continue
239241
if isinstance(pre_parsed, str | int | float):
240242
# This is likely that the raw value is e.g. `"hello"` which we
241243
# Should really be parsed as '"hello"' in Python - but if we parse

0 commit comments

Comments
 (0)