From ff178c720abd2b99e30fa9376699c7d8aaac8748 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Fri, 14 Aug 2026 12:57:22 +0000 Subject: [PATCH 1/6] Log MCPServer handler exceptions once, by kind 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. --- docs/handlers/logging.md | 2 + docs/migration.md | 2 +- docs/servers/handling-errors.md | 30 +- docs/servers/uri-templates.md | 9 +- docs/troubleshooting.md | 2 + docs_src/handling_errors/tutorial004.py | 14 + src/mcp/server/mcpserver/exceptions.py | 41 +- src/mcp/server/mcpserver/prompts/base.py | 4 +- .../server/mcpserver/resources/templates.py | 14 +- src/mcp/server/mcpserver/resources/types.py | 28 +- src/mcp/server/mcpserver/server.py | 68 ++- src/mcp/server/mcpserver/tools/base.py | 49 +- tests/docs_src/test_handling_errors.py | 47 +- tests/docs_src/test_troubleshooting.py | 10 + tests/interaction/_requirements.py | 23 + tests/interaction/mcpserver/test_prompts.py | 32 ++ tests/interaction/mcpserver/test_resources.py | 32 ++ tests/interaction/mcpserver/test_tools.py | 31 ++ .../resources/test_file_resources.py | 8 +- .../resources/test_function_resources.py | 20 +- tests/server/mcpserver/test_server.py | 523 +++++++++++++++++- 21 files changed, 915 insertions(+), 74 deletions(-) create mode 100644 docs_src/handling_errors/tutorial004.py diff --git a/docs/handlers/logging.md b/docs/handlers/logging.md index 6f6c839314..1c839d70b5 100644 --- a/docs/handlers/logging.md +++ b/docs/handlers/logging.md @@ -70,6 +70,8 @@ went to standard error: the terminal, not the wire. don't want log lines, you want spans. Your server already emits them: the SDK traces every message with OpenTelemetry out of the box. See **[OpenTelemetry](../run/opentelemetry.md)**. +You don't have to log your own handlers' crashes either. When a tool or resource function raises something unexpected, the SDK writes the `ERROR` record with the traceback for you, on its own `mcp.*` loggers; a failure you raised deliberately (`ToolError`, `ResourceNotFoundError`) is an `INFO` line instead. A prompt function that raises is an `ERROR` record too, whatever it raised. **[Handling errors](../servers/handling-errors.md#what-lands-in-your-log)** has the split. (In a test using `Client(mcp, raise_exceptions=True)`, a prompt failure is handed to your test as the exception rather than logged.) + ## Recap * The MCP protocol's logging capability is deprecated by the 2026-07-28 spec and not replaced. Don't build on it. diff --git a/docs/migration.md b/docs/migration.md index b094d79f84..e59b4a6ac6 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1016,7 +1016,7 @@ except MCPError as e: ### Resource not found returns `-32602` and resource lookups raise typed exceptions (SEP-2164) -Reading a missing resource now returns JSON-RPC error code `-32602` (invalid params) with the requested URI in `error.data` (`{"uri": ...}`), per [SEP-2164](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2164). Previously the server returned code `0` with no `data`. Clients can now reliably distinguish not-found from other errors; a template handler that raises `ResourceNotFoundError` (from `mcp.server.mcpserver.exceptions`) produces this same response. +Reading a missing resource now returns JSON-RPC error code `-32602` (invalid params) with the requested URI in `error.data` (`{"uri": ...}`), per [SEP-2164](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2164). Previously the server returned code `0` with no `data`. Clients can now reliably distinguish not-found from other errors; a resource handler (static or template) that raises `ResourceNotFoundError` (from `mcp.server.mcpserver.exceptions`) produces this same response. The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). diff --git a/docs/servers/handling-errors.md b/docs/servers/handling-errors.md index 4262f586a7..6519889312 100644 --- a/docs/servers/handling-errors.md +++ b/docs/servers/handling-errors.md @@ -115,10 +115,29 @@ Send `get_author` a `title` that isn't a string and the SDK rejects it against t It means a whole class of `raise` statements you don't write: don't re-validate your own type hints. !!! info - Everything on this page is what a **client** sees, and the in-memory `Client` you'll write - tests with sees exactly the same thing. Even `raise_exceptions=True` doesn't turn a tool error - back into a traceback: by the time that flag could act, your exception is already the - `is_error=True` result. Assert on the result. **[Testing](../get-started/testing.md)** covers the pattern. + Everything so far is what a **client** sees, and the in-memory `Client` you'll write tests + with sees exactly the same thing. Even `raise_exceptions=True` doesn't hand a failing tool's + exception back to the caller: by the time that flag could act, your exception is already the + `is_error=True` result. Assert on the result; the traceback is in the server's log (next + section), which pytest's `caplog` captures. **[Testing](../get-started/testing.md)** covers the pattern. + +## What lands in your log + +Your server keeps its own record of these failures, and it draws one more line: between a failure you anticipated and one you didn't. + +`get_author` raised a plain `ValueError`. The model got the message, but the SDK can't know you *meant* that exception, so it assumes you didn't: the call is logged at `ERROR` with the full traceback. That is exactly what you want on the day the exception is a `KeyError` from three libraries down and the result text says only `'id'`. + +When the failure is one you planned for, say so with `ToolError`: + +```python title="server.py" hl_lines="2 12-13" +--8<-- "docs_src/handling_errors/tutorial004.py" +``` + +The model reads precisely what it read before. The difference is on your side: a `ToolError` is logged as one `INFO` line with no traceback, so a production log at `WARNING` stays quiet until something is actually broken. Bad arguments and unknown tool names are `INFO` lines too; those are the caller's mistakes, not yours. + +Resources draw the same line. The `-32603` from a crashing resource handler names only the URI, so the `ERROR` record in your log is the one place the cause and its traceback exist. `ResourceNotFoundError`, including the SDK's own `Unknown resource`, is an `INFO` line. (A template parameter that fails its type annotation, `books://{id}` read with an `id` that isn't an `int`, currently counts as a crash.) + +Prompts aren't split yet: any failure in a prompt function, including an unknown name or a missing argument, is one `ERROR` record with its traceback, written by the transport layer that turns it into the JSON-RPC error. ## Recap @@ -127,7 +146,8 @@ It means a whole class of `raise` statements you don't write: don't re-validate * The deciding question: *could a smarter model have avoided this?* Yes -> exception. No -> `MCPError`. * `ResourceNotFoundError` from a resource handler -> the protocol's `-32602`, with the URI in `data`. * Bad arguments are rejected against the schema before your function runs; you don't `raise` for those. -* `from mcp import MCPError`; the error-code constants come from `mcp.types`. +* In your log: an exception you didn't raise as `ToolError` is an `ERROR` record with its traceback; `ToolError`, bad tool arguments, unknown tool names, and `ResourceNotFoundError` are one `INFO` line each. +* `from mcp import MCPError`; `ToolError` and `ResourceNotFoundError` come from `mcp.server.mcpserver.exceptions`; the error-code constants come from `mcp.types`. 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)**. diff --git a/docs/servers/uri-templates.md b/docs/servers/uri-templates.md index 406a8fda6a..a79889b2af 100644 --- a/docs/servers/uri-templates.md +++ b/docs/servers/uri-templates.md @@ -199,10 +199,11 @@ These checks are a heuristic pre-filter; for filesystem access, `safe_join` remains the containment boundary. !!! tip - If your handler can't fulfil the request (the file doesn't exist, - the id is unknown), raise an exception. The SDK turns it into an - error response. See **[Handling errors](handling-errors.md)** for the difference between a - protocol error and a tool error. + If your handler can't fulfil the request (the file doesn't exist, the id is unknown), raise + `ResourceNotFoundError` from `mcp.server.mcpserver.exceptions`. The client gets `-32602` with + your message and the URI, and your log gets one `INFO` line; any other exception is treated as + a crash (`-32603`, and an `ERROR` record with the traceback). See + **[Handling errors](handling-errors.md#a-resource-that-doesnt-exist)**. ## Resources on the low-level Server diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 75a6652ecc..f549dfbadd 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -92,6 +92,8 @@ 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. +If `` alone doesn't tell you what broke, the traceback is in the **server's log**: an exception the tool didn't raise as `ToolError` is logged there at `ERROR`, as `Tool '' raised an unexpected exception`. + ## `TypeError: The @tool decorator was used incorrectly. Did you forget to call it? Use @tool() instead of @tool` You wrote `@mcp.tool` instead of `@mcp.tool()`. `tool()` is a decorator *factory*: without the parentheses, Python hands your function to its `name=` parameter. diff --git a/docs_src/handling_errors/tutorial004.py b/docs_src/handling_errors/tutorial004.py new file mode 100644 index 0000000000..9676a10075 --- /dev/null +++ b/docs_src/handling_errors/tutorial004.py @@ -0,0 +1,14 @@ +from mcp.server import MCPServer +from mcp.server.mcpserver.exceptions import ToolError + +mcp = MCPServer("Bookshop") + +CATALOG = {"Dune": "Frank Herbert", "Neuromancer": "William Gibson"} + + +@mcp.tool() +def get_author(title: str) -> str: + """Look up the author of a book in the catalog.""" + if title not in CATALOG: + raise ToolError(f"No book titled {title!r} in the catalog.") + return CATALOG[title] diff --git a/src/mcp/server/mcpserver/exceptions.py b/src/mcp/server/mcpserver/exceptions.py index 239785e9a9..9f2415d976 100644 --- a/src/mcp/server/mcpserver/exceptions.py +++ b/src/mcp/server/mcpserver/exceptions.py @@ -6,20 +6,55 @@ class MCPServerError(Exception): class ResourceError(MCPServerError): - """Error in resource operations.""" + """Error in resource operations. + + When a resource or resource template handler raises this, its message reaches + the client as a `-32603` protocol error. + """ class ResourceNotFoundError(ResourceError): """Resource does not exist. - Raise this from a resource template handler to signal that the requested instance does not exist; + Raise this from a resource handler to signal that the requested instance does not exist; clients receive `-32602` (invalid params) per [SEP-2164](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2164). """ +class UnexpectedResourceError(ResourceError): + """A resource read failed with something other than `ResourceError` or `MCPError`. + + MCPServer raises this itself, around a crash in a resource or resource + template handler or a failed file read; you never raise it. `__cause__` is + the original exception, which the server logs with its traceback. The + message names only the URI, so the original text is withheld from the client. + """ + + class ToolError(MCPServerError): - """Error in tool operations.""" + """A tool failure the model should read. + + Raise this from a tool (or a resolver) for a failure you anticipate: the + call returns `is_error=True` with the message in `content`, and the server + logs it at INFO without a traceback. Any other exception reaches the model + the same way but is treated as a crash and logged at ERROR with its traceback. + + The SDK raises it too, for an unknown tool name and for arguments that fail + the input schema, and `UnexpectedToolError` subclasses it, so `except ToolError` + around `MCPServer.call_tool()` catches every tool failure, crash or not. + """ + + +class UnexpectedToolError(ToolError): + """A tool call failed with something other than `ToolError` or `MCPError`. + + MCPServer raises this itself, around a crash in the tool (or a resolver) or a + return value that fails output conversion; you never raise it. `__cause__` is + the original exception, which the server logs with its traceback before + returning the usual `is_error=True` result. Catch it around + `MCPServer.call_tool()` to tell a crash from a deliberate `ToolError`. + """ class InvalidSignature(Exception): diff --git a/src/mcp/server/mcpserver/prompts/base.py b/src/mcp/server/mcpserver/prompts/base.py index 0a010de7d2..253a05348f 100644 --- a/src/mcp/server/mcpserver/prompts/base.py +++ b/src/mcp/server/mcpserver/prompts/base.py @@ -196,5 +196,5 @@ async def render( return messages except MCPError: raise - except Exception as e: - raise ValueError(f"Error rendering prompt {self.name}: {e}") + except Exception as exc: + raise ValueError(f"Error rendering prompt {self.name}: {exc}") from exc diff --git a/src/mcp/server/mcpserver/resources/templates.py b/src/mcp/server/mcpserver/resources/templates.py index 096e821d81..7c789dc57d 100644 --- a/src/mcp/server/mcpserver/resources/templates.py +++ b/src/mcp/server/mcpserver/resources/templates.py @@ -11,18 +11,15 @@ from mcp_types import Annotations, Icon, InputRequiredResult from pydantic import BaseModel, Field, validate_call -from mcp.server.mcpserver.exceptions import ResourceError +from mcp.server.mcpserver.exceptions import ResourceError, UnexpectedResourceError from mcp.server.mcpserver.resources.types import FunctionResource, Resource from mcp.server.mcpserver.utilities.context_injection import find_context_parameter, inject_context from mcp.server.mcpserver.utilities.func_metadata import func_metadata -from mcp.server.mcpserver.utilities.logging import get_logger from mcp.shared._callable_inspection import is_async_callable from mcp.shared.exceptions import MCPError from mcp.shared.path_security import contains_path_traversal, is_absolute_path from mcp.shared.uri_template import UriTemplate -logger = get_logger(__name__) - if TYPE_CHECKING: from mcp.server.context import LifespanContextT, RequestT from mcp.server.mcpserver.context import Context @@ -218,7 +215,9 @@ async def create_resource( carrying the echoed opaque state. Raises: - ResourceError: If creating the resource fails. + ResourceError: If the template function raises `ResourceError`. + UnexpectedResourceError: If the template function raises anything other + than `ResourceError` or `MCPError`; `__cause__` is the original. """ try: # Add context to params if needed @@ -247,5 +246,6 @@ async def create_resource( except (ResourceError, MCPError): raise except Exception as exc: - logger.exception(f"Error creating resource from template {uri}") - raise ResourceError(f"Error creating resource from template {uri}") from exc + # Name only the URI: the original text is withheld from the client, and + # the server logs the traceback from `__cause__`. + raise UnexpectedResourceError(f"Error creating resource from template {uri}") from exc diff --git a/src/mcp/server/mcpserver/resources/types.py b/src/mcp/server/mcpserver/resources/types.py index 2edf342337..f3751989d7 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -16,6 +16,7 @@ from mcp_types import Annotations, Icon, InputRequiredResult from pydantic import Field, validate_call +from mcp.server.mcpserver.exceptions import ResourceError, UnexpectedResourceError from mcp.server.mcpserver.resources.base import Resource from mcp.shared._callable_inspection import is_async_callable from mcp.shared.exceptions import MCPError @@ -79,7 +80,12 @@ class FunctionResource(Resource): fn: Callable[[], Any] = Field(exclude=True) async def read(self) -> str | bytes: - """Read the resource by calling the wrapped function.""" + """Read the resource by calling the wrapped function. + + Raises: + UnexpectedResourceError: If the function raises anything other than + `ResourceError` or `MCPError`; `__cause__` is the original. + """ try: fn = self.fn if is_async_callable(fn): @@ -103,10 +109,12 @@ async def read(self) -> str | bytes: return result else: return pydantic_core.to_json(result, fallback=str, indent=2).decode() - except MCPError: + except (MCPError, ResourceError): raise - except Exception as e: - raise ValueError(f"Error reading resource {self.uri}: {e}") + except Exception as exc: + # Name only the URI: the original text is withheld from the client, and + # the server logs the traceback from `__cause__`. + raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc @classmethod def from_function( @@ -187,8 +195,8 @@ async def read(self) -> str | bytes: if self.encoding is None: return await anyio.to_thread.run_sync(self.path.read_bytes) return await anyio.to_thread.run_sync(partial(self.path.read_text, encoding=self.encoding)) - except Exception as e: - raise ValueError(f"Error reading file {self.path}: {e}") + except Exception as exc: + raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc class HttpResource(Resource): @@ -232,8 +240,8 @@ def list_files(self) -> list[Path]: # pragma: no cover if self.pattern: return list(self.path.glob(self.pattern)) if not self.recursive else list(self.path.rglob(self.pattern)) return list(self.path.glob("*")) if not self.recursive else list(self.path.rglob("*")) - except Exception as e: - raise ValueError(f"Error listing directory {self.path}: {e}") + except Exception as exc: + raise ValueError(f"Error listing directory {self.path}: {exc}") from exc async def read(self) -> str: # Always returns JSON string # pragma: no cover """Read the directory listing.""" @@ -241,5 +249,5 @@ async def read(self) -> str: # Always returns JSON string # pragma: no cover files = await anyio.to_thread.run_sync(self.list_files) file_list = [str(f.relative_to(self.path)) for f in files if f.is_file()] return json.dumps({"files": file_list}, indent=2) - except Exception as e: - raise ValueError(f"Error reading directory {self.path}: {e}") + except Exception as exc: + raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index bc79c44a36..93319cf273 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -71,7 +71,13 @@ from mcp.server.lowlevel.server import LifespanResultT, Server from mcp.server.lowlevel.server import lifespan as default_lifespan from mcp.server.mcpserver.context import Context -from mcp.server.mcpserver.exceptions import ResourceError, ResourceNotFoundError +from mcp.server.mcpserver.exceptions import ( + ResourceError, + ResourceNotFoundError, + ToolError, + UnexpectedResourceError, + UnexpectedToolError, +) from mcp.server.mcpserver.prompts import Prompt, PromptManager from mcp.server.mcpserver.resources import ( DEFAULT_RESOURCE_SECURITY, @@ -420,8 +426,9 @@ async def _handle_call_tool( return await self.call_tool(params.name, params.arguments or {}, context) except MCPError: raise - except Exception as e: - return CallToolResult(content=[TextContent(type="text", text=str(e))], is_error=True) + except Exception as exc: + _log_handler_exception("Tool", params.name, exc) + return CallToolResult(content=[TextContent(type="text", text=str(exc))], is_error=True) async def _handle_list_resources( self, ctx: ServerRequestContext[LifespanResultT], params: PaginatedRequestParams | None @@ -434,10 +441,10 @@ async def _handle_read_resource( context = Context(request_context=ctx, mcp_server=self, input_params=params, subscriptions=self._subscriptions) try: results = await self.read_resource(params.uri, context) - except ResourceNotFoundError as err: - raise MCPError(code=INVALID_PARAMS, message=str(err), data={"uri": str(params.uri)}) except ResourceError as err: - raise MCPError(code=INTERNAL_ERROR, message=str(err), data={"uri": str(params.uri)}) + _log_handler_exception("Resource", str(params.uri), err) + code = INVALID_PARAMS if isinstance(err, ResourceNotFoundError) else INTERNAL_ERROR + raise MCPError(code=code, message=str(err), data={"uri": str(params.uri)}) if isinstance(results, InputRequiredResult): return results contents: list[TextResourceContents | BlobResourceContents] = [] @@ -498,7 +505,15 @@ async def list_tools(self) -> list[MCPTool]: async def call_tool( self, name: str, arguments: dict[str, Any], context: Context[LifespanResultT, Any] | None = None ) -> CallToolResult | InputRequiredResult: - """Call a tool by name with arguments.""" + """Call a tool by name with arguments. + + Raises: + ToolError: If the tool is unknown, the arguments fail validation, or the + tool (or a resolver) raises `ToolError`. + 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. + """ if context is None: context = Context(mcp_server=self, subscriptions=self._subscriptions) return await self._tool_manager.call_tool(name, arguments, context, convert_result=True) @@ -549,7 +564,10 @@ async def read_resource( Raises: ResourceNotFoundError: If no resource or template matches the URI. - ResourceError: If template creation or resource reading fails. + ResourceError: If the resource or template function raises `ResourceError`. + UnexpectedResourceError: If reading the resource (or creating it from a + template) raises anything other than `ResourceError` or `MCPError`; + `__cause__` is the original. """ if context is None: context = Context(mcp_server=self, subscriptions=self._subscriptions) @@ -560,12 +578,14 @@ async def read_resource( try: content = await resource.read() return [ReadResourceContents(content=content, mime_type=resource.mime_type, meta=resource.meta)] - except MCPError: + except (MCPError, ResourceError): + # Includes the UnexpectedResourceError the built-in resource types raise + # around a crash in the function or the file read. raise except Exception as exc: - logger.exception(f"Error getting resource {uri}") - # If an exception happens when reading the resource, we should not leak the exception to the client. - raise ResourceError(f"Error reading resource {uri}") from exc + # A custom Resource subclass whose read() raised: wrap it the same way, + # naming only the URI so the original text is withheld from the client. + raise UnexpectedResourceError(f"Error reading resource {uri}") from exc def add_tool( self, @@ -1293,10 +1313,32 @@ async def get_prompt( except MCPError: raise except Exception as e: - logger.exception(f"Error getting prompt {name}") + # Not logged here: this escapes `_handle_get_prompt` as-is, so the + # dispatcher boundary that turns it into the JSON-RPC error logs it once + # with its traceback (or, in-process with `raise_exceptions=True`, + # hands it to the caller instead). raise ValueError(str(e)) from e +def _log_handler_exception(kind: Literal["Tool", "Resource"], name: str, exc: Exception) -> None: + """Record a tool or resource handler failure; the one place MCPServer logs them. + + Called from the `except` block that turns the failure into a response. A + `ToolError` or `ResourceError` (deliberate, an unknown name, arguments that + failed validation, `ResourceNotFoundError`) is an anticipated outcome the + client already receives in full: one INFO record, no traceback, the text + repr-quoted so peer-supplied names and newlines stay on one line. Anything + else, including the `Unexpected*` wrappers whose `__cause__` is what the + handler actually raised, is a crash in user code: ERROR with the traceback. + """ + if isinstance(exc, ToolError | ResourceError) and not isinstance( + exc, UnexpectedToolError | UnexpectedResourceError + ): + logger.info("%s %r failed: %r", kind, name, str(exc)) + else: + logger.exception("%s %r raised an unexpected exception", kind, name, exc_info=exc) + + def _version_gated(method: MethodBinding) -> RequestHandler: """Wrap a method handler so a request at a disallowed protocol version is rejected. diff --git a/src/mcp/server/mcpserver/tools/base.py b/src/mcp/server/mcpserver/tools/base.py index 23248707a3..cd556e9726 100644 --- a/src/mcp/server/mcpserver/tools/base.py +++ b/src/mcp/server/mcpserver/tools/base.py @@ -5,9 +5,9 @@ from typing import TYPE_CHECKING, Any from mcp_types import Icon, InputRequiredResult, ToolAnnotations -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, ValidationError -from mcp.server.mcpserver.exceptions import InvalidSignature, ToolError +from mcp.server.mcpserver.exceptions import InvalidSignature, ToolError, UnexpectedToolError from mcp.server.mcpserver.resolve import ( build_resolver_plans, find_resolved_parameters, @@ -128,21 +128,33 @@ async def run( ) -> Any: """Run the tool with arguments. + Every failure other than `MCPError` is raised with its message prefixed + `Error executing tool : `, and `__cause__` set to what was raised. + Raises: - ToolError: If the tool function raises during execution. + ToolError: If the arguments fail validation against the input schema, or + the tool function (or a resolver) raises `ToolError`. + UnexpectedToolError: If the tool function (or a resolver) raises anything + other than `ToolError` or `MCPError`, or its return value fails output + conversion. """ + try: + validated = self.fn_metadata.validate_arguments(arguments) + except ValidationError as exc: + # The caller's arguments don't match the input schema. That is the model's + # mistake to read and correct, so it is reported like a deliberate ToolError. + raise ToolError(f"Error executing tool {self.name}: {exc}") from exc + try: pass_directly: dict[str, Any] = {} if self.context_kwarg is not None: pass_directly[self.context_kwarg] = context - # Resolvers see the same validated arguments the tool body receives: - # validate once and reuse it, so a `default_factory`/stateful validator - # can't hand a by-name resolver a different value than the body. - pre_validated: dict[str, Any] | None = None + # Resolvers see the same validated arguments the tool body receives, so a + # `default_factory`/stateful validator can't hand a by-name resolver a + # different value than the body. if self.resolved_params: - pre_validated = self.fn_metadata.validate_arguments(arguments) - resolved = await resolve_arguments(self.resolved_params, self.resolver_plans, pre_validated, context) + resolved = await resolve_arguments(self.resolved_params, self.resolver_plans, validated, context) if isinstance(resolved, InputRequiredResult): # A resolver still needs client input (>= 2026-07-28): surface the # batched questions instead of running the tool body this round. @@ -154,13 +166,14 @@ async def run( self.is_async, arguments, pass_directly or None, - pre_validated=pre_validated, + pre_validated=validated, ) # Registration rejects the annotated form of this combination; this covers - # a body that returns an InputRequiredResult without declaring it. + # a body that returns an InputRequiredResult without declaring it. It is + # an authoring bug, so it is raised as a crash rather than a ToolError. if self.resolved_params and isinstance(result, InputRequiredResult): - raise ToolError( + raise RuntimeError( "the tool returned an InputRequiredResult but its parameters use Resolve(...); " "a call has one input_required channel, so the multi-round flow is driven " "either by resolvers or by the tool body, not both" @@ -177,5 +190,13 @@ async def run( # it as a top-level JSON-RPC error rather than wrapping it as a # `CallToolResult(isError=True)` execution failure. raise - except Exception as e: - raise ToolError(f"Error executing tool {self.name}: {e}") from e + # Everything else reaches the model as an is_error result under this tool's + # name. The wrapper's type is what tells the server whether to log a crash. + except UnexpectedToolError as exc: + # A nested tool call crashed: still a crash under this tool's name. + raise UnexpectedToolError(f"Error executing tool {self.name}: {exc}") from exc + except ToolError as exc: + # Raised deliberately by the tool or a resolver: anticipated. + raise ToolError(f"Error executing tool {self.name}: {exc}") from exc + except Exception as exc: + raise UnexpectedToolError(f"Error executing tool {self.name}: {exc}") from exc diff --git a/tests/docs_src/test_handling_errors.py b/tests/docs_src/test_handling_errors.py index 0c2629169c..e2aab0e423 100644 --- a/tests/docs_src/test_handling_errors.py +++ b/tests/docs_src/test_handling_errors.py @@ -1,9 +1,11 @@ """`docs/servers/handling-errors.md`: every claim the page makes, proved against the real SDK.""" +import logging + import pytest from mcp_types import INVALID_PARAMS, ErrorData, TextContent, TextResourceContents -from docs_src.handling_errors import tutorial001, tutorial002, tutorial003 +from docs_src.handling_errors import tutorial001, tutorial002, tutorial003, tutorial004 from mcp import Client, MCPError # See test_index.py for why this is a per-module mark and not a conftest hook. @@ -68,7 +70,8 @@ async def test_resource_not_found_error_maps_to_invalid_params() -> None: async def test_raise_exceptions_does_not_turn_a_tool_error_into_a_traceback() -> None: - """The closing `!!! info`: even `raise_exceptions=True` leaves a failing tool as the `is_error=True` result.""" + """The `!!! info` before the log section: even `raise_exceptions=True` leaves a failing tool as the + `is_error=True` result.""" async with Client(tutorial001.mcp, raise_exceptions=True) as client: result = await client.call_tool("get_author", {"title": "Nothing"}) assert result.is_error @@ -84,3 +87,43 @@ async def test_a_title_the_template_knows_reads_normally() -> None: (contents,) = result.contents assert isinstance(contents, TextResourceContents) assert contents.text == "Dune by Frank Herbert" + + +async def test_a_plain_exception_is_logged_as_a_crash_with_its_traceback(caplog: pytest.LogCaptureFixture) -> None: + """tutorial001, "What lands in your log": the `ValueError` is one ERROR record carrying the traceback.""" + caplog.set_level(logging.INFO) + async with Client(tutorial001.mcp) as client: + await client.call_tool("get_author", {"title": "Nothing"}) + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert record.levelno == logging.ERROR + assert record.exc_info is not None + logged = record.exc_info[1] + assert logged is not None and isinstance(logged.__cause__, ValueError) + assert str(logged.__cause__) == "No book titled 'Nothing' in the catalog." + + +async def test_tool_error_reads_the_same_to_the_model_and_logs_one_info_line( + caplog: pytest.LogCaptureFixture, +) -> None: + """tutorial004: swapping in `ToolError` leaves the result byte-identical and the log at INFO, no traceback.""" + caplog.set_level(logging.INFO) + async with Client(tutorial004.mcp) as client: + result = await client.call_tool("get_author", {"title": "Nothing"}) + assert result.is_error + assert result.content == [ + TextContent(type="text", text="Error executing tool get_author: No book titled 'Nothing' in the catalog.") + ] + records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert [(r.levelno, r.exc_info) for r in records] == [(logging.INFO, None)] + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_a_bad_argument_is_an_info_line_not_a_crash(caplog: pytest.LogCaptureFixture) -> None: + """ "What lands in your log": schema rejection of the arguments is logged at INFO with no traceback.""" + caplog.set_level(logging.INFO) + async with Client(tutorial001.mcp) as client: + result = await client.call_tool("get_author", {"title": 42}) + assert result.is_error + records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert [(r.levelno, r.exc_info) for r in records] == [(logging.INFO, None)] + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] diff --git a/tests/docs_src/test_troubleshooting.py b/tests/docs_src/test_troubleshooting.py index 9c94b643c1..7d79e21b5e 100644 --- a/tests/docs_src/test_troubleshooting.py +++ b/tests/docs_src/test_troubleshooting.py @@ -83,6 +83,16 @@ async def test_a_failing_tool_returns_is_error_true_instead_of_raising() -> None ] +async def test_a_failing_tool_leaves_its_traceback_in_the_server_log(caplog: pytest.LogCaptureFixture) -> None: + """The `Error executing tool` entry's pointer to the server log: the exact ERROR message it names.""" + with caplog.at_level(logging.ERROR, logger="mcp.server.mcpserver.server"): + async with Client(tutorial001.mcp) as client: + await client.call_tool("forecast", {"city": "Atlantis"}) + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert record.getMessage() == "Tool 'forecast' raised an unexpected exception" + assert record.exc_info is not None + + async def test_an_unknown_tool_is_the_same_kind_of_result() -> None: """`Unknown tool: ` travels the same `is_error=True` path as a failing tool.""" async with Client(tutorial001.mcp) as client: diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 964a1829d2..3b5f819f18 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -1020,6 +1020,14 @@ def __post_init__(self) -> None: "tool result with isError true and the failure text in content; it does not become a JSON-RPC error." ), ), + "mcpserver:tool:handler-throws:logged": Requirement( + source="sdk", + behavior=( + "An exception other than ToolError raised by a tool function is logged server-side exactly once, " + "at ERROR with its traceback, before the isError result is returned; the transport does not change " + "how many records are written." + ), + ), "mcpserver:tool:input-validation": Requirement( source=f"{SPEC_BASE_URL}/server/tools#error-handling", behavior=( @@ -1311,6 +1319,13 @@ def __post_init__(self) -> None: "(-32603 Internal error), with the original exception text withheld." ), ), + "mcpserver:resource:read-throws:logged": Requirement( + source="sdk", + behavior=( + "The exception withheld from the -32603 response is logged server-side exactly once, at ERROR " + "with its traceback; the transport does not change how many records are written." + ), + ), "mcpserver:resource:static": Requirement( source="sdk", behavior=( @@ -1427,6 +1442,14 @@ def __post_init__(self) -> None: source="sdk", behavior="A prompt with optional arguments can be fetched without supplying them.", ), + "mcpserver:prompt:render-throws:logged": Requirement( + source="sdk", + behavior=( + "An exception raised by a prompt function is logged server-side exactly once, at ERROR with its " + "traceback, by whichever layer turns it into the JSON-RPC error; the transport does not change how " + "many records are written." + ), + ), "mcpserver:prompt:unknown-name": Requirement( source=f"{SPEC_BASE_URL}/server/prompts#error-handling", behavior="prompts/get for a name that was never registered returns JSON-RPC error -32602 (Invalid params).", diff --git a/tests/interaction/mcpserver/test_prompts.py b/tests/interaction/mcpserver/test_prompts.py index 8409e50207..5a357592ae 100644 --- a/tests/interaction/mcpserver/test_prompts.py +++ b/tests/interaction/mcpserver/test_prompts.py @@ -1,5 +1,7 @@ """Prompt interactions against MCPServer, driven through the public Client API.""" +import logging + import pytest from inline_snapshot import snapshot from mcp_types import ( @@ -141,6 +143,36 @@ def repeat(phrase: str, count: int) -> str: assert exc_info.value.error.message.startswith("Error rendering prompt repeat: 1 validation error") +@requirement("mcpserver:prompt:render-throws:logged") +async def test_get_prompt_function_exception_is_logged_once_with_its_traceback( + connect: Connect, caplog: pytest.LogCaptureFixture +) -> None: + """An exception raised by a prompt function is logged exactly once, at ERROR, with its traceback. + + MCPServer lets the failure escape to the dispatcher boundary, which owns both the JSON-RPC error and + the log record; the owning logger therefore differs by transport, but the count must not. + """ + mcp = MCPServer("prompter") + raised = RuntimeError("template store unreachable") + + @mcp.prompt() + def briefing() -> str: + raise raised + + caplog.set_level(logging.ERROR) + async with connect(mcp) as client: + with pytest.raises(MCPError): + await client.get_prompt("briefing") + + def chains_to_raised(exc: BaseException | None) -> bool: + while exc is not None and exc is not raised: + exc = exc.__cause__ or exc.__context__ + return exc is raised + + records = [r for r in caplog.records if r.exc_info and chains_to_raised(r.exc_info[1])] + assert [r.levelname for r in records] == ["ERROR"] + + @requirement("mcpserver:prompt:optional-args") async def test_get_prompt_with_an_optional_argument_omitted_uses_the_default( connect: Connect, unstamped: Unstamp diff --git a/tests/interaction/mcpserver/test_resources.py b/tests/interaction/mcpserver/test_resources.py index eadf4794e6..162fa9a813 100644 --- a/tests/interaction/mcpserver/test_resources.py +++ b/tests/interaction/mcpserver/test_resources.py @@ -1,5 +1,7 @@ """Resource interactions against MCPServer, driven through the public Client API.""" +import logging + import pytest from inline_snapshot import snapshot from mcp_types import ( @@ -152,6 +154,36 @@ def boom() -> str: ) +@requirement("mcpserver:resource:read-throws:logged") +async def test_resource_function_exception_is_logged_once_with_its_traceback( + connect: Connect, caplog: pytest.LogCaptureFixture +) -> None: + """The exception withheld from the -32603 response is logged exactly once, at ERROR, with its traceback. + + The client sees only the URI, so this record is the operator's only route to the cause; it must be + written on every transport and never duplicated by a dispatcher boundary. + """ + mcp = MCPServer("library") + raised = RuntimeError("nope") + + @mcp.resource("res://boom") + def boom() -> str: + raise raised + + caplog.set_level(logging.ERROR) + async with connect(mcp) as client: + with pytest.raises(MCPError): + await client.read_resource("res://boom") + + def chains_to_raised(exc: BaseException | None) -> bool: + while exc is not None and exc is not raised: + exc = exc.__cause__ or exc.__context__ + return exc is raised + + records = [r for r in caplog.records if r.exc_info and chains_to_raised(r.exc_info[1])] + assert [(r.name, r.levelname) for r in records] == [("mcp.server.mcpserver.server", "ERROR")] + + @requirement("mcpserver:resource:duplicate-name") async def test_registering_a_duplicate_resource_uri_warns_and_keeps_the_first( connect: Connect, unstamped: Unstamp diff --git a/tests/interaction/mcpserver/test_tools.py b/tests/interaction/mcpserver/test_tools.py index a6418ac9c5..c275fc60b1 100644 --- a/tests/interaction/mcpserver/test_tools.py +++ b/tests/interaction/mcpserver/test_tools.py @@ -119,6 +119,37 @@ def flux() -> str: ) +@requirement("mcpserver:tool:handler-throws:logged") +async def test_call_tool_function_exception_is_logged_once_with_its_traceback( + connect: Connect, caplog: pytest.LogCaptureFixture +) -> None: + """The exception behind an is_error result is logged exactly once, at ERROR, with its traceback. + + The result text carries only `str(exc)`; the traceback exists nowhere but this record, so it must + be written on every transport and never duplicated by a dispatcher boundary. + """ + mcp = MCPServer("errors") + raised = LookupError("no such row") + + @mcp.tool() + def explode() -> str: + raise raised + + caplog.set_level(logging.ERROR) + async with connect(mcp) as client: + result = await client.call_tool("explode", {}) + + assert result.is_error is True + + def chains_to_raised(exc: BaseException | None) -> bool: + while exc is not None and exc is not raised: + exc = exc.__cause__ or exc.__context__ + return exc is raised + + records = [r for r in caplog.records if r.exc_info and chains_to_raised(r.exc_info[1])] + assert [(r.name, r.levelname) for r in records] == [("mcp.server.mcpserver.server", "ERROR")] + + @requirement("mcpserver:tool:unknown-name") async def test_call_tool_unknown_name_returns_error_result(connect: Connect, unstamped: Unstamp) -> None: """Calling a tool name that was never registered is reported as an is_error result. diff --git a/tests/server/mcpserver/resources/test_file_resources.py b/tests/server/mcpserver/resources/test_file_resources.py index 042ea422aa..3604bf1b32 100644 --- a/tests/server/mcpserver/resources/test_file_resources.py +++ b/tests/server/mcpserver/resources/test_file_resources.py @@ -6,6 +6,7 @@ import pytest from pydantic import ValidationError +from mcp.server.mcpserver.exceptions import UnexpectedResourceError from mcp.server.mcpserver.resources import FileResource @@ -178,8 +179,10 @@ async def test_missing_file_error(temp_file: Path): name="test", path=missing, ) - with pytest.raises(ValueError, match="Error reading file"): + with pytest.raises(UnexpectedResourceError) as exc: await resource.read() + assert str(exc.value) == "Error reading resource file:///missing.txt" + assert isinstance(exc.value.__cause__, FileNotFoundError) @pytest.mark.skipif(os.name == "nt", reason="File permissions behave differently on Windows") @@ -192,7 +195,8 @@ async def test_permission_error(temp_file: Path): # pragma: lax no cover name="test", path=temp_file, ) - with pytest.raises(ValueError, match="Error reading file"): + with pytest.raises(UnexpectedResourceError) as exc: await resource.read() + assert isinstance(exc.value.__cause__, PermissionError) finally: temp_file.chmod(0o644) # Restore permissions diff --git a/tests/server/mcpserver/resources/test_function_resources.py b/tests/server/mcpserver/resources/test_function_resources.py index 5a5c5c48dd..dc57dbc31c 100644 --- a/tests/server/mcpserver/resources/test_function_resources.py +++ b/tests/server/mcpserver/resources/test_function_resources.py @@ -7,6 +7,7 @@ from mcp_types import InputRequiredResult from pydantic import BaseModel +from mcp.server.mcpserver.exceptions import UnexpectedResourceError from mcp.server.mcpserver.resources import FunctionResource @@ -80,18 +81,22 @@ def get_data() -> dict[str, str]: @pytest.mark.anyio async def test_error_handling(self): - """Test error handling in FunctionResource.""" + """A crash in the function is wrapped as UnexpectedResourceError naming only the URI, + with the function's own exception as `__cause__`.""" + raised = ValueError("Test error") def failing_func() -> str: - raise ValueError("Test error") + raise raised resource = FunctionResource( uri="function://test", name="test", fn=failing_func, ) - with pytest.raises(ValueError, match="Error reading resource function://test"): + with pytest.raises(UnexpectedResourceError) as exc: await resource.read() + assert str(exc.value) == snapshot("Error reading resource function://test") + assert exc.value.__cause__ is raised @pytest.mark.anyio async def test_basemodel_conversion(self): @@ -255,9 +260,10 @@ def ask() -> InputRequiredResult: return InputRequiredResult(request_state="round-1") resource = FunctionResource(uri="resource://ask", name="ask", fn=ask) - with pytest.raises(ValueError) as exc: + with pytest.raises(UnexpectedResourceError) as exc: await resource.read() - assert str(exc.value) == snapshot( - "Error reading resource resource://ask: static resources cannot return " - "InputRequiredResult; only resource template functions participate in the multi-round-trip flow" + assert str(exc.value) == snapshot("Error reading resource resource://ask") + assert str(exc.value.__cause__) == snapshot( + "static resources cannot return InputRequiredResult; " + "only resource template functions participate in the multi-round-trip flow" ) diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 48e900dcab..cd56990f9d 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1,7 +1,8 @@ import base64 +import logging from pathlib import Path from types import SimpleNamespace -from typing import Any +from typing import Annotated, Any from unittest.mock import AsyncMock, MagicMock, patch import anyio @@ -24,6 +25,7 @@ ElicitRequestFormParams, ElicitResult, EmbeddedResource, + ErrorData, GetPromptResult, Icon, ImageContent, @@ -41,16 +43,23 @@ TextContent, TextResourceContents, ) -from pydantic import BaseModel +from pydantic import BaseModel, ValidationError from starlette.applications import Starlette from starlette.routing import Mount, Route from mcp.client import Client from mcp.server.context import ServerRequestContext -from mcp.server.mcpserver import Context, MCPServer, ResourceSecurity -from mcp.server.mcpserver.exceptions import ResourceNotFoundError, ToolError +from mcp.server.mcpserver import Context, MCPServer, RequestStateSecurity, Resolve, ResourceSecurity +from mcp.server.mcpserver.exceptions import ( + ResourceError, + ResourceNotFoundError, + ToolError, + UnexpectedResourceError, + UnexpectedToolError, +) from mcp.server.mcpserver.prompts.base import Message, UserMessage from mcp.server.mcpserver.resources import FileResource, FunctionResource +from mcp.server.mcpserver.resources import Resource as MCPServerResource from mcp.server.mcpserver.utilities.types import Audio, Image from mcp.server.subscriptions import ( InMemorySubscriptionBus, @@ -2233,6 +2242,512 @@ def thing() -> str: assert exc.value.error.data == {"requiredCapabilities": ["elicitation"]} +def _cause_chain(exc: BaseException | None) -> list[BaseException]: + """`exc` and everything it chains back to, explicitly (`__cause__`) or implicitly (`__context__`).""" + chain: list[BaseException] = [] + while exc is not None: + chain.append(exc) + exc = exc.__cause__ or exc.__context__ + return chain + + +def _server_records(caplog: pytest.LogCaptureFixture) -> list[tuple[str, str, bool]]: + """(level, message, has-traceback) for every record MCPServer itself wrote.""" + return [ + (r.levelname, r.getMessage(), r.exc_info is not None) + for r in caplog.records + if r.name == "mcp.server.mcpserver.server" + ] + + +def _logged_exception(caplog: pytest.LogCaptureFixture) -> BaseException: + """The exception attached to the one MCPServer record that carries a traceback.""" + (exc_info,) = [r.exc_info for r in caplog.records if r.name == "mcp.server.mcpserver.server" and r.exc_info] + assert exc_info[1] is not None + return exc_info[1] + + +async def test_tool_raising_unexpected_exception_is_logged_once_at_error_with_its_traceback( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: a tool crash still reaches the model as is_error, and the server logs the + original exception exactly once, at ERROR, with the traceback the result text lacks.""" + mcp = MCPServer() + raised = KeyError("k") + + @mcp.tool() + def lookup() -> str: + raise raised + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("lookup", {}) + + assert result.is_error is True + assert result.content == [TextContent(type="text", text="Error executing tool lookup: 'k'")] + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'lookup' raised an unexpected exception", True)]) + assert raised in _cause_chain(_logged_exception(caplog)) + assert len([r for r in caplog.records if r.levelno >= logging.WARNING]) == 1 + + +async def test_tool_raising_tool_error_is_logged_at_info_without_traceback(caplog: pytest.LogCaptureFixture): + """SDK-defined: ToolError marks an anticipated failure, so the same is_error result is + logged as one INFO record with no traceback rather than as a crash.""" + mcp = MCPServer() + + @mcp.tool() + def forecast(city: str) -> str: + raise ToolError(f"no forecast for {city}") + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("forecast", {"city": "Atlantis"}) + + assert result.is_error is True + assert result.content == [TextContent(type="text", text="Error executing tool forecast: no forecast for Atlantis")] + assert _server_records(caplog) == snapshot( + [("INFO", "Tool 'forecast' failed: 'Error executing tool forecast: no forecast for Atlantis'", False)] + ) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_tool_error_subclass_is_still_anticipated(caplog: pytest.LogCaptureFixture): + """SDK-defined: a user's ToolError subclass is treated like ToolError - INFO, no traceback - + and reaches a programmatic caller as a plain ToolError carrying the tool-name prefix.""" + mcp = MCPServer() + + class QuotaExceeded(ToolError): + pass + + @mcp.tool() + def spend() -> str: + raise QuotaExceeded("daily quota used up") + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("spend", {}) + with pytest.raises(ToolError) as exc: + await mcp.call_tool("spend", {}) + + assert result.is_error is True + assert type(exc.value) is ToolError + assert str(exc.value) == snapshot("Error executing tool spend: daily quota used up") + assert _server_records(caplog) == snapshot( + [("INFO", "Tool 'spend' failed: 'Error executing tool spend: daily quota used up'", False)] + ) + + +async def test_tool_argument_validation_failure_is_logged_at_info_without_traceback( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: arguments the model got wrong are the model's to correct, so the rejection + is logged as one INFO record with no traceback; the message is repr-quoted onto one line.""" + mcp = MCPServer() + + @mcp.tool() + def add(a: int, b: int) -> int: + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("add", {"a": "one", "b": 2}) + + assert result.is_error is True + ((level, message, has_traceback),) = _server_records(caplog) + assert (level, has_traceback) == ("INFO", False) + # pydantic owns the rest of the text; pin only the SDK's part and the single-line rendering. + assert message.startswith("Tool 'add' failed: ") and "Error executing tool add: 1 validation error" in message + assert "\n" not in message + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_tool_argument_validation_failure_chains_directly_to_the_validation_error(): + """SDK-defined: a programmatic caller sees a plain ToolError whose `__cause__` is pydantic's + ValidationError, with no intermediate wrapper.""" + mcp = MCPServer() + + @mcp.tool() + def add(a: int, b: int) -> int: + raise NotImplementedError + + with pytest.raises(ToolError) as exc: + await mcp.call_tool("add", {"a": "one", "b": 2}) + assert type(exc.value) is ToolError + assert isinstance(exc.value.__cause__, ValidationError) + + +async def test_validation_error_raised_inside_the_tool_body_is_a_crash(caplog: pytest.LogCaptureFixture): + """SDK-defined: only the SDK's own argument validation is anticipated; a pydantic + ValidationError from the tool's code is logged as a crash with its traceback.""" + mcp = MCPServer() + + class Row(BaseModel): + n: int + + @mcp.tool() + def parse() -> str: + Row.model_validate({"n": "x"}) + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("parse", {}) + + assert result.is_error is True + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'parse' raised an unexpected exception", True)]) + assert isinstance(_cause_chain(_logged_exception(caplog))[-1], ValidationError) + + +async def test_return_value_failing_the_output_schema_is_a_crash(caplog: pytest.LogCaptureFixture): + """SDK-defined: a return value that doesn't match the declared output schema is the tool's + bug, so it is logged as a crash even though the model still gets an is_error result.""" + mcp = MCPServer() + + class Weather(BaseModel): + temperature: float + + @mcp.tool() + def get_weather() -> Weather: + reading: Any = {"temperature": "warm"} + return reading + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("get_weather", {}) + + assert result.is_error is True + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'get_weather' raised an unexpected exception", True)]) + + +async def test_unknown_tool_is_logged_at_info_without_traceback(caplog: pytest.LogCaptureFixture): + """SDK-defined: a call to a name that was never registered is the caller's mistake, logged + as one INFO record alongside the is_error result.""" + mcp = MCPServer() + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("nope", {}) + + assert result.is_error is True + assert _server_records(caplog) == snapshot([("INFO", "Tool 'nope' failed: 'Unknown tool: nope'", False)]) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_tool_raising_mcp_error_is_not_logged_by_mcpserver(caplog: pytest.LogCaptureFixture): + """SDK-defined: MCPError is a protocol answer the tool chose, so MCPServer writes no record for it.""" + mcp = MCPServer() + + @mcp.tool() + def gated() -> str: + raise MCPError(code=INVALID_PARAMS, message="not for you") + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.call_tool("gated", {}) + + assert exc.value.error.code == INVALID_PARAMS + assert _server_records(caplog) == [] + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_resolver_raising_tool_error_is_anticipated(caplog: pytest.LogCaptureFixture): + """SDK-defined: a ToolError from a Resolve() resolver is classified like one from the tool + body - INFO, no traceback.""" + mcp = MCPServer(name="resolvers", request_state_security=RequestStateSecurity.ephemeral()) + + async def current_user(ctx: Context) -> str: + raise ToolError("sign in first") + + @mcp.tool() + async def whoami(user: Annotated[str, Resolve(current_user)]) -> str: + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("whoami", {}) + + assert result.content == [TextContent(type="text", text="Error executing tool whoami: sign in first")] + assert _server_records(caplog) == snapshot( + [("INFO", "Tool 'whoami' failed: 'Error executing tool whoami: sign in first'", False)] + ) + + +async def test_resolver_crash_is_logged_as_the_tools_crash(caplog: pytest.LogCaptureFixture): + """SDK-defined: an unexpected exception in a Resolve() resolver is the tool's crash - ERROR + with a traceback reaching the resolver's exception.""" + mcp = MCPServer(name="resolvers", request_state_security=RequestStateSecurity.ephemeral()) + raised = ConnectionError("user directory unreachable") + + async def current_user(ctx: Context) -> str: + raise raised + + @mcp.tool() + async def whoami(user: Annotated[str, Resolve(current_user)]) -> str: + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("whoami", {}) + + assert result.is_error is True + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'whoami' raised an unexpected exception", True)]) + assert raised in _cause_chain(_logged_exception(caplog)) + + +async def test_static_resource_raising_unexpected_exception_is_logged_once_at_error_with_its_traceback( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: the client gets a -32603 naming only the URI, and the withheld original is + logged exactly once, at ERROR, with its traceback.""" + mcp = MCPServer() + raised = RuntimeError("connection pool exhausted") + + @mcp.resource("db://stats") + def stats() -> str: + raise raised + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.read_resource("db://stats") + + assert exc.value.error == snapshot( + ErrorData(code=INTERNAL_ERROR, message="Error reading resource db://stats", data={"uri": "db://stats"}) + ) + assert _server_records(caplog) == snapshot( + [("ERROR", "Resource 'db://stats' raised an unexpected exception", True)] + ) + assert raised in _cause_chain(_logged_exception(caplog)) + assert len([r for r in caplog.records if r.levelno >= logging.WARNING]) == 1 + + +async def test_resource_template_raising_unexpected_exception_is_logged_once_at_error_with_its_traceback( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: a template handler crash surfaces as -32603 naming only the URI, and the + withheld original is logged exactly once, at ERROR, with its traceback.""" + mcp = MCPServer() + raised = RuntimeError("connection pool exhausted") + + @mcp.resource("db://tables/{table}") + def describe(table: str) -> str: + raise raised + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.read_resource("db://tables/users") + + assert exc.value.error == snapshot( + ErrorData( + code=INTERNAL_ERROR, + message="Error creating resource from template db://tables/users", + data={"uri": "db://tables/users"}, + ) + ) + assert _server_records(caplog) == snapshot( + [("ERROR", "Resource 'db://tables/users' raised an unexpected exception", True)] + ) + assert raised in _cause_chain(_logged_exception(caplog)) + assert len([r for r in caplog.records if r.levelno >= logging.WARNING]) == 1 + + +async def test_read_resource_wraps_a_crash_as_unexpected_resource_error_chained_to_the_original(): + """SDK-defined: for static and template resources alike, a programmatic caller gets + UnexpectedResourceError naming only the URI, with `__cause__` the handler's own exception.""" + mcp = MCPServer() + raised = RuntimeError("connection pool exhausted") + + @mcp.resource("db://stats") + def stats() -> str: + raise raised + + @mcp.resource("db://tables/{table}") + def describe(table: str) -> str: + raise raised + + with pytest.raises(UnexpectedResourceError) as static: + await mcp.read_resource("db://stats") + with pytest.raises(UnexpectedResourceError) as template: + await mcp.read_resource("db://tables/users") + + assert str(static.value) == snapshot("Error reading resource db://stats") + assert static.value.__cause__ is raised + assert str(template.value) == snapshot("Error creating resource from template db://tables/users") + assert template.value.__cause__ is raised + + +async def test_custom_resource_subclass_crash_is_wrapped_and_logged_like_a_function_resource( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: a hand-written Resource subclass whose read() raises gets the same treatment as + a decorated function - -32603 naming only the URI, one ERROR record chaining to the original.""" + raised = OSError("sensor bus offline") + + class SensorResource(MCPServerResource): + async def read(self) -> str: + raise raised + + mcp = MCPServer() + mcp.add_resource(SensorResource(uri="sensor://temp", name="temp")) + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.read_resource("sensor://temp") + + assert exc.value.error == snapshot( + ErrorData(code=INTERNAL_ERROR, message="Error reading resource sensor://temp", data={"uri": "sensor://temp"}) + ) + assert _server_records(caplog) == snapshot( + [("ERROR", "Resource 'sensor://temp' raised an unexpected exception", True)] + ) + logged = _logged_exception(caplog) + assert isinstance(logged, UnexpectedResourceError) and logged.__cause__ is raised + + +async def test_static_resource_raising_resource_not_found_error_is_invalid_params_logged_at_info( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: ResourceNotFoundError from a static resource handler passes through as -32602 + with the handler's message, as it does from a template handler, and is logged at INFO.""" + mcp = MCPServer() + + @mcp.resource("reports://latest") + def latest() -> str: + raise ResourceNotFoundError("no report has been generated yet") + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.read_resource("reports://latest") + + assert exc.value.error == snapshot( + ErrorData(code=INVALID_PARAMS, message="no report has been generated yet", data={"uri": "reports://latest"}) + ) + assert _server_records(caplog) == snapshot( + [("INFO", "Resource 'reports://latest' failed: 'no report has been generated yet'", False)] + ) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_deliberate_resource_error_passes_its_message_through_and_is_logged_at_info( + caplog: pytest.LogCaptureFixture, +): + """SDK-defined: a ResourceError the handler raised on purpose reaches the client as -32603 with + the handler's message, from a static resource as from a template, and is one INFO record each.""" + mcp = MCPServer() + + @mcp.resource("db://stats") + def stats() -> str: + raise ResourceError("stats database is in maintenance") + + @mcp.resource("db://tables/{table}") + def describe(table: str) -> str: + raise ResourceError(f"table {table} is being rebuilt") + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as static: + await client.read_resource("db://stats") + with pytest.raises(MCPError) as template: + await client.read_resource("db://tables/users") + + assert static.value.error == snapshot( + ErrorData(code=INTERNAL_ERROR, message="stats database is in maintenance", data={"uri": "db://stats"}) + ) + assert template.value.error == snapshot( + ErrorData(code=INTERNAL_ERROR, message="table users is being rebuilt", data={"uri": "db://tables/users"}) + ) + assert _server_records(caplog) == snapshot( + [ + ("INFO", "Resource 'db://stats' failed: 'stats database is in maintenance'", False), + ("INFO", "Resource 'db://tables/users' failed: 'table users is being rebuilt'", False), + ] + ) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_prompt_raising_unexpected_exception_is_logged_once(caplog: pytest.LogCaptureFixture): + """SDK-defined: a prompt crash is logged exactly once, by the dispatcher boundary that turns it + into the JSON-RPC error, and not a second time by MCPServer.""" + mcp = MCPServer() + raised = RuntimeError("template store unreachable") + + @mcp.prompt() + def briefing() -> str: + raise raised + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.get_prompt("briefing") + + assert exc.value.error.code == INTERNAL_ERROR + assert _server_records(caplog) == [] + (record,) = [r for r in caplog.records if r.levelno >= logging.WARNING] + assert record.levelno == logging.ERROR + assert record.exc_info is not None and raised in _cause_chain(record.exc_info[1]) + + +async def test_call_tool_wraps_a_crash_as_unexpected_tool_error_chained_to_the_original(): + """SDK-defined: programmatic callers can tell a crash from a deliberate ToolError by type and + reach the original exception through `__cause__`.""" + mcp = MCPServer() + raised = RuntimeError("boom") + + @mcp.tool() + def explode() -> str: + raise raised + + with pytest.raises(UnexpectedToolError) as exc: + await mcp.call_tool("explode", {}) + assert str(exc.value) == snapshot("Error executing tool explode: boom") + assert exc.value.__cause__ is raised + + +async def test_call_tool_keeps_a_deliberate_tool_error_a_plain_tool_error(): + """SDK-defined: a ToolError raised by the tool is re-raised as a plain ToolError carrying the + tool-name prefix, never reclassified as unexpected.""" + mcp = MCPServer() + + @mcp.tool() + def refuse() -> str: + raise ToolError("not today") + + with pytest.raises(ToolError) as exc: + await mcp.call_tool("refuse", {}) + assert type(exc.value) is ToolError + assert str(exc.value) == snapshot("Error executing tool refuse: not today") + + +async def test_nested_tool_crash_stays_unexpected_through_the_outer_tool(caplog: pytest.LogCaptureFixture): + """SDK-defined: when a tool awaits another tool that crashes, the outer wrapper keeps the + UnexpectedToolError classification, so the crash is still logged once with its traceback.""" + mcp = MCPServer() + raised = ZeroDivisionError("division by zero") + + @mcp.tool() + def inner() -> str: + raise raised + + @mcp.tool() + async def outer(ctx: Context) -> str: + await ctx.mcp_server.call_tool("inner", {}) + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("outer", {}) + + assert result.content == [ + TextContent(type="text", text="Error executing tool outer: Error executing tool inner: division by zero") + ] + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'outer' raised an unexpected exception", True)]) + assert raised in _cause_chain(_logged_exception(caplog)) + + async def test_context_exposes_client_capabilities_from_connection(): mcp = MCPServer() seen: list[ClientCapabilities | None] = [] From 96b5cc8639bf69a51ebca555bbb8c59a9d075d92 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Sun, 16 Aug 2026 12:11:03 +0000 Subject: [PATCH 2/6] Inline handler logging and trim the docs 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. --- docs/handlers/logging.md | 2 +- docs/servers/handling-errors.md | 19 ++++---- docs/servers/uri-templates.md | 5 +-- docs/troubleshooting.md | 2 +- src/mcp/server/mcpserver/exceptions.py | 8 ++-- .../server/mcpserver/resources/templates.py | 2 +- src/mcp/server/mcpserver/resources/types.py | 2 +- src/mcp/server/mcpserver/server.py | 44 ++++++++----------- tests/docs_src/test_handling_errors.py | 5 +-- tests/interaction/_requirements.py | 23 ---------- tests/interaction/mcpserver/test_prompts.py | 32 -------------- tests/interaction/mcpserver/test_resources.py | 32 -------------- tests/interaction/mcpserver/test_tools.py | 31 ------------- 13 files changed, 39 insertions(+), 168 deletions(-) diff --git a/docs/handlers/logging.md b/docs/handlers/logging.md index 1c839d70b5..bac877a8d3 100644 --- a/docs/handlers/logging.md +++ b/docs/handlers/logging.md @@ -70,7 +70,7 @@ went to standard error: the terminal, not the wire. don't want log lines, you want spans. Your server already emits them: the SDK traces every message with OpenTelemetry out of the box. See **[OpenTelemetry](../run/opentelemetry.md)**. -You don't have to log your own handlers' crashes either. When a tool or resource function raises something unexpected, the SDK writes the `ERROR` record with the traceback for you, on its own `mcp.*` loggers; a failure you raised deliberately (`ToolError`, `ResourceNotFoundError`) is an `INFO` line instead. A prompt function that raises is an `ERROR` record too, whatever it raised. **[Handling errors](../servers/handling-errors.md#what-lands-in-your-log)** has the split. (In a test using `Client(mcp, raise_exceptions=True)`, a prompt failure is handed to your test as the exception rather than logged.) +You also don't need a `try`/`except` in every handler just to record failures. When a tool or resource function raises, the SDK logs it for you. **[Handling errors](../servers/handling-errors.md#what-the-server-logs)** explains what gets logged and at which level. ## Recap diff --git a/docs/servers/handling-errors.md b/docs/servers/handling-errors.md index 6519889312..9e7cedff2f 100644 --- a/docs/servers/handling-errors.md +++ b/docs/servers/handling-errors.md @@ -118,14 +118,14 @@ It means a whole class of `raise` statements you don't write: don't re-validate Everything so far is what a **client** sees, and the in-memory `Client` you'll write tests with sees exactly the same thing. Even `raise_exceptions=True` doesn't hand a failing tool's exception back to the caller: by the time that flag could act, your exception is already the - `is_error=True` result. Assert on the result; the traceback is in the server's log (next - section), which pytest's `caplog` captures. **[Testing](../get-started/testing.md)** covers the pattern. + `is_error=True` result. Assert on the result. If you need the traceback, it is in the server's + log (next section), and pytest's `caplog` captures it. **[Testing](../get-started/testing.md)** covers the pattern. -## What lands in your log +## What the server logs -Your server keeps its own record of these failures, and it draws one more line: between a failure you anticipated and one you didn't. +The server also logs these failures, and how it logs them depends on whether you anticipated the failure. -`get_author` raised a plain `ValueError`. The model got the message, but the SDK can't know you *meant* that exception, so it assumes you didn't: the call is logged at `ERROR` with the full traceback. That is exactly what you want on the day the exception is a `KeyError` from three libraries down and the result text says only `'id'`. +`get_author` raised a plain `ValueError`. The model got the message, but the SDK can't tell that you raised it on purpose, so it treats the call as a crash and logs it at `ERROR` with the full traceback. That is what you want on the day the exception is a `KeyError` from deep inside a library and the result text says only `'id'`. When the failure is one you planned for, say so with `ToolError`: @@ -133,11 +133,9 @@ When the failure is one you planned for, say so with `ToolError`: --8<-- "docs_src/handling_errors/tutorial004.py" ``` -The model reads precisely what it read before. The difference is on your side: a `ToolError` is logged as one `INFO` line with no traceback, so a production log at `WARNING` stays quiet until something is actually broken. Bad arguments and unknown tool names are `INFO` lines too; those are the caller's mistakes, not yours. +`ToolError` comes from `mcp.server.mcpserver.exceptions`. The model reads exactly what it read before. The difference is in your log, where a `ToolError` is a single `INFO` line with no traceback, so a production log at `WARNING` stays quiet until something is actually broken. Bad arguments and unknown tool names are logged at `INFO` too, because those are the caller's mistakes rather than yours. -Resources draw the same line. The `-32603` from a crashing resource handler names only the URI, so the `ERROR` record in your log is the one place the cause and its traceback exist. `ResourceNotFoundError`, including the SDK's own `Unknown resource`, is an `INFO` line. (A template parameter that fails its type annotation, `books://{id}` read with an `id` that isn't an `int`, currently counts as a crash.) - -Prompts aren't split yet: any failure in a prompt function, including an unknown name or a missing argument, is one `ERROR` record with its traceback, written by the transport layer that turns it into the JSON-RPC error. +Resources work the same way. A crashing resource handler is logged at `ERROR` with its traceback, which matters more here because the `-32603` the client receives names only the URI. `ResourceNotFoundError` is an `INFO` line. ## Recap @@ -146,8 +144,7 @@ Prompts aren't split yet: any failure in a prompt function, including an unknown * The deciding question: *could a smarter model have avoided this?* Yes -> exception. No -> `MCPError`. * `ResourceNotFoundError` from a resource handler -> the protocol's `-32602`, with the URI in `data`. * Bad arguments are rejected against the schema before your function runs; you don't `raise` for those. -* In your log: an exception you didn't raise as `ToolError` is an `ERROR` record with its traceback; `ToolError`, bad tool arguments, unknown tool names, and `ResourceNotFoundError` are one `INFO` line each. -* `from mcp import MCPError`; `ToolError` and `ResourceNotFoundError` come from `mcp.server.mcpserver.exceptions`; the error-code constants come from `mcp.types`. +* `from mcp import MCPError`; the error-code constants come from `mcp.types`. 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)**. diff --git a/docs/servers/uri-templates.md b/docs/servers/uri-templates.md index a79889b2af..5a0d1f0575 100644 --- a/docs/servers/uri-templates.md +++ b/docs/servers/uri-templates.md @@ -201,9 +201,8 @@ These checks are a heuristic pre-filter; for filesystem access, !!! tip If your handler can't fulfil the request (the file doesn't exist, the id is unknown), raise `ResourceNotFoundError` from `mcp.server.mcpserver.exceptions`. The client gets `-32602` with - your message and the URI, and your log gets one `INFO` line; any other exception is treated as - a crash (`-32603`, and an `ERROR` record with the traceback). See - **[Handling errors](handling-errors.md#a-resource-that-doesnt-exist)**. + your message and the URI. Any other exception is treated as a crash and the client gets a + generic `-32603`. See **[Handling errors](handling-errors.md#a-resource-that-doesnt-exist)**. ## Resources on the low-level Server diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f549dfbadd..49d972ce25 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -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. -If `` alone doesn't tell you what broke, the traceback is in the **server's log**: an exception the tool didn't raise as `ToolError` is logged there at `ERROR`, as `Tool '' raised an unexpected exception`. +If `` alone doesn't tell you what broke, look in the **server's log**. Unless the tool raised `ToolError`, the exception is logged there at `ERROR` with its traceback, as `Tool '' raised an unexpected exception`. ## `TypeError: The @tool decorator was used incorrectly. Did you forget to call it? Use @tool() instead of @tool` diff --git a/src/mcp/server/mcpserver/exceptions.py b/src/mcp/server/mcpserver/exceptions.py index 9f2415d976..a2cd0c1d8c 100644 --- a/src/mcp/server/mcpserver/exceptions.py +++ b/src/mcp/server/mcpserver/exceptions.py @@ -16,8 +16,8 @@ class ResourceError(MCPServerError): class ResourceNotFoundError(ResourceError): """Resource does not exist. - Raise this from a resource handler to signal that the requested instance does not exist; - clients receive `-32602` (invalid params) per + Raise this from a resource handler to signal that the requested instance does not exist. + Clients receive `-32602` (invalid params) per [SEP-2164](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2164). """ @@ -26,7 +26,7 @@ class UnexpectedResourceError(ResourceError): """A resource read failed with something other than `ResourceError` or `MCPError`. MCPServer raises this itself, around a crash in a resource or resource - template handler or a failed file read; you never raise it. `__cause__` is + template handler or a failed file read. You never raise it. `__cause__` is the original exception, which the server logs with its traceback. The message names only the URI, so the original text is withheld from the client. """ @@ -50,7 +50,7 @@ class UnexpectedToolError(ToolError): """A tool call failed with something other than `ToolError` or `MCPError`. MCPServer raises this itself, around a crash in the tool (or a resolver) or a - return value that fails output conversion; you never raise it. `__cause__` is + return value that fails output conversion. You never raise it. `__cause__` is the original exception, which the server logs with its traceback before returning the usual `is_error=True` result. Catch it around `MCPServer.call_tool()` to tell a crash from a deliberate `ToolError`. diff --git a/src/mcp/server/mcpserver/resources/templates.py b/src/mcp/server/mcpserver/resources/templates.py index 7c789dc57d..0afa9b4adc 100644 --- a/src/mcp/server/mcpserver/resources/templates.py +++ b/src/mcp/server/mcpserver/resources/templates.py @@ -217,7 +217,7 @@ async def create_resource( Raises: ResourceError: If the template function raises `ResourceError`. UnexpectedResourceError: If the template function raises anything other - than `ResourceError` or `MCPError`; `__cause__` is the original. + than `ResourceError` or `MCPError`. `__cause__` is the original exception. """ try: # Add context to params if needed diff --git a/src/mcp/server/mcpserver/resources/types.py b/src/mcp/server/mcpserver/resources/types.py index f3751989d7..f77b32b610 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -84,7 +84,7 @@ async def read(self) -> str | bytes: Raises: UnexpectedResourceError: If the function raises anything other than - `ResourceError` or `MCPError`; `__cause__` is the original. + `ResourceError` or `MCPError`. `__cause__` is the original exception. """ try: fn = self.fn diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index 93319cf273..168a7df672 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -427,7 +427,14 @@ async def _handle_call_tool( except MCPError: raise except Exception as exc: - _log_handler_exception("Tool", params.name, exc) + # A ToolError (deliberate, unknown tool, rejected arguments) is an outcome + # the model already reads in full, so it is one INFO record, repr-quoted to + # keep peer-supplied text on one line. Anything else is a crash in the + # tool: log the traceback that the result text doesn't carry. + if isinstance(exc, ToolError) and not isinstance(exc, UnexpectedToolError): + logger.info("Tool %r failed: %r", params.name, str(exc)) + else: + logger.exception("Tool %r raised an unexpected exception", params.name) return CallToolResult(content=[TextContent(type="text", text=str(exc))], is_error=True) async def _handle_list_resources( @@ -442,7 +449,13 @@ async def _handle_read_resource( try: results = await self.read_resource(params.uri, context) except ResourceError as err: - _log_handler_exception("Resource", str(params.uri), err) + # UnexpectedResourceError wraps a crash whose text is withheld from the + # client, so the traceback goes to the log. Any other ResourceError was + # raised on purpose (or is the SDK's "Unknown resource") and is one INFO record. + if isinstance(err, UnexpectedResourceError): + logger.exception("Resource %r raised an unexpected exception", str(params.uri)) + else: + logger.info("Resource %r failed: %r", str(params.uri), str(err)) code = INVALID_PARAMS if isinstance(err, ResourceNotFoundError) else INTERNAL_ERROR raise MCPError(code=code, message=str(err), data={"uri": str(params.uri)}) if isinstance(results, InputRequiredResult): @@ -511,8 +524,8 @@ async def call_tool( ToolError: If the tool is unknown, the arguments fail validation, or the tool (or a resolver) raises `ToolError`. 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. + `ToolError` or `MCPError`, or its return value fails output conversion. + `__cause__` is the original exception. """ if context is None: context = Context(mcp_server=self, subscriptions=self._subscriptions) @@ -566,8 +579,8 @@ async def read_resource( ResourceNotFoundError: If no resource or template matches the URI. ResourceError: If the resource or template function raises `ResourceError`. UnexpectedResourceError: If reading the resource (or creating it from a - template) raises anything other than `ResourceError` or `MCPError`; - `__cause__` is the original. + template) raises anything other than `ResourceError` or `MCPError`. + `__cause__` is the original exception. """ if context is None: context = Context(mcp_server=self, subscriptions=self._subscriptions) @@ -1320,25 +1333,6 @@ async def get_prompt( raise ValueError(str(e)) from e -def _log_handler_exception(kind: Literal["Tool", "Resource"], name: str, exc: Exception) -> None: - """Record a tool or resource handler failure; the one place MCPServer logs them. - - Called from the `except` block that turns the failure into a response. A - `ToolError` or `ResourceError` (deliberate, an unknown name, arguments that - failed validation, `ResourceNotFoundError`) is an anticipated outcome the - client already receives in full: one INFO record, no traceback, the text - repr-quoted so peer-supplied names and newlines stay on one line. Anything - else, including the `Unexpected*` wrappers whose `__cause__` is what the - handler actually raised, is a crash in user code: ERROR with the traceback. - """ - if isinstance(exc, ToolError | ResourceError) and not isinstance( - exc, UnexpectedToolError | UnexpectedResourceError - ): - logger.info("%s %r failed: %r", kind, name, str(exc)) - else: - logger.exception("%s %r raised an unexpected exception", kind, name, exc_info=exc) - - def _version_gated(method: MethodBinding) -> RequestHandler: """Wrap a method handler so a request at a disallowed protocol version is rejected. diff --git a/tests/docs_src/test_handling_errors.py b/tests/docs_src/test_handling_errors.py index e2aab0e423..8872ba7b4c 100644 --- a/tests/docs_src/test_handling_errors.py +++ b/tests/docs_src/test_handling_errors.py @@ -90,7 +90,7 @@ async def test_a_title_the_template_knows_reads_normally() -> None: async def test_a_plain_exception_is_logged_as_a_crash_with_its_traceback(caplog: pytest.LogCaptureFixture) -> None: - """tutorial001, "What lands in your log": the `ValueError` is one ERROR record carrying the traceback.""" + """tutorial001, "What the server logs": the `ValueError` is one ERROR record carrying the traceback.""" caplog.set_level(logging.INFO) async with Client(tutorial001.mcp) as client: await client.call_tool("get_author", {"title": "Nothing"}) @@ -99,7 +99,6 @@ async def test_a_plain_exception_is_logged_as_a_crash_with_its_traceback(caplog: assert record.exc_info is not None logged = record.exc_info[1] assert logged is not None and isinstance(logged.__cause__, ValueError) - assert str(logged.__cause__) == "No book titled 'Nothing' in the catalog." async def test_tool_error_reads_the_same_to_the_model_and_logs_one_info_line( @@ -119,7 +118,7 @@ async def test_tool_error_reads_the_same_to_the_model_and_logs_one_info_line( async def test_a_bad_argument_is_an_info_line_not_a_crash(caplog: pytest.LogCaptureFixture) -> None: - """ "What lands in your log": schema rejection of the arguments is logged at INFO with no traceback.""" + """ "What the server logs": schema rejection of the arguments is logged at INFO with no traceback.""" caplog.set_level(logging.INFO) async with Client(tutorial001.mcp) as client: result = await client.call_tool("get_author", {"title": 42}) diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 3b5f819f18..964a1829d2 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -1020,14 +1020,6 @@ def __post_init__(self) -> None: "tool result with isError true and the failure text in content; it does not become a JSON-RPC error." ), ), - "mcpserver:tool:handler-throws:logged": Requirement( - source="sdk", - behavior=( - "An exception other than ToolError raised by a tool function is logged server-side exactly once, " - "at ERROR with its traceback, before the isError result is returned; the transport does not change " - "how many records are written." - ), - ), "mcpserver:tool:input-validation": Requirement( source=f"{SPEC_BASE_URL}/server/tools#error-handling", behavior=( @@ -1319,13 +1311,6 @@ def __post_init__(self) -> None: "(-32603 Internal error), with the original exception text withheld." ), ), - "mcpserver:resource:read-throws:logged": Requirement( - source="sdk", - behavior=( - "The exception withheld from the -32603 response is logged server-side exactly once, at ERROR " - "with its traceback; the transport does not change how many records are written." - ), - ), "mcpserver:resource:static": Requirement( source="sdk", behavior=( @@ -1442,14 +1427,6 @@ def __post_init__(self) -> None: source="sdk", behavior="A prompt with optional arguments can be fetched without supplying them.", ), - "mcpserver:prompt:render-throws:logged": Requirement( - source="sdk", - behavior=( - "An exception raised by a prompt function is logged server-side exactly once, at ERROR with its " - "traceback, by whichever layer turns it into the JSON-RPC error; the transport does not change how " - "many records are written." - ), - ), "mcpserver:prompt:unknown-name": Requirement( source=f"{SPEC_BASE_URL}/server/prompts#error-handling", behavior="prompts/get for a name that was never registered returns JSON-RPC error -32602 (Invalid params).", diff --git a/tests/interaction/mcpserver/test_prompts.py b/tests/interaction/mcpserver/test_prompts.py index 5a357592ae..8409e50207 100644 --- a/tests/interaction/mcpserver/test_prompts.py +++ b/tests/interaction/mcpserver/test_prompts.py @@ -1,7 +1,5 @@ """Prompt interactions against MCPServer, driven through the public Client API.""" -import logging - import pytest from inline_snapshot import snapshot from mcp_types import ( @@ -143,36 +141,6 @@ def repeat(phrase: str, count: int) -> str: assert exc_info.value.error.message.startswith("Error rendering prompt repeat: 1 validation error") -@requirement("mcpserver:prompt:render-throws:logged") -async def test_get_prompt_function_exception_is_logged_once_with_its_traceback( - connect: Connect, caplog: pytest.LogCaptureFixture -) -> None: - """An exception raised by a prompt function is logged exactly once, at ERROR, with its traceback. - - MCPServer lets the failure escape to the dispatcher boundary, which owns both the JSON-RPC error and - the log record; the owning logger therefore differs by transport, but the count must not. - """ - mcp = MCPServer("prompter") - raised = RuntimeError("template store unreachable") - - @mcp.prompt() - def briefing() -> str: - raise raised - - caplog.set_level(logging.ERROR) - async with connect(mcp) as client: - with pytest.raises(MCPError): - await client.get_prompt("briefing") - - def chains_to_raised(exc: BaseException | None) -> bool: - while exc is not None and exc is not raised: - exc = exc.__cause__ or exc.__context__ - return exc is raised - - records = [r for r in caplog.records if r.exc_info and chains_to_raised(r.exc_info[1])] - assert [r.levelname for r in records] == ["ERROR"] - - @requirement("mcpserver:prompt:optional-args") async def test_get_prompt_with_an_optional_argument_omitted_uses_the_default( connect: Connect, unstamped: Unstamp diff --git a/tests/interaction/mcpserver/test_resources.py b/tests/interaction/mcpserver/test_resources.py index 162fa9a813..eadf4794e6 100644 --- a/tests/interaction/mcpserver/test_resources.py +++ b/tests/interaction/mcpserver/test_resources.py @@ -1,7 +1,5 @@ """Resource interactions against MCPServer, driven through the public Client API.""" -import logging - import pytest from inline_snapshot import snapshot from mcp_types import ( @@ -154,36 +152,6 @@ def boom() -> str: ) -@requirement("mcpserver:resource:read-throws:logged") -async def test_resource_function_exception_is_logged_once_with_its_traceback( - connect: Connect, caplog: pytest.LogCaptureFixture -) -> None: - """The exception withheld from the -32603 response is logged exactly once, at ERROR, with its traceback. - - The client sees only the URI, so this record is the operator's only route to the cause; it must be - written on every transport and never duplicated by a dispatcher boundary. - """ - mcp = MCPServer("library") - raised = RuntimeError("nope") - - @mcp.resource("res://boom") - def boom() -> str: - raise raised - - caplog.set_level(logging.ERROR) - async with connect(mcp) as client: - with pytest.raises(MCPError): - await client.read_resource("res://boom") - - def chains_to_raised(exc: BaseException | None) -> bool: - while exc is not None and exc is not raised: - exc = exc.__cause__ or exc.__context__ - return exc is raised - - records = [r for r in caplog.records if r.exc_info and chains_to_raised(r.exc_info[1])] - assert [(r.name, r.levelname) for r in records] == [("mcp.server.mcpserver.server", "ERROR")] - - @requirement("mcpserver:resource:duplicate-name") async def test_registering_a_duplicate_resource_uri_warns_and_keeps_the_first( connect: Connect, unstamped: Unstamp diff --git a/tests/interaction/mcpserver/test_tools.py b/tests/interaction/mcpserver/test_tools.py index c275fc60b1..a6418ac9c5 100644 --- a/tests/interaction/mcpserver/test_tools.py +++ b/tests/interaction/mcpserver/test_tools.py @@ -119,37 +119,6 @@ def flux() -> str: ) -@requirement("mcpserver:tool:handler-throws:logged") -async def test_call_tool_function_exception_is_logged_once_with_its_traceback( - connect: Connect, caplog: pytest.LogCaptureFixture -) -> None: - """The exception behind an is_error result is logged exactly once, at ERROR, with its traceback. - - The result text carries only `str(exc)`; the traceback exists nowhere but this record, so it must - be written on every transport and never duplicated by a dispatcher boundary. - """ - mcp = MCPServer("errors") - raised = LookupError("no such row") - - @mcp.tool() - def explode() -> str: - raise raised - - caplog.set_level(logging.ERROR) - async with connect(mcp) as client: - result = await client.call_tool("explode", {}) - - assert result.is_error is True - - def chains_to_raised(exc: BaseException | None) -> bool: - while exc is not None and exc is not raised: - exc = exc.__cause__ or exc.__context__ - return exc is raised - - records = [r for r in caplog.records if r.exc_info and chains_to_raised(r.exc_info[1])] - assert [(r.name, r.levelname) for r in records] == [("mcp.server.mcpserver.server", "ERROR")] - - @requirement("mcpserver:tool:unknown-name") async def test_call_tool_unknown_name_returns_error_result(connect: Connect, unstamped: Unstamp) -> None: """Calling a tool name that was never registered is reported as an is_error result. From 6fd5e05a322acaf91804302f5a8646ebac46f358 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:13:05 +0000 Subject: [PATCH 3/6] Wrap crashing validators, treat ResourceError in a tool as anticipated 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). --- docs/handlers/logging.md | 4 +- docs/migration.md | 2 +- docs/servers/handling-errors.md | 4 +- docs/servers/uri-templates.md | 8 +- docs/troubleshooting.md | 2 +- docs_src/uri_templates/tutorial002.py | 6 +- src/mcp/server/mcpserver/context.py | 8 +- src/mcp/server/mcpserver/exceptions.py | 32 ++-- .../server/mcpserver/resources/templates.py | 2 - src/mcp/server/mcpserver/resources/types.py | 87 ++++------- src/mcp/server/mcpserver/server.py | 24 +-- src/mcp/server/mcpserver/tools/base.py | 34 +++-- tests/docs_src/test_uri_templates.py | 13 ++ tests/interaction/_requirements.py | 11 +- tests/interaction/mcpserver/test_resources.py | 23 +++ .../resources/test_file_resources.py | 8 +- .../resources/test_function_resources.py | 17 +-- tests/server/mcpserver/test_resolve.py | 9 +- tests/server/mcpserver/test_server.py | 137 +++++++++++++++++- 19 files changed, 296 insertions(+), 135 deletions(-) diff --git a/docs/handlers/logging.md b/docs/handlers/logging.md index bac877a8d3..2370750d82 100644 --- a/docs/handlers/logging.md +++ b/docs/handlers/logging.md @@ -49,6 +49,8 @@ The default is `"INFO"`. `logging.basicConfig()` never replaces handlers that already exist. If you configure logging yourself before creating the server, your configuration wins. +You also don't need a `try`/`except` in every handler just to record failures. When a tool or resource function raises, the SDK logs it for you. **[Handling errors](../servers/handling-errors.md#what-the-server-logs)** explains what gets logged and at which level. + ## Try it Run the server with the MCP Inspector: @@ -70,8 +72,6 @@ went to standard error: the terminal, not the wire. don't want log lines, you want spans. Your server already emits them: the SDK traces every message with OpenTelemetry out of the box. See **[OpenTelemetry](../run/opentelemetry.md)**. -You also don't need a `try`/`except` in every handler just to record failures. When a tool or resource function raises, the SDK logs it for you. **[Handling errors](../servers/handling-errors.md#what-the-server-logs)** explains what gets logged and at which level. - ## Recap * The MCP protocol's logging capability is deprecated by the 2026-07-28 spec and not replaced. Don't build on it. diff --git a/docs/migration.md b/docs/migration.md index e59b4a6ac6..ed1b2a58b5 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1018,7 +1018,7 @@ except MCPError as e: Reading a missing resource now returns JSON-RPC error code `-32602` (invalid params) with the requested URI in `error.data` (`{"uri": ...}`), per [SEP-2164](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2164). Previously the server returned code `0` with no `data`. Clients can now reliably distinguish not-found from other errors; a resource handler (static or template) that raises `ResourceNotFoundError` (from `mcp.server.mcpserver.exceptions`) produces this same response. -The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). +The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). Likewise, `FunctionResource.read()` and `FileResource.read()` no longer wrap failures in `ValueError`: called directly they raise whatever the function or file read raised, and through `MCPServer.read_resource()` that arrives as `UnexpectedResourceError` (a `ResourceError`) with the original as `__cause__`. ### `Resource` classes reject unknown keyword arguments diff --git a/docs/servers/handling-errors.md b/docs/servers/handling-errors.md index 9e7cedff2f..005396ace9 100644 --- a/docs/servers/handling-errors.md +++ b/docs/servers/handling-errors.md @@ -123,7 +123,7 @@ It means a whole class of `raise` statements you don't write: don't re-validate ## What the server logs -The server also logs these failures, and how it logs them depends on whether you anticipated the failure. +The server also logs tool and resource failures, and how it logs them depends on whether you anticipated the failure. `get_author` raised a plain `ValueError`. The model got the message, but the SDK can't tell that you raised it on purpose, so it treats the call as a crash and logs it at `ERROR` with the full traceback. That is what you want on the day the exception is a `KeyError` from deep inside a library and the result text says only `'id'`. @@ -135,7 +135,7 @@ When the failure is one you planned for, say so with `ToolError`: `ToolError` comes from `mcp.server.mcpserver.exceptions`. The model reads exactly what it read before. The difference is in your log, where a `ToolError` is a single `INFO` line with no traceback, so a production log at `WARNING` stays quiet until something is actually broken. Bad arguments and unknown tool names are logged at `INFO` too, because those are the caller's mistakes rather than yours. -Resources work the same way. A crashing resource handler is logged at `ERROR` with its traceback, which matters more here because the `-32603` the client receives names only the URI. `ResourceNotFoundError` is an `INFO` line. +Resources work the same way. A crashing resource handler is logged at `ERROR` with its traceback, which matters more here because the `-32603` the client receives names only the URI. `ResourceNotFoundError` and `ResourceError` are the anticipated kind and are logged at `INFO`. ## Recap diff --git a/docs/servers/uri-templates.md b/docs/servers/uri-templates.md index 5a0d1f0575..1d6e13c0a0 100644 --- a/docs/servers/uri-templates.md +++ b/docs/servers/uri-templates.md @@ -159,7 +159,7 @@ The built-in checks stop the common cases but can't know your sandbox boundary. For filesystem access, use `safe_join` to resolve the path and verify it stays inside your base directory: -```python title="server.py" hl_lines="4 14" +```python title="server.py" hl_lines="5 15" --8<-- "docs_src/uri_templates/tutorial002.py" ``` @@ -200,9 +200,9 @@ These checks are a heuristic pre-filter; for filesystem access, !!! tip If your handler can't fulfil the request (the file doesn't exist, the id is unknown), raise - `ResourceNotFoundError` from `mcp.server.mcpserver.exceptions`. The client gets `-32602` with - your message and the URI. Any other exception is treated as a crash and the client gets a - generic `-32603`. See **[Handling errors](handling-errors.md#a-resource-that-doesnt-exist)**. + `ResourceNotFoundError` as `read_manual` does above. The client gets `-32602` with your message + and the URI. An unexpected exception becomes a generic `-32603` instead. See + **[Handling errors](handling-errors.md#a-resource-that-doesnt-exist)**. ## Resources on the low-level Server diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 49d972ce25..2d53123a69 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -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. -If `` alone doesn't tell you what broke, look in the **server's log**. Unless the tool raised `ToolError`, the exception is logged there at `ERROR` with its traceback, as `Tool '' raised an unexpected exception`. +If `` alone doesn't tell you what broke and the tool crashed (rather than raising `ToolError`, being unknown, or rejecting an argument), the traceback is in the **server's log** at `ERROR`, as `Tool '' raised an unexpected exception`. ## `TypeError: The @tool decorator was used incorrectly. Did you forget to call it? Use @tool() instead of @tool` diff --git a/docs_src/uri_templates/tutorial002.py b/docs_src/uri_templates/tutorial002.py index 2ad1ec7c1e..ebe16cc680 100644 --- a/docs_src/uri_templates/tutorial002.py +++ b/docs_src/uri_templates/tutorial002.py @@ -1,6 +1,7 @@ from pathlib import Path from mcp.server import MCPServer +from mcp.server.mcpserver.exceptions import ResourceNotFoundError from mcp.shared.path_security import safe_join mcp = MCPServer("Bookshop") @@ -11,4 +12,7 @@ @mcp.resource("manuals://{+path}") def read_manual(path: str) -> str: """A staff manual page, served from a directory on disk.""" - return safe_join(DOCS_ROOT, path).read_text() + file = safe_join(DOCS_ROOT, path) + if not file.is_file(): + raise ResourceNotFoundError(f"No manual at {path!r}.") + return file.read_text() diff --git a/src/mcp/server/mcpserver/context.py b/src/mcp/server/mcpserver/context.py index bf4c26a248..07c4799dc1 100644 --- a/src/mcp/server/mcpserver/context.py +++ b/src/mcp/server/mcpserver/context.py @@ -169,8 +169,12 @@ async def read_resource(self, uri: str | AnyUrl) -> Iterable[ReadResourceContent The resource content as either text or bytes Raises: - ResourceNotFoundError: If no resource or template matches the URI. - ResourceError: If template creation or resource reading fails. + ResourceNotFoundError: If no resource or template matches the URI, or the + handler raised it. + ResourceError: If the resource or template function raises `ResourceError`. + UnexpectedResourceError: If the resource or template function raises anything + else. `__cause__` is the original exception. Left uncaught in a tool, this + is logged as the tool's crash, while the two above are not. RuntimeError: If the resource returned an `InputRequiredResult`. """ assert self._mcp_server is not None, "Context is not available outside of a request" diff --git a/src/mcp/server/mcpserver/exceptions.py b/src/mcp/server/mcpserver/exceptions.py index a2cd0c1d8c..0a39d61981 100644 --- a/src/mcp/server/mcpserver/exceptions.py +++ b/src/mcp/server/mcpserver/exceptions.py @@ -6,10 +6,18 @@ class MCPServerError(Exception): class ResourceError(MCPServerError): - """Error in resource operations. - - When a resource or resource template handler raises this, its message reaches - the client as a `-32603` protocol error. + """A resource failure you anticipated. + + Raise this from a resource or resource template handler for a failure you saw + coming: the client receives a `-32603` protocol error carrying your message + (`ResourceNotFoundError` below is the `-32602` variant), and the server logs it + at INFO without a traceback. Any other exception is treated as a crash: the + client gets a generic message naming only the URI, and the server logs the + traceback at ERROR. + + The SDK raises it too, and `UnexpectedResourceError` subclasses it, so + `except ResourceError` around `MCPServer.read_resource()` catches every read + failure, crash or not. """ @@ -25,20 +33,22 @@ class ResourceNotFoundError(ResourceError): class UnexpectedResourceError(ResourceError): """A resource read failed with something other than `ResourceError` or `MCPError`. - MCPServer raises this itself, around a crash in a resource or resource - template handler or a failed file read. You never raise it. `__cause__` is - the original exception, which the server logs with its traceback. The - message names only the URI, so the original text is withheld from the client. + The SDK raises this itself, around a crash in a resource or resource template + handler. You never raise it. `__cause__` is the original exception, which the + server logs with its traceback. The message names only the URI, so the + original text is withheld from the client. """ class ToolError(MCPServerError): - """A tool failure the model should read. + """A tool failure you anticipated. - Raise this from a tool (or a resolver) for a failure you anticipate: the + Raise this from a tool (or a resolver) for a failure you saw coming: the call returns `is_error=True` with the message in `content`, and the server logs it at INFO without a traceback. Any other exception reaches the model the same way but is treated as a crash and logged at ERROR with its traceback. + A `ResourceError` that escapes the tool (say from `ctx.read_resource()`) counts + as anticipated too. The SDK raises it too, for an unknown tool name and for arguments that fail the input schema, and `UnexpectedToolError` subclasses it, so `except ToolError` @@ -49,7 +59,7 @@ class ToolError(MCPServerError): class UnexpectedToolError(ToolError): """A tool call failed with something other than `ToolError` or `MCPError`. - MCPServer raises this itself, around a crash in the tool (or a resolver) or a + The SDK raises this itself, around a crash in the tool (or a resolver) or a return value that fails output conversion. You never raise it. `__cause__` is the original exception, which the server logs with its traceback before returning the usual `is_error=True` result. Catch it around diff --git a/src/mcp/server/mcpserver/resources/templates.py b/src/mcp/server/mcpserver/resources/templates.py index 0afa9b4adc..7be8378c60 100644 --- a/src/mcp/server/mcpserver/resources/templates.py +++ b/src/mcp/server/mcpserver/resources/templates.py @@ -246,6 +246,4 @@ async def create_resource( except (ResourceError, MCPError): raise except Exception as exc: - # Name only the URI: the original text is withheld from the client, and - # the server logs the traceback from `__cause__`. raise UnexpectedResourceError(f"Error creating resource from template {uri}") from exc diff --git a/src/mcp/server/mcpserver/resources/types.py b/src/mcp/server/mcpserver/resources/types.py index f77b32b610..c8b479bb78 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -16,10 +16,8 @@ from mcp_types import Annotations, Icon, InputRequiredResult from pydantic import Field, validate_call -from mcp.server.mcpserver.exceptions import ResourceError, UnexpectedResourceError from mcp.server.mcpserver.resources.base import Resource from mcp.shared._callable_inspection import is_async_callable -from mcp.shared.exceptions import MCPError # `application/*` types that are textual but predate the `+json`/`+xml` # structured-syntax suffixes, so the suffix rule below can't catch them. @@ -80,41 +78,29 @@ class FunctionResource(Resource): fn: Callable[[], Any] = Field(exclude=True) async def read(self) -> str | bytes: - """Read the resource by calling the wrapped function. - - Raises: - UnexpectedResourceError: If the function raises anything other than - `ResourceError` or `MCPError`. `__cause__` is the original exception. - """ - try: - fn = self.fn - if is_async_callable(fn): - result = await fn() - else: - result = await anyio.to_thread.run_sync(self.fn) - - if isinstance(result, InputRequiredResult): - # A static resource function can never read the retry's - # input_responses (it takes no Context), so this can only be a - # mistake — reject it instead of JSON-dumping it as content. - raise ValueError( - "static resources cannot return InputRequiredResult; only resource " - "template functions participate in the multi-round-trip flow" - ) - if isinstance(result, Resource): # pragma: no cover - return await result.read() - elif isinstance(result, bytes): - return result - elif isinstance(result, str): - return result - else: - return pydantic_core.to_json(result, fallback=str, indent=2).decode() - except (MCPError, ResourceError): - raise - except Exception as exc: - # Name only the URI: the original text is withheld from the client, and - # the server logs the traceback from `__cause__`. - raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc + """Read the resource by calling the wrapped function.""" + fn = self.fn + if is_async_callable(fn): + result = await fn() + else: + result = await anyio.to_thread.run_sync(self.fn) + + if isinstance(result, InputRequiredResult): + # A static resource function can never read the retry's + # input_responses (it takes no Context), so this can only be a + # mistake — reject it instead of JSON-dumping it as content. + raise ValueError( + "static resources cannot return InputRequiredResult; only resource " + "template functions participate in the multi-round-trip flow" + ) + if isinstance(result, Resource): # pragma: no cover + return await result.read() + elif isinstance(result, bytes): + return result + elif isinstance(result, str): + return result + else: + return pydantic_core.to_json(result, fallback=str, indent=2).decode() @classmethod def from_function( @@ -191,12 +177,9 @@ def validate_text_encoding(cls, encoding: str | None) -> str | None: async def read(self) -> str | bytes: """Read the file content.""" - try: - if self.encoding is None: - return await anyio.to_thread.run_sync(self.path.read_bytes) - return await anyio.to_thread.run_sync(partial(self.path.read_text, encoding=self.encoding)) - except Exception as exc: - raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc + if self.encoding is None: + return await anyio.to_thread.run_sync(self.path.read_bytes) + return await anyio.to_thread.run_sync(partial(self.path.read_text, encoding=self.encoding)) class HttpResource(Resource): @@ -236,18 +219,12 @@ def list_files(self) -> list[Path]: # pragma: no cover if not self.path.is_dir(): raise NotADirectoryError(f"Not a directory: {self.path}") - try: - if self.pattern: - return list(self.path.glob(self.pattern)) if not self.recursive else list(self.path.rglob(self.pattern)) - return list(self.path.glob("*")) if not self.recursive else list(self.path.rglob("*")) - except Exception as exc: - raise ValueError(f"Error listing directory {self.path}: {exc}") from exc + if self.pattern: + return list(self.path.glob(self.pattern)) if not self.recursive else list(self.path.rglob(self.pattern)) + return list(self.path.glob("*")) if not self.recursive else list(self.path.rglob("*")) async def read(self) -> str: # Always returns JSON string # pragma: no cover """Read the directory listing.""" - try: - files = await anyio.to_thread.run_sync(self.list_files) - file_list = [str(f.relative_to(self.path)) for f in files if f.is_file()] - return json.dumps({"files": file_list}, indent=2) - except Exception as exc: - raise UnexpectedResourceError(f"Error reading resource {self.uri}") from exc + files = await anyio.to_thread.run_sync(self.list_files) + file_list = [str(f.relative_to(self.path)) for f in files if f.is_file()] + return json.dumps({"files": file_list}, indent=2) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index 168a7df672..07d94f6496 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -427,10 +427,7 @@ async def _handle_call_tool( except MCPError: raise except Exception as exc: - # A ToolError (deliberate, unknown tool, rejected arguments) is an outcome - # the model already reads in full, so it is one INFO record, repr-quoted to - # keep peer-supplied text on one line. Anything else is a crash in the - # tool: log the traceback that the result text doesn't carry. + # %r keeps peer-supplied text (names, pydantic messages) on one line. if isinstance(exc, ToolError) and not isinstance(exc, UnexpectedToolError): logger.info("Tool %r failed: %r", params.name, str(exc)) else: @@ -449,9 +446,6 @@ async def _handle_read_resource( try: results = await self.read_resource(params.uri, context) except ResourceError as err: - # UnexpectedResourceError wraps a crash whose text is withheld from the - # client, so the traceback goes to the log. Any other ResourceError was - # raised on purpose (or is the SDK's "Unknown resource") and is one INFO record. if isinstance(err, UnexpectedResourceError): logger.exception("Resource %r raised an unexpected exception", str(params.uri)) else: @@ -584,20 +578,15 @@ async def read_resource( """ if context is None: context = Context(mcp_server=self, subscriptions=self._subscriptions) - resource = await self._resource_manager.get_resource(uri, context) - if isinstance(resource, InputRequiredResult): - return resource - try: + resource = await self._resource_manager.get_resource(uri, context) + if isinstance(resource, InputRequiredResult): + return resource content = await resource.read() return [ReadResourceContents(content=content, mime_type=resource.mime_type, meta=resource.meta)] except (MCPError, ResourceError): - # Includes the UnexpectedResourceError the built-in resource types raise - # around a crash in the function or the file read. raise except Exception as exc: - # A custom Resource subclass whose read() raised: wrap it the same way, - # naming only the URI so the original text is withheld from the client. raise UnexpectedResourceError(f"Error reading resource {uri}") from exc def add_tool( @@ -1326,10 +1315,7 @@ async def get_prompt( except MCPError: raise except Exception as e: - # Not logged here: this escapes `_handle_get_prompt` as-is, so the - # dispatcher boundary that turns it into the JSON-RPC error logs it once - # with its traceback (or, in-process with `raise_exceptions=True`, - # hands it to the caller instead). + # Not logged here: the dispatcher boundary logs it once. raise ValueError(str(e)) from e diff --git a/src/mcp/server/mcpserver/tools/base.py b/src/mcp/server/mcpserver/tools/base.py index cd556e9726..0768372c39 100644 --- a/src/mcp/server/mcpserver/tools/base.py +++ b/src/mcp/server/mcpserver/tools/base.py @@ -7,7 +7,13 @@ from mcp_types import Icon, InputRequiredResult, ToolAnnotations from pydantic import BaseModel, Field, ValidationError -from mcp.server.mcpserver.exceptions import InvalidSignature, ToolError, UnexpectedToolError +from mcp.server.mcpserver.exceptions import ( + InvalidSignature, + ResourceError, + ToolError, + UnexpectedResourceError, + UnexpectedToolError, +) from mcp.server.mcpserver.resolve import ( build_resolver_plans, find_resolved_parameters, @@ -133,17 +139,21 @@ async def run( Raises: ToolError: If the arguments fail validation against the input schema, or - the tool function (or a resolver) raises `ToolError`. - UnexpectedToolError: If the tool function (or a resolver) raises anything - other than `ToolError` or `MCPError`, or its return value fails output - conversion. + the tool function (or a resolver) raises `ToolError` or `ResourceError`. + UnexpectedToolError: If argument validation, the tool function, or a + resolver raises anything else, or the return value fails output conversion. """ try: validated = self.fn_metadata.validate_arguments(arguments) except ValidationError as exc: - # The caller's arguments don't match the input schema. That is the model's - # mistake to read and correct, so it is reported like a deliberate ToolError. + # The caller's arguments don't match the input schema: the model's mistake + # to read and correct, so it is reported like a deliberate ToolError. raise ToolError(f"Error executing tool {self.name}: {exc}") from exc + except MCPError: + raise + except Exception as exc: + # A custom validator or default_factory that raises is a crash. + raise UnexpectedToolError(f"Error executing tool {self.name}: {exc}") from exc try: pass_directly: dict[str, Any] = {} @@ -191,12 +201,12 @@ async def run( # `CallToolResult(isError=True)` execution failure. raise # Everything else reaches the model as an is_error result under this tool's - # name. The wrapper's type is what tells the server whether to log a crash. - except UnexpectedToolError as exc: - # A nested tool call crashed: still a crash under this tool's name. + # name, and the wrapper's type tells the server whether to log a crash. + except (UnexpectedToolError, UnexpectedResourceError) as exc: + # A nested tool call or resource read crashed: still a crash here. raise UnexpectedToolError(f"Error executing tool {self.name}: {exc}") from exc - except ToolError as exc: - # Raised deliberately by the tool or a resolver: anticipated. + except (ToolError, ResourceError) as exc: + # Raised deliberately by the tool, a resolver, or a resource it read. raise ToolError(f"Error executing tool {self.name}: {exc}") from exc except Exception as exc: raise UnexpectedToolError(f"Error executing tool {self.name}: {exc}") from exc diff --git a/tests/docs_src/test_uri_templates.py b/tests/docs_src/test_uri_templates.py index 4b2b6edaf1..17cca68cc1 100644 --- a/tests/docs_src/test_uri_templates.py +++ b/tests/docs_src/test_uri_templates.py @@ -139,6 +139,19 @@ async def test_safe_join_serves_a_file_inside_the_base_directory( assert content.text == "# Printer setup" +async def test_a_missing_manual_is_resource_not_found(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """tutorial002 and the closing tip: a path with no file behind it is `-32602` with the handler's message.""" + monkeypatch.setattr(tutorial002, "DOCS_ROOT", tmp_path) + async with Client(tutorial002.mcp) as client: + with pytest.raises(MCPError) as exc: + await client.read_resource("manuals://printing/missing.md") + assert exc.value.error == ErrorData( + code=INVALID_PARAMS, + message="No manual at 'printing/missing.md'.", + data={"uri": "manuals://printing/missing.md"}, + ) + + def test_safe_join_raises_when_the_resolved_path_escapes_the_base(tmp_path: Path) -> None: """tutorial002: a path that climbs out of `DOCS_ROOT` raises `PathEscapeError`.""" with pytest.raises(PathEscapeError): diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 964a1829d2..890ac1822b 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -1307,8 +1307,15 @@ def __post_init__(self) -> None: "mcpserver:resource:read-throws-surfaced": Requirement( source="sdk", behavior=( - "A resource function that raises is surfaced to the caller as a JSON-RPC error response " - "(-32603 Internal error), with the original exception text withheld." + "A resource function that raises an unexpected exception is surfaced to the caller as a JSON-RPC " + "error response (-32603 Internal error), with the original exception text withheld." + ), + ), + "mcpserver:resource:static-not-found": Requirement( + source="sdk", + behavior=( + "A static (fixed-URI) resource function that raises ResourceNotFoundError is surfaced as -32602 " + "with the handler's message and the URI in data, the same as from a template function." ), ), "mcpserver:resource:static": Requirement( diff --git a/tests/interaction/mcpserver/test_resources.py b/tests/interaction/mcpserver/test_resources.py index eadf4794e6..914d3cbbeb 100644 --- a/tests/interaction/mcpserver/test_resources.py +++ b/tests/interaction/mcpserver/test_resources.py @@ -14,6 +14,7 @@ from mcp import MCPError from mcp.server.mcpserver import MCPServer +from mcp.server.mcpserver.exceptions import ResourceNotFoundError from tests._stamp import Unstamp from tests.interaction._connect import Connect from tests.interaction._requirements import requirement @@ -152,6 +153,28 @@ def boom() -> str: ) +@requirement("mcpserver:resource:static-not-found") +async def test_static_resource_function_raising_not_found_is_invalid_params(connect: Connect) -> None: + """ResourceNotFoundError from a fixed-URI resource function reaches the caller as -32602 with its message. + + A static resource can still be absent (a report not generated yet, a file that comes and goes), + and the handler's message passes through exactly as it does from a template function. + """ + mcp = MCPServer("library") + + @mcp.resource("reports://latest") + def latest() -> str: + raise ResourceNotFoundError("no report has been generated yet") + + async with connect(mcp) as client: + with pytest.raises(MCPError) as exc_info: + await client.read_resource("reports://latest") + + assert exc_info.value.error == snapshot( + ErrorData(code=-32602, message="no report has been generated yet", data={"uri": "reports://latest"}) + ) + + @requirement("mcpserver:resource:duplicate-name") async def test_registering_a_duplicate_resource_uri_warns_and_keeps_the_first( connect: Connect, unstamped: Unstamp diff --git a/tests/server/mcpserver/resources/test_file_resources.py b/tests/server/mcpserver/resources/test_file_resources.py index 3604bf1b32..56203ad2e9 100644 --- a/tests/server/mcpserver/resources/test_file_resources.py +++ b/tests/server/mcpserver/resources/test_file_resources.py @@ -6,7 +6,6 @@ import pytest from pydantic import ValidationError -from mcp.server.mcpserver.exceptions import UnexpectedResourceError from mcp.server.mcpserver.resources import FileResource @@ -179,10 +178,8 @@ async def test_missing_file_error(temp_file: Path): name="test", path=missing, ) - with pytest.raises(UnexpectedResourceError) as exc: + with pytest.raises(FileNotFoundError): await resource.read() - assert str(exc.value) == "Error reading resource file:///missing.txt" - assert isinstance(exc.value.__cause__, FileNotFoundError) @pytest.mark.skipif(os.name == "nt", reason="File permissions behave differently on Windows") @@ -195,8 +192,7 @@ async def test_permission_error(temp_file: Path): # pragma: lax no cover name="test", path=temp_file, ) - with pytest.raises(UnexpectedResourceError) as exc: + with pytest.raises(PermissionError): await resource.read() - assert isinstance(exc.value.__cause__, PermissionError) finally: temp_file.chmod(0o644) # Restore permissions diff --git a/tests/server/mcpserver/resources/test_function_resources.py b/tests/server/mcpserver/resources/test_function_resources.py index dc57dbc31c..d38ddd840c 100644 --- a/tests/server/mcpserver/resources/test_function_resources.py +++ b/tests/server/mcpserver/resources/test_function_resources.py @@ -7,7 +7,6 @@ from mcp_types import InputRequiredResult from pydantic import BaseModel -from mcp.server.mcpserver.exceptions import UnexpectedResourceError from mcp.server.mcpserver.resources import FunctionResource @@ -81,22 +80,19 @@ def get_data() -> dict[str, str]: @pytest.mark.anyio async def test_error_handling(self): - """A crash in the function is wrapped as UnexpectedResourceError naming only the URI, - with the function's own exception as `__cause__`.""" - raised = ValueError("Test error") + """read() lets the function's own exception propagate; MCPServer.read_resource does the wrapping.""" def failing_func() -> str: - raise raised + raise ValueError("Test error") resource = FunctionResource( uri="function://test", name="test", fn=failing_func, ) - with pytest.raises(UnexpectedResourceError) as exc: + with pytest.raises(ValueError) as exc: await resource.read() - assert str(exc.value) == snapshot("Error reading resource function://test") - assert exc.value.__cause__ is raised + assert str(exc.value) == "Test error" @pytest.mark.anyio async def test_basemodel_conversion(self): @@ -260,10 +256,9 @@ def ask() -> InputRequiredResult: return InputRequiredResult(request_state="round-1") resource = FunctionResource(uri="resource://ask", name="ask", fn=ask) - with pytest.raises(UnexpectedResourceError) as exc: + with pytest.raises(ValueError) as exc: await resource.read() - assert str(exc.value) == snapshot("Error reading resource resource://ask") - assert str(exc.value.__cause__) == snapshot( + assert str(exc.value) == snapshot( "static resources cannot return InputRequiredResult; " "only resource template functions participate in the multi-round-trip flow" ) diff --git a/tests/server/mcpserver/test_resolve.py b/tests/server/mcpserver/test_resolve.py index aa5ced266a..966bb94ed1 100644 --- a/tests/server/mcpserver/test_resolve.py +++ b/tests/server/mcpserver/test_resolve.py @@ -1,6 +1,7 @@ """Tests for resolver dependency injection (MRTR) on MCPServer tools.""" import json +import logging from collections.abc import Callable from datetime import datetime from typing import Annotated, Any, Literal, TypeVar, cast @@ -1761,10 +1762,13 @@ async def listy(login: Annotated[Login, Resolve(lookup)]) -> list[str]: @pytest.mark.anyio -async def test_tool_returning_input_required_dynamically_with_resolvers_is_an_error(): +async def test_tool_returning_input_required_dynamically_with_resolvers_is_an_error( + caplog: pytest.LogCaptureFixture, +): # The annotated form of this combination is rejected at registration; a body # that returns an InputRequiredResult without declaring it fails loudly at the # same boundary instead of silently fighting the resolvers for the channel. + # It is an authoring bug, so it is logged as a crash rather than at INFO. mcp = MCPServer(name="DynamicChannelClash", request_state_security=RequestStateSecurity.ephemeral()) async def lookup(ctx: Context) -> Login: @@ -1774,11 +1778,14 @@ async def lookup(ctx: Context) -> Login: async def sneaky(login: Annotated[Login, Resolve(lookup)]): return InputRequiredResult(input_requests={}, request_state="opaque") + caplog.set_level(logging.INFO) async with Client(mcp) as client: result = await client.call_tool("sneaky", {}) assert result.is_error assert isinstance(result.content[0], TextContent) assert "the multi-round flow is driven either by resolvers or by the tool body" in result.content[0].text + records = [(r.levelname, r.getMessage()) for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert records == [("ERROR", "Tool 'sneaky' raised an unexpected exception")] def test_question_digest_pins_the_rendered_question(): diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index cd56990f9d..d37e17b48b 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -43,7 +43,7 @@ TextContent, TextResourceContents, ) -from pydantic import BaseModel, ValidationError +from pydantic import AfterValidator, BaseModel, ValidationError from starlette.applications import Starlette from starlette.routing import Mount, Route @@ -2243,11 +2243,11 @@ def thing() -> str: def _cause_chain(exc: BaseException | None) -> list[BaseException]: - """`exc` and everything it chains back to, explicitly (`__cause__`) or implicitly (`__context__`).""" + """`exc` and everything it explicitly chains back to via `__cause__` (`raise ... from ...`).""" chain: list[BaseException] = [] while exc is not None: chain.append(exc) - exc = exc.__cause__ or exc.__context__ + exc = exc.__cause__ return chain @@ -2495,6 +2495,137 @@ async def whoami(user: Annotated[str, Resolve(current_user)]) -> str: assert raised in _cause_chain(_logged_exception(caplog)) +async def test_argument_validator_that_crashes_is_the_tools_crash(caplog: pytest.LogCaptureFixture): + """SDK-defined: pydantic only turns ValueError/AssertionError into ValidationError, so a validator + raising anything else is a bug in the tool's schema and is wrapped and logged as a crash.""" + mcp = MCPServer() + raised = TypeError("codes are compared as integers") + + def check(code: str) -> str: + raise raised + + @mcp.tool() + def redeem(code: Annotated[str, AfterValidator(check)]) -> str: + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("redeem", {"code": "SAVE10"}) + with pytest.raises(UnexpectedToolError) as exc: + await mcp.call_tool("redeem", {"code": "SAVE10"}) + + assert result.content == [ + TextContent(type="text", text="Error executing tool redeem: codes are compared as integers") + ] + assert exc.value.__cause__ is raised + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'redeem' raised an unexpected exception", True)]) + + +async def test_argument_validator_raising_mcp_error_is_a_protocol_error(caplog: pytest.LogCaptureFixture): + """SDK-defined: MCPError keeps its meaning wherever it is raised, including inside an argument + validator: the request fails with that code and MCPServer logs nothing.""" + mcp = MCPServer() + + def check(code: str) -> str: + raise MCPError(code=INVALID_PARAMS, message="codes are issued per session") + + @mcp.tool() + def redeem(code: Annotated[str, AfterValidator(check)]) -> str: + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.call_tool("redeem", {"code": "SAVE10"}) + + assert exc.value.error == snapshot(ErrorData(code=INVALID_PARAMS, message="codes are issued per session")) + assert _server_records(caplog) == [] + + +async def test_resource_error_escaping_a_tool_is_anticipated(caplog: pytest.LogCaptureFixture): + """SDK-defined: a tool that lets ResourceNotFoundError from ctx.read_resource() propagate has + reported an anticipated failure, so it is INFO here just as it is for resources/read.""" + mcp = MCPServer() + + @mcp.resource("books://{title}") + def book(title: str) -> str: + raise ResourceNotFoundError(f"No book titled {title!r}.") + + @mcp.tool() + async def summarise(title: str, ctx: Context) -> str: + await ctx.read_resource(f"books://{title}") + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("summarise", {"title": "Nothing"}) + with pytest.raises(ToolError) as exc: + await mcp.call_tool("summarise", {"title": "Nothing"}) + + assert result.content == [ + TextContent(type="text", text="Error executing tool summarise: No book titled 'Nothing'.") + ] + assert type(exc.value) is ToolError + assert _server_records(caplog) == snapshot( + [("INFO", "Tool 'summarise' failed: \"Error executing tool summarise: No book titled 'Nothing'.\"", False)] + ) + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + +async def test_resource_crash_escaping_a_tool_is_the_tools_crash(caplog: pytest.LogCaptureFixture): + """SDK-defined: a crashing resource read inside a tool stays a crash under the tool's name, logged + once, with the traceback reaching the resource function's own exception.""" + mcp = MCPServer() + raised = ConnectionError("catalog database unreachable") + + @mcp.resource("books://{title}") + def book(title: str) -> str: + raise raised + + @mcp.tool() + async def summarise(title: str, ctx: Context) -> str: + await ctx.read_resource(f"books://{title}") + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("summarise", {"title": "Dune"}) + + assert result.content == [ + TextContent( + type="text", + text="Error executing tool summarise: Error creating resource from template books://Dune", + ) + ] + assert _server_records(caplog) == snapshot([("ERROR", "Tool 'summarise' raised an unexpected exception", True)]) + assert raised in _cause_chain(_logged_exception(caplog)) + + +async def test_tool_that_recovers_from_a_missing_resource_logs_nothing(caplog: pytest.LogCaptureFixture): + """SDK-defined: MCPServer.read_resource() itself writes no record, so a tool that catches + ResourceNotFoundError and carries on leaves the log clean.""" + mcp = MCPServer() + + @mcp.resource("books://{title}") + def book(title: str) -> str: + raise ResourceNotFoundError(f"No book titled {title!r}.") + + @mcp.tool() + async def summarise(title: str, ctx: Context) -> str: + try: + await ctx.read_resource(f"books://{title}") + except ResourceNotFoundError: + return "not in the catalog" + raise NotImplementedError + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + result = await client.call_tool("summarise", {"title": "Nothing"}) + + assert result.content == [TextContent(type="text", text="not in the catalog")] + assert _server_records(caplog) == [] + + async def test_static_resource_raising_unexpected_exception_is_logged_once_at_error_with_its_traceback( caplog: pytest.LogCaptureFixture, ): From d0c72f2d44c4b1d8dfa826108b6bf47aa730cf2e Mon Sep 17 00:00:00 2001 From: Max <224885523+maxisbey@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:43:45 +0100 Subject: [PATCH 4/6] remove previous exception interpolation from raised exception Co-authored-by: Marcelo Trylesinski --- src/mcp/server/mcpserver/prompts/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/server/mcpserver/prompts/base.py b/src/mcp/server/mcpserver/prompts/base.py index 7028dd8f0b..a43f452df3 100644 --- a/src/mcp/server/mcpserver/prompts/base.py +++ b/src/mcp/server/mcpserver/prompts/base.py @@ -210,4 +210,4 @@ async def render( except MCPError: raise except Exception as exc: - raise ValueError(f"Error rendering prompt {self.name}: {exc}") from exc + raise ValueError(f"Error rendering prompt {self.name}) from exc From f6f7627b24daaf93782c4a4fc844935632fb11bd Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:51:15 +0000 Subject: [PATCH 5/6] Close the f-string in Prompt.render and pin the shorter message 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 ", the legacy-path interaction test snapshots that instead of matching the pydantic prefix. --- src/mcp/server/mcpserver/prompts/base.py | 2 +- tests/interaction/mcpserver/test_prompts.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/mcp/server/mcpserver/prompts/base.py b/src/mcp/server/mcpserver/prompts/base.py index a43f452df3..a13b72f1aa 100644 --- a/src/mcp/server/mcpserver/prompts/base.py +++ b/src/mcp/server/mcpserver/prompts/base.py @@ -210,4 +210,4 @@ async def render( except MCPError: raise except Exception as exc: - raise ValueError(f"Error rendering prompt {self.name}) from exc + raise ValueError(f"Error rendering prompt {self.name}") from exc diff --git a/tests/interaction/mcpserver/test_prompts.py b/tests/interaction/mcpserver/test_prompts.py index 8409e50207..3872858b4b 100644 --- a/tests/interaction/mcpserver/test_prompts.py +++ b/tests/interaction/mcpserver/test_prompts.py @@ -123,8 +123,8 @@ async def test_get_prompt_with_a_wrong_type_argument_is_rejected_before_the_func The decorated function is wrapped in pydantic's validate_call, so a value that cannot be coerced to the parameter's annotation fails before the body executes. The function body - raises NotImplementedError to prove it never ran. The error is wrapped in the SDK's stable - rendering-error prefix; the body of the message is raw pydantic output and is not asserted. + raises NotImplementedError to prove it never ran. The client sees only the SDK's + rendering-error message naming the prompt, with the pydantic detail withheld. """ mcp = MCPServer("prompter") @@ -137,8 +137,7 @@ def repeat(phrase: str, count: int) -> str: with pytest.raises(MCPError) as exc_info: await client.get_prompt("repeat", {"phrase": "hi", "count": "many"}) - assert exc_info.value.error.code == 0 - assert exc_info.value.error.message.startswith("Error rendering prompt repeat: 1 validation error") + assert exc_info.value.error == snapshot(ErrorData(code=0, message="Error rendering prompt repeat")) @requirement("mcpserver:prompt:optional-args") From fb7e31649a36039ae90783f0e2a37ce5cb20167d Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Wed, 19 Aug 2026 14:02:19 +0000 Subject: [PATCH 6/6] Drop the migration.md addition; the guide is closed to new entries 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 --- docs/migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migration.md b/docs/migration.md index ed1b2a58b5..e59b4a6ac6 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1018,7 +1018,7 @@ except MCPError as e: Reading a missing resource now returns JSON-RPC error code `-32602` (invalid params) with the requested URI in `error.data` (`{"uri": ...}`), per [SEP-2164](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2164). Previously the server returned code `0` with no `data`. Clients can now reliably distinguish not-found from other errors; a resource handler (static or template) that raises `ResourceNotFoundError` (from `mcp.server.mcpserver.exceptions`) produces this same response. -The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). Likewise, `FunctionResource.read()` and `FileResource.read()` no longer wrap failures in `ValueError`: called directly they raise whatever the function or file read raised, and through `MCPServer.read_resource()` that arrives as `UnexpectedResourceError` (a `ResourceError`) with the original as `__cause__`. +The underlying lookups now raise typed exceptions instead of `ValueError`. `ResourceManager.get_resource()` raises `ResourceNotFoundError` when no resource or template matches the URI, and `ResourceTemplate.create_resource()` raises `ResourceError` when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`; `ResourceNotFoundError` subclasses `ResourceError`). ### `Resource` classes reject unknown keyword arguments