Skip to content

Commit 02a2b92

Browse files
committed
Address review: repr rejected-argument names, guard the completion result, 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.
1 parent ab89da8 commit 02a2b92

10 files changed

Lines changed: 156 additions & 99 deletions

File tree

docs/deprecated.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ That is the whole API. There is no per-method switch, and you don't want one: th
123123
`Error executing tool old_log`, and the captured server log names the culprit:
124124

125125
```text
126-
mcp.MCPDeprecationWarning: The logging capability is deprecated as of 2026-07-28 (SEP-2577).
126+
mcp.shared.exceptions.MCPDeprecationWarning: The logging capability is deprecated as of 2026-07-28 (SEP-2577).
127127
```
128128

129129
One line of pytest configuration, and a deprecated call can never sneak back into your

docs/servers/handling-errors.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ When it can't, raise `ResourceNotFoundError`. The SDK turns it into the protocol
125125
}
126126
```
127127

128-
Notice there is no `is_error=True` half-result here. A resource read either returns contents or fails: resources have only the protocol path. `ResourceError` is the same thing for a failure that isn't "not found" (`-32603`, your message). Any other exception is a crash: the client gets `-32603` naming only the URI, and the traceback goes to your log at `ERROR`. Templates and everything else about resources live in **[Resources](resources.md)**.
128+
Notice there is no `is_error=True` half-result here. A resource read either returns contents or fails: resources have only the protocol path. `ResourceError` is the same thing for a failure that isn't "not found" (`-32603`, your message), and both are one `INFO` line in your log. Any other exception bar `MCPError` is a crash: the client gets `-32603` naming only the URI, and the traceback goes to your log at `ERROR`. Templates and everything else about resources live in **[Resources](resources.md)**.
129129

130130
## Errors you never raise
131131

@@ -136,8 +136,8 @@ Send `get_author` a `title` that isn't a string and the SDK rejects it against t
136136
It means a whole class of `raise` statements you don't write: don't re-validate your own type hints.
137137

138138
!!! info
139-
Everything on this page is what a **client** sees, and the in-memory `Client` you'll write
140-
tests with sees exactly the same thing. Even `raise_exceptions=True` doesn't hand a failing
139+
Everything a **client** sees on this page, the in-memory `Client` you'll write tests with
140+
sees too. Even `raise_exceptions=True` doesn't hand a failing
141141
tool's exception back to the caller: by the time that flag could act, your exception is already
142142
the `is_error=True` result. Assert on the result. If you need the traceback of a crash, it is in
143143
the server's log, and pytest's `caplog` captures it. **[Testing](../get-started/testing.md)** covers the pattern.
@@ -150,7 +150,7 @@ It means a whole class of `raise` statements you don't write: don't re-validate
150150
* Any **other exception** is a crash -> `is_error=True` with only `Error executing tool <name>` for the model, and an `ERROR` record with the traceback for you.
151151
* `ResourceNotFoundError` from a resource handler -> the protocol's `-32602`, with the URI in `data`.
152152
* Bad arguments are rejected against the schema before your function runs; you don't `raise` for those.
153-
* Imports: `from mcp import MCPError`, `from mcp.server.mcpserver.exceptions import ToolError, ResourceNotFoundError`, and the error-code constants from `mcp.types`.
153+
* Imports: `from mcp import MCPError`, `from mcp.server.mcpserver.exceptions import ToolError, ResourceError, ResourceNotFoundError`, and the error-code constants from `mcp.types`.
154154

155155
Errors handled. That is everything a server *exposes*. What every handler can read, and do back to the client while it runs, is the next section: **[Inside your handler](../handlers/index.md)**.
156156

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**: 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`.
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`.
9696

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

src/mcp/server/mcpserver/exceptions.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,10 @@ class ToolError(MCPServerError):
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
4848
read, and the server logs it at INFO without a traceback. Any other exception
49-
is treated as a crash: the model sees only `Error executing tool <name>`, and
50-
the server logs the traceback at ERROR. A `ResourceError` that escapes the tool
51-
(say from `ctx.read_resource()`) counts as anticipated too.
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.
5253
5354
The SDK raises it too, for an unknown tool name and for arguments that fail
5455
the input schema, and `UnexpectedToolError` subclasses it, so `except ToolError`
@@ -61,8 +62,9 @@ class UnexpectedToolError(ToolError):
6162
6263
The SDK raises this itself, around a crash in the tool (or a resolver) or a
6364
return value that fails output conversion. You never raise it. The message is
64-
only `Error executing tool <name>`, so nothing from the original reaches the
65-
client. `__cause__` is the original exception, which the server logs with its
65+
only `Error executing tool <name>` (followed by the same for a nested tool or
66+
resource that crashed), so nothing from the original reaches the client.
67+
`__cause__` is the original exception, which the server logs with its
6668
traceback before returning the `is_error=True` result. Catch it around
6769
`MCPServer.call_tool()` to tell a crash from a deliberate `ToolError`.
6870
"""

src/mcp/server/mcpserver/resolve.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,10 @@ async def _fulfil(marker: _Marker, key: str, res: _Resolution) -> ElicitationRes
580580
except ValueError as e:
581581
# Accepted with no content, or content that fails the schema: the same
582582
# client mistake the input_required path below reports as a ToolError.
583-
raise ToolError(f"Resolver {key!r}: {e}") from e
583+
# (A pydantic ValidationError here means a non-conformant client sent a
584+
# malformed ElicitResult; its text is not repeated back.)
585+
detail = "received an invalid elicitation response" if isinstance(e, ValidationError) else str(e)
586+
raise ToolError(f"Resolver {key!r}: {detail}") from e
584587
result = await res.context.session.send_request(
585588
_render_request(marker),
586589
_result_type(marker),

src/mcp/server/mcpserver/server.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ async def _handle_call_tool(
431431
if isinstance(exc.__cause__, ValidationError):
432432
# Field names only: the rejected values are the caller's data.
433433
fields = sorted({".".join(str(part) for part in err["loc"]) for err in exc.__cause__.errors()})
434-
logger.info("Tool %r rejected arguments: %s", params.name, ", ".join(fields))
434+
logger.info("Tool %r rejected arguments: %r", params.name, fields)
435435
else:
436436
# %r keeps peer-supplied text on one line.
437437
logger.info("Tool %r failed: %r", params.name, str(exc))
@@ -521,10 +521,10 @@ async def call_tool(
521521
522522
Raises:
523523
ToolError: If the tool is unknown, the arguments fail validation, or the
524-
tool (or a resolver) raises `ToolError`.
525-
UnexpectedToolError: If the tool (or a resolver) raises anything other than
526-
`ToolError` or `MCPError`, or its return value fails output conversion.
527-
`__cause__` is the original exception.
524+
tool (or a resolver) raises `ToolError` or `ResourceError`.
525+
UnexpectedToolError: If the tool (or a resolver) raises anything else, or
526+
its return value fails output conversion. `__cause__` is the original
527+
exception.
528528
"""
529529
if context is None:
530530
context = Context(mcp_server=self, subscriptions=self._subscriptions)
@@ -740,16 +740,16 @@ async def handler(
740740
) -> CompleteResult:
741741
try:
742742
result = await func(params.ref, params.argument, params.context)
743+
return CompleteResult(
744+
completion=result if result is not None else Completion(values=[], total=None, has_more=None),
745+
)
743746
except MCPError:
744747
raise
745748
except Exception as exc:
746749
logger.exception("Completion for argument %r raised an unexpected exception", params.argument.name)
747750
raise MCPError(
748751
code=INTERNAL_ERROR, message=f"Error completing argument {params.argument.name}"
749752
) from exc
750-
return CompleteResult(
751-
completion=result if result is not None else Completion(values=[], total=None, has_more=None),
752-
)
753753

754754
self._lowlevel_server.add_request_handler("completion/complete", CompleteRequestParams, handler)
755755
return func

src/mcp/server/mcpserver/tools/base.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,7 @@ async def run(
173173
return self.fn_metadata.convert_result(resolved) if convert_result else resolved
174174
pass_directly |= resolved
175175

176-
result = await self.fn_metadata.call_fn_with_arg_validation(
177-
self.fn,
178-
self.is_async,
179-
arguments,
180-
pass_directly or None,
181-
pre_validated=validated,
182-
)
176+
result = await self.fn_metadata.call_fn(self.fn, self.is_async, validated, pass_directly)
183177

184178
# Registration rejects the annotated form of this combination; this covers
185179
# a body that returns an InputRequiredResult without declaring it. It is

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

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
)
2424
from pydantic.fields import FieldInfo
2525
from pydantic.json_schema import GenerateJsonSchema, JsonSchemaWarningKind
26-
from typing_extensions import NotRequired, ReadOnly, TypedDict, get_type_hints, is_typeddict
26+
from typing_extensions import NotRequired, ReadOnly, TypedDict, deprecated, get_type_hints, is_typeddict
2727
from typing_inspection.introspection import (
2828
UNKNOWN,
2929
AnnotationSource,
@@ -35,6 +35,7 @@
3535
from mcp.server.mcpserver.exceptions import InvalidSignature
3636
from mcp.server.mcpserver.utilities.logging import get_logger
3737
from mcp.server.mcpserver.utilities.types import Audio, Image
38+
from mcp.shared.exceptions import MCPDeprecationWarning
3839

3940
logger = get_logger(__name__)
4041

@@ -125,6 +126,28 @@ def validate_arguments(self, arguments_to_validate: dict[str, Any]) -> dict[str,
125126
arguments_parsed_model = self.arg_model.model_validate(arguments_pre_parsed)
126127
return arguments_parsed_model.model_dump_one_level()
127128

129+
async def call_fn(
130+
self,
131+
fn: Callable[..., Any | Awaitable[Any]],
132+
fn_is_async: bool,
133+
arguments: dict[str, Any],
134+
arguments_to_pass_directly: dict[str, Any] | None = None,
135+
) -> Any:
136+
"""Call the function with already-validated `arguments` plus `arguments_to_pass_directly`.
137+
138+
`arguments` is the output of `validate_arguments`. A sync function runs on a
139+
worker thread.
140+
"""
141+
kwargs = arguments | (arguments_to_pass_directly or {})
142+
if fn_is_async:
143+
return await fn(**kwargs)
144+
return await anyio.to_thread.run_sync(functools.partial(fn, **kwargs))
145+
146+
@deprecated(
147+
"FuncMetadata.call_fn_with_arg_validation() is deprecated and will be removed in 3.0; "
148+
"call validate_arguments() and then call_fn() instead.",
149+
category=MCPDeprecationWarning,
150+
)
128151
async def call_fn_with_arg_validation(
129152
self,
130153
fn: Callable[..., Any | Awaitable[Any]],
@@ -133,25 +156,12 @@ async def call_fn_with_arg_validation(
133156
arguments_to_pass_directly: dict[str, Any] | None,
134157
pre_validated: dict[str, Any] | None = None,
135158
) -> Any:
136-
"""Call the given function with arguments validated and injected.
159+
"""Validate `arguments_to_validate` (unless `pre_validated` is given) and call the function.
137160
138-
Arguments are first attempted to be parsed from JSON, then validated against
139-
the argument model, before being passed to the function. Pass `pre_validated`
140-
(the output of `validate_arguments`) to reuse an earlier validation pass -
141-
validating twice can re-run `default_factory`/stateful validators and hand the
142-
function different values than a caller already observed.
161+
Deprecated: call `validate_arguments` and then `call_fn`.
143162
"""
144-
# Copy so a caller-provided `pre_validated` dict is never mutated in place.
145-
arguments_parsed_dict = dict(
146-
pre_validated if pre_validated is not None else self.validate_arguments(arguments_to_validate)
147-
)
148-
149-
arguments_parsed_dict |= arguments_to_pass_directly or {}
150-
151-
if fn_is_async:
152-
return await fn(**arguments_parsed_dict)
153-
else:
154-
return await anyio.to_thread.run_sync(functools.partial(fn, **arguments_parsed_dict))
163+
arguments = pre_validated if pre_validated is not None else self.validate_arguments(arguments_to_validate)
164+
return await self.call_fn(fn, fn_is_async, arguments, arguments_to_pass_directly)
155165

156166
def convert_result(self, result: Any) -> CallToolResult | InputRequiredResult:
157167
"""Convert a function call result into a `CallToolResult`.

0 commit comments

Comments
 (0)