diff --git a/docs/client/index.md b/docs/client/index.md index b1a1dbc234..767f9eb06a 100644 --- a/docs/client/index.md +++ b/docs/client/index.md @@ -81,7 +81,7 @@ That schema is everything a UI needs to render an argument form, and everything `call_tool(name, arguments)` runs the tool and gives you back a `CallToolResult`. -```python title="client.py" hl_lines="26-33" +```python title="client.py" hl_lines="27-34" --8<-- "docs_src/client/tutorial003.py" ``` @@ -113,7 +113,7 @@ A tool that raises does **not** raise in your client. It comes back as an ordina !!! check Ask `lookup_book` for `"Solaris"` (a title that isn't in the catalog) and the function raises - `ValueError`. The call still returns normally: + `ToolError`. The call still returns normally: ```python result.is_error # True @@ -121,9 +121,10 @@ A tool that raises does **not** raise in your client. It comes back as an ordina result.structured_content # None ``` - The exception's message landed in `content`, where the **model** can read it and try again. That - is deliberate: a tool error is part of the conversation, not a crash. Always look at `is_error` - before you trust `structured_content`. + The `ToolError`'s message landed in `content`, where the **model** can read it and try again. That + is deliberate: a tool error is part of the conversation, not a crash. (Had the tool crashed with + some other exception, `content` would say only `Error executing tool lookup_book`.) Always look at + `is_error` before you trust `structured_content`. !!! warning `is_error=True` covers more than your own `raise`. Ask for a tool the server doesn't even have diff --git a/docs/deprecated.md b/docs/deprecated.md index 71844aa5e2..05e37903fe 100644 --- a/docs/deprecated.md +++ b/docs/deprecated.md @@ -119,10 +119,11 @@ That is the whole API. There is no per-method switch, and you don't want one: th Run the filter the other way and you get a free regression test. Add `"error::mcp.MCPDeprecationWarning"` to the `filterwarnings` setting in your pytest configuration and the deprecated call **raises** instead of warning. A tool named - `old_log` that still calls `ctx.info()` stops passing and starts reporting: + `old_log` that still calls `ctx.info()` stops passing: the call comes back `is_error=True` with + `Error executing tool old_log`, and the captured server log names the culprit: ```text - Error executing tool old_log: The logging capability is deprecated as of 2026-07-28 (SEP-2577). + mcp.shared.exceptions.MCPDeprecationWarning: The logging capability is deprecated as of 2026-07-28 (SEP-2577). ``` One line of pytest configuration, and a deprecated call can never sneak back into your diff --git a/docs/handlers/elicitation.md b/docs/handlers/elicitation.md index c9a0a4fabc..478e4824fa 100644 --- a/docs/handlers/elicitation.md +++ b/docs/handlers/elicitation.md @@ -84,7 +84,8 @@ That schema is the form. `Field(description=...)` is the label; a default pre-fi !!! warning An elicitation schema is not as expressive as a tool's input schema. Flat, primitive fields only: `str`, `int`, `float`, `bool`, or a `Literal` of strings (it becomes an `enum`). - Put a model inside the model and `ctx.elicit` raises before anything is sent to the client: + Put a model inside the model and `ctx.elicit` raises before anything is sent to the client. + The tool call fails with `Error executing tool `, and your server log has the reason: ```text TypeError: Elicitation schema field 'address' rendered as {'$ref': '#/$defs/Address'}, which is not a valid PrimitiveSchemaDefinition @@ -107,8 +108,8 @@ A refusal is not an error. The tool decides what declining means (here, no booki !!! tip The answer is validated against your model before your code sees it. A client that sends - `"maybe"` for a `bool` doesn't corrupt your booking: the call fails with a - schema-mismatch error, your `if` never runs. + `"maybe"` for a `bool` doesn't corrupt your booking: `ctx.elicit` raises `ValueError`, the call + fails, and your `if` never runs. ## Send the user to a URL diff --git a/docs/handlers/logging.md b/docs/handlers/logging.md index 6f6c839314..dd5540a71b 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#any-other-exception)** explains what gets logged and at which level. + ## Try it Run the server with the MCP Inspector: diff --git a/docs/migration.md b/docs/migration.md index b094d79f84..f231710ad7 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`). @@ -2737,7 +2737,7 @@ One behavioral caveat when moving progress-reporting handlers onto `Client(serve Every deprecation below is a runtime warning as well as a type-checker one: deprecated methods and helpers emit `mcp.MCPDeprecationWarning` on each call, and the deprecated `Server(...)` constructor parameters (`on_set_logging_level`, `on_roots_list_changed`, `on_progress`) emit it at construction time. The category subclasses `UserWarning`, not `DeprecationWarning`, so it is visible by default; [Deprecated features](deprecated.md) has the full list and each replacement. -Under pytest's `filterwarnings = ["error"]`, that warning becomes an exception at the first deprecated call. Inside an `@mcp.tool()` handler the exception is caught like any other and returned as `CallToolResult(is_error=True)` (`Error executing tool ...: The logging capability is deprecated as of 2026-07-28 (SEP-2577).`), which reads as a failing tool rather than a warning. Keep the warnings visible but non-fatal with: +Under pytest's `filterwarnings = ["error"]`, that warning becomes an exception at the first deprecated call. Inside an `@mcp.tool()` handler the exception is caught like any other and returned as `CallToolResult(is_error=True)` (`Error executing tool ...`, with the `MCPDeprecationWarning` traceback in the server log), which reads as a failing tool rather than a warning. Keep the warnings visible but non-fatal with: ```toml [tool.pytest.ini_options] diff --git a/docs/servers/handling-errors.md b/docs/servers/handling-errors.md index 4262f586a7..e1f6fffba6 100644 --- a/docs/servers/handling-errors.md +++ b/docs/servers/handling-errors.md @@ -1,8 +1,8 @@ # Handling errors -A tool can fail in two ways, and the SDK treats them very differently. +A tool can fail in three ways, and the SDK treats each differently. -Raise an ordinary exception and the **model** sees it. Raise `MCPError` and the **protocol** sees it. +Raise `ToolError` and the **model** sees your message. Raise `MCPError` and the **protocol** sees it. Raise anything else and it is a crash: the model learns only that the call failed, and your log gets the traceback. This page is about choosing. @@ -10,11 +10,11 @@ This page is about choosing. Take a tool that looks something up, and let the lookup miss: -```python title="server.py" hl_lines="11-12" +```python title="server.py" hl_lines="2 12-13" --8<-- "docs_src/handling_errors/tutorial001.py" ``` -There is nothing MCP about those two lines. `get_author` raises a plain `ValueError`, the way any Python function would. +`ToolError`, from `mcp.server.mcpserver.exceptions`, is how a tool tells the model that something went wrong. Call it with a title that isn't in the catalog and look at the result: @@ -25,13 +25,15 @@ result.structured_content # None ``` * The request **succeeded**. There is a result; nothing was raised at the caller. -* `is_error` is `True`, and your exception's message (prefixed with the tool name) is in `content`, exactly where the model reads. +* `is_error` is `True`, and your message (prefixed with the tool name) is in `content`, exactly where the model reads. * `structured_content` is `None`. A failed call has no return value to structure. -This is a **tool error**, and it is the default for *any* exception your tool raises. It is also almost always what you want. +This is a **tool error**, and it is almost always what you want. The model is the one calling your tool. It picked the arguments. So a tool error is a turn in the conversation: the model reads *"No book titled 'Nothing' in the catalog."*, realises it guessed the title wrong, and calls again with a better one. You wrote one `raise` and got a self-correcting agent. +On the server, a `ToolError` is one `INFO` line in the log, with no traceback. You saw it coming, so there is nothing to investigate. + !!! tip Never `return` an error message from a tool. A returned string has `is_error=False`, so to the model (and to every client UI) it looks like the tool worked and that string was the answer. @@ -39,7 +41,7 @@ The model is the one calling your tool. It picked the arguments. So a tool error ## An error the model cannot fix -Now swap `ValueError` for `MCPError`. +Now swap `ToolError` for `MCPError`. ```python title="server.py" hl_lines="1 3 14" --8<-- "docs_src/handling_errors/tutorial002.py" @@ -72,10 +74,10 @@ Now swap `ValueError` for `MCPError`. The two paths answer two different questions. -* **Raise any exception** for a failure of *execution*: the thing your tool tried to do didn't work. The model chose the call, so the model should see the consequence and get a chance to recover. A misspelled title, an upstream API that timed out, a row that doesn't exist: all tool errors. +* **Raise `ToolError`** for a failure of *execution*: the thing your tool tried to do didn't work. The model chose the call, so the model should see the consequence and get a chance to recover. A misspelled title, an upstream API that timed out, a row that doesn't exist: all tool errors. * **Raise `MCPError`** when the *request itself* should be rejected: the client is missing a capability your tool depends on, the server isn't in a state to serve anyone, the caller skipped a required step. No retry from the model fixes any of those, so there is nothing to gain from handing it the message. -One question decides it: **could a smarter model have avoided this?** Yes -> ordinary exception. No -> `MCPError`. +One question decides it: **could a smarter model have avoided this?** Yes -> `ToolError`. No -> `MCPError`. By that test, the second version of `get_author` made the wrong choice: a better title fixes it, so the model deserved to see the message. It's there to show you the mechanism, not to recommend it. @@ -84,6 +86,25 @@ By that test, the second version of `get_author` made the wrong choice: a better `data` payload. Whatever you put in them is what the client receives: the SDK forwards a raised `MCPError` verbatim instead of sanitising it. +## Any other exception + +Now take the check out and let the dictionary lookup fail on its own: + +```python title="server.py" hl_lines="11" +--8<-- "docs_src/handling_errors/tutorial004.py" +``` + +`CATALOG[title]` raises `KeyError`. You didn't plan for it, so the SDK treats it as a crash: + +```python +result.is_error # True +result.content # [TextContent(text="Error executing tool get_author")] +``` + +The call still returns `is_error=True`, so the model knows it failed and can move on. What it doesn't get is the exception's text: a `KeyError` from your code, or a stack of SQL from a driver three libraries down, may describe your server's internals, so it never leaves the server. + +You get it instead. The server logs the crash at `ERROR` with the full traceback, as `Tool 'get_author' raised an unexpected exception`. A production log at `WARNING` therefore stays quiet through every `ToolError` and speaks up the moment something is actually broken. + ## A resource that doesn't exist Resources draw the same line, and ship one named exception for the common case. @@ -104,7 +125,7 @@ When it can't, raise `ResourceNotFoundError`. The SDK turns it into the protocol } ``` -Notice there is no `is_error=True` half-result here. A resource read either returns contents or fails: resources have only the protocol path. Templates and everything else about resources live in **[Resources](resources.md)**. +Notice there is no `is_error=True` half-result here. A resource read either returns contents or fails: resources have only the protocol path. `ResourceError` is the same thing for a failure that isn't "not found" (`-32603`, your message), and both are one `INFO` line in your log. Any other exception bar `MCPError` is a crash: the client gets `-32603` naming only the URI, and the traceback goes to your log at `ERROR`. Templates and everything else about resources live in **[Resources](resources.md)**. ## Errors you never raise @@ -115,19 +136,21 @@ 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 a **client** sees on this page, the in-memory `Client` you'll write tests with + sees too. 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. If you need the traceback of a crash, it is in + the server's log, and pytest's `caplog` captures it. **[Testing](../get-started/testing.md)** covers the pattern. ## Recap -* Raise **any exception** in a tool -> the call returns `is_error=True` with your message in `content`. The model reads it and can retry. This is the default. +* Raise **`ToolError`** in a tool -> the call returns `is_error=True` with your message in `content`. The model reads it and can retry. * Raise **`MCPError`** -> the call itself fails with a JSON-RPC error. The model sees nothing; the host deals with it. `code`, `message`, and `data` survive intact. -* The deciding question: *could a smarter model have avoided this?* Yes -> exception. No -> `MCPError`. +* The deciding question: *could a smarter model have avoided this?* Yes -> `ToolError`. No -> `MCPError`. +* Any **other exception** is a crash -> `is_error=True` with only `Error executing tool ` for the model, and an `ERROR` record with the traceback for you. * `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`. +* Imports: `from mcp import MCPError`, `from mcp.server.mcpserver.exceptions import ToolError, ResourceError, ResourceNotFoundError`, and the error-code constants 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/structured-output.md b/docs/servers/structured-output.md index 3964897cd1..792bcb0c6c 100644 --- a/docs/servers/structured-output.md +++ b/docs/servers/structured-output.md @@ -182,18 +182,19 @@ You don't notice while you build the value by hand: Pydantic already made sure y The annotation promises `WeatherData`. The upstream response stopped sending `humidity`. !!! check - Call `get_weather` and it does not quietly hand the client a half-empty object. The call fails, - and the first lines of the error name the field: + Call `get_weather` and it does not quietly hand the client a half-empty object. The call fails: + the client gets `is_error=True` with `Error executing tool get_weather`, so the model knows the + call failed instead of confidently reading weather that isn't there. The field name is for you, + in the server log at `ERROR`: ```text - Error executing tool get_weather: 1 validation error for WeatherData + Tool 'get_weather' raised an unexpected exception + ... + pydantic_core._pydantic_core.ValidationError: 1 validation error for WeatherData humidity Field required [type=missing, input_value={'temperature': 16.2, 'conditions': 'Overcast'}, input_type=dict] ``` - That text comes back as the tool result with `is_error=True`, so the model knows the call failed - instead of confidently reading weather that isn't there. - Returning a plain `dict` from a `-> WeatherData` tool is fine, by the way. That's exactly what `json.loads` produced. Validation is on the value, not on the Python type. ## Opting out diff --git a/docs/servers/uri-templates.md b/docs/servers/uri-templates.md index 406a8fda6a..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" ``` @@ -199,10 +199,10 @@ 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` 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 75a6652ecc..1e452be3ff 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -76,11 +76,11 @@ async def main() -> None: `__aexit__` is the disconnection, which is why there is no `client.close()` to forget. **[Testing](get-started/testing.md)** is built on exactly this pattern. -## `Error executing tool : ` and `Unknown tool: ` +## `Error executing tool : `, `Error executing tool `, and `Unknown tool: ` You are reading a **result**, not an exception. `call_tool` did not raise, and it never will for a failing tool. -Call `forecast` for a city the server doesn't know, and the exception it raises comes back with the request marked as *succeeded*: +Call `forecast` for a city the server doesn't know, and the `ToolError` it raises comes back with the request marked as *succeeded*: ```python result.is_error # True @@ -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. +The bare form, `Error executing tool ` with no message, means the tool **crashed**: something other than `ToolError` was raised while running it (or its return value failed the output schema), and that exception's text is kept off the wire. The traceback is in the **server's log** at `ERROR`, as `Tool '' 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. @@ -404,7 +406,7 @@ mcp = MCPServer("Weather", request_state_security=RequestStateSecurity(keys=[key ## Recap * `ExceptionGroup: unhandled errors in a TaskGroup` is never the error. Read the **last line**; catching `MCPError` *inside* the `async with Client(...)` block skips the wrapping entirely. -* `call_tool` does not raise for a failing tool. `Error executing tool ...` and `Unknown tool: ...` are results: check `result.is_error`. +* `call_tool` does not raise for a failing tool. `Error executing tool ...` and `Unknown tool: ...` are results: check `result.is_error`. No message after the tool name means it crashed, and the traceback is in the server log. * `Client must be used within an async context manager` -> use `async with`. `Use @tool() instead of @tool` -> add the parentheses. * `Tool already exists:` in the server log is the only sign that two same-named tools collapsed into one. * One 421, three spellings: `Server returned an error response` (the python `Client`), `421 Misdirected Request` / `Invalid Host header` (everything else), `Invalid Host header: ` (the server log). Fix: `transport_security=TransportSecuritySettings(allowed_hosts=[...])`. diff --git a/docs/whats-new.md b/docs/whats-new.md index 0a4ed4c35f..068efb26ce 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -129,7 +129,7 @@ On those types, every Python attribute is now snake_case: `result.is_error`, `to The renames announce themselves. These do not: * **Sync functions run on a worker thread.** A `def` tool (or resource, prompt, or resolver) no longer blocks the event loop; the trade is that its body no longer runs *on* the event-loop thread, which matters to thread-affine code. `async def` handlers are untouched. **[Migration Guide](migration.md#sync-handler-functions-now-run-on-a-worker-thread)**. -* **`MCPError` (v1's `McpError`) raised inside a tool is a protocol error now.** The model never sees it. Every other exception still becomes an `is_error=True` result the model can read and react to. **[Handling errors](servers/handling-errors.md)** is the split. +* **`MCPError` (v1's `McpError`) raised inside a tool is a protocol error now.** The model never sees it. Every other exception still becomes an `is_error=True` result, but only a `ToolError`'s message reaches the model: any other exception now reads `Error executing tool `, with the traceback in your server log. **[Handling errors](servers/handling-errors.md)** is the split. * **Results are validated before they leave.** A hand-built `Tool` whose `input_schema` is `{}` now fails `tools/list` (the spec requires `"type": "object"`). Servers built on `@mcp.tool()` never see this; the SDK writes their schemas. * **Your client validates what it receives.** `list_tools()` and `call_tool()` check the server's answer against the negotiated protocol version, so a not-quite-valid server that v1's lenient parse tolerated now raises `pydantic.ValidationError`. If you connect to servers you do not control, expect to be the one who finds them; the **[Migration Guide](migration.md#client-validates-inbound-traffic-against-the-protocol-schema)** has the details. * **URI templates are real RFC 6570 now.** `{+path}`, `{?query}` and friends work, matching is exact instead of regex-loose, and path traversal in extracted values is rejected by default. Stricter templates fail at decoration time, not on the first request. **[URI templates](servers/uri-templates.md)**. diff --git a/docs_src/client/tutorial003.py b/docs_src/client/tutorial003.py index bf74c46748..0831f5f752 100644 --- a/docs_src/client/tutorial003.py +++ b/docs_src/client/tutorial003.py @@ -2,6 +2,7 @@ from mcp import Client from mcp.server import MCPServer +from mcp.server.mcpserver.exceptions import ToolError from mcp.types import TextContent mcp = MCPServer("Bookshop") @@ -17,7 +18,7 @@ class Book(BaseModel): def lookup_book(title: str) -> Book: """Look up a book by its exact title.""" if title != "Dune": - raise ValueError(f"No book titled {title!r} in the catalog.") + raise ToolError(f"No book titled {title!r} in the catalog.") return Book(title="Dune", author="Frank Herbert", year=1965) diff --git a/docs_src/handling_errors/tutorial001.py b/docs_src/handling_errors/tutorial001.py index 003ea94669..9676a10075 100644 --- a/docs_src/handling_errors/tutorial001.py +++ b/docs_src/handling_errors/tutorial001.py @@ -1,4 +1,5 @@ from mcp.server import MCPServer +from mcp.server.mcpserver.exceptions import ToolError mcp = MCPServer("Bookshop") @@ -9,5 +10,5 @@ def get_author(title: str) -> str: """Look up the author of a book in the catalog.""" if title not in CATALOG: - raise ValueError(f"No book titled {title!r} in the catalog.") + raise ToolError(f"No book titled {title!r} in the catalog.") return CATALOG[title] diff --git a/docs_src/handling_errors/tutorial004.py b/docs_src/handling_errors/tutorial004.py new file mode 100644 index 0000000000..baca11d666 --- /dev/null +++ b/docs_src/handling_errors/tutorial004.py @@ -0,0 +1,11 @@ +from mcp.server import MCPServer + +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.""" + return CATALOG[title] diff --git a/docs_src/troubleshooting/tutorial001.py b/docs_src/troubleshooting/tutorial001.py index e83a552df0..0b0f4840a7 100644 --- a/docs_src/troubleshooting/tutorial001.py +++ b/docs_src/troubleshooting/tutorial001.py @@ -1,5 +1,5 @@ from mcp.server import MCPServer -from mcp.server.mcpserver.exceptions import ResourceNotFoundError +from mcp.server.mcpserver.exceptions import ResourceNotFoundError, ToolError mcp = MCPServer("Weather") @@ -10,7 +10,7 @@ def forecast(city: str) -> str: """Today's forecast for one city.""" if city not in FORECASTS: - raise ValueError(f"No forecast for {city!r}.") + raise ToolError(f"No forecast for {city!r}.") return FORECASTS[city] diff --git a/docs_src/uri_templates/tutorial002.py b/docs_src/uri_templates/tutorial002.py index 3d0dc5c36b..94ca94c10d 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(encoding="utf-8") + file = safe_join(DOCS_ROOT, path) + if not file.is_file(): + raise ResourceNotFoundError(f"No manual at {path!r}.") + return file.read_text(encoding="utf-8") 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 239785e9a9..22656f3781 100644 --- a/src/mcp/server/mcpserver/exceptions.py +++ b/src/mcp/server/mcpserver/exceptions.py @@ -6,20 +6,68 @@ class MCPServerError(Exception): class ResourceError(MCPServerError): - """Error in resource operations.""" + """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. + """ class ResourceNotFoundError(ResourceError): """Resource does not exist. - Raise this from a resource template 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). """ +class UnexpectedResourceError(ResourceError): + """A resource read failed with something other than `ResourceError` or `MCPError`. + + 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): - """Error in tool operations.""" + """A tool failure you anticipated. + + Raise this from a tool (or a resolver) for a failure you saw coming: the + call returns `is_error=True` with your message in `content` for the model to + read, and the server logs it at INFO without a traceback. Any other exception + (bar `MCPError`, which is a protocol error) is treated as a crash: the model + sees only `Error executing tool `, and the server logs the traceback at + ERROR. 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` + 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`. + + 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. The message is + only `Error executing tool ` (followed by the same for a nested tool or + resource that crashed), so nothing from the original reaches the client. + `__cause__` is the original exception, which the server logs with its + traceback before returning the `is_error=True` result. Catch it around + `MCPServer.call_tool()` to tell a crash from a deliberate `ToolError`. + """ class InvalidSignature(Exception): diff --git a/src/mcp/server/mcpserver/prompts/base.py b/src/mcp/server/mcpserver/prompts/base.py index d30c0b3c60..a13b72f1aa 100644 --- a/src/mcp/server/mcpserver/prompts/base.py +++ b/src/mcp/server/mcpserver/prompts/base.py @@ -209,5 +209,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}") from exc diff --git a/src/mcp/server/mcpserver/resolve.py b/src/mcp/server/mcpserver/resolve.py index d4a744af37..2afbc516e3 100644 --- a/src/mcp/server/mcpserver/resolve.py +++ b/src/mcp/server/mcpserver/resolve.py @@ -575,7 +575,15 @@ async def _fulfil(marker: _Marker, key: str, res: _Resolution) -> ElicitationRes if res.context.session.can_send_request: _require_capability(res.context, marker, key) if isinstance(marker, Elicit): - return await res.context.elicit(marker.message, marker.schema) + try: + return await res.context.elicit(marker.message, marker.schema) + except ValueError as e: + # Accepted with no content, or content that fails the schema: the same + # client mistake the input_required path below reports as a ToolError. + # (A pydantic ValidationError here means a non-conformant client sent a + # malformed ElicitResult; its text is not repeated back.) + detail = "received an invalid elicitation response" if isinstance(e, ValidationError) else str(e) + raise ToolError(f"Resolver {key!r}: {detail}") from e result = await res.context.session.send_request( _render_request(marker), _result_type(marker), diff --git a/src/mcp/server/mcpserver/resources/templates.py b/src/mcp/server/mcpserver/resources/templates.py index 2ea99c19b6..621b2e9448 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 @@ -217,7 +214,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 exception. """ try: # Add context to params if needed @@ -246,5 +245,4 @@ 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 + 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..c8b479bb78 100644 --- a/src/mcp/server/mcpserver/resources/types.py +++ b/src/mcp/server/mcpserver/resources/types.py @@ -18,7 +18,6 @@ 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,33 +79,28 @@ class FunctionResource(Resource): async def read(self) -> str | bytes: """Read the resource by calling the wrapped function.""" - 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: - raise - except Exception as e: - raise ValueError(f"Error reading resource {self.uri}: {e}") + 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( @@ -183,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 e: - raise ValueError(f"Error reading file {self.path}: {e}") + 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): @@ -228,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 e: - raise ValueError(f"Error listing directory {self.path}: {e}") + 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 e: - raise ValueError(f"Error reading directory {self.path}: {e}") + 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 70e45329c5..d792f9cedc 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -44,7 +44,7 @@ from mcp_types import Resource as MCPResource from mcp_types import ResourceTemplate as MCPResourceTemplate from mcp_types import Tool as MCPTool -from pydantic import BaseModel +from pydantic import BaseModel, ValidationError from pydantic.networks import AnyUrl from starlette.applications import Starlette from starlette.middleware import Middleware @@ -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,18 @@ 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: + if isinstance(exc, ToolError) and not isinstance(exc, UnexpectedToolError): + if isinstance(exc.__cause__, ValidationError): + # Field names only: the rejected values are the caller's data. + fields = sorted({".".join(str(part) for part in err["loc"]) for err in exc.__cause__.errors()}) + logger.info("Tool %r rejected arguments: %r", params.name, fields) + else: + # %r keeps peer-supplied text on one line. + 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( self, ctx: ServerRequestContext[LifespanResultT], params: PaginatedRequestParams | None @@ -434,10 +450,13 @@ 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)}) + 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): return results contents: list[TextResourceContents | BlobResourceContents] = [] @@ -498,7 +517,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` or `ResourceError`. + UnexpectedToolError: If the tool (or a resolver) raises anything else, or + its return value fails output conversion. `__cause__` is the original + exception. + """ 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,23 +576,23 @@ 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 exception. """ 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: + except (MCPError, ResourceError): 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 + raise UnexpectedResourceError(f"Error reading resource {uri}") from exc def add_tool( self, @@ -711,10 +738,18 @@ def decorator(func: _CallableT) -> _CallableT: async def handler( ctx: ServerRequestContext[LifespanResultT], params: CompleteRequestParams ) -> CompleteResult: - result = await func(params.ref, params.argument, params.context) - return CompleteResult( - completion=result if result is not None else Completion(values=[], total=None, has_more=None), - ) + try: + result = await func(params.ref, params.argument, params.context) + return CompleteResult( + completion=result if result is not None else Completion(values=[], total=None, has_more=None), + ) + except MCPError: + raise + except Exception as exc: + logger.exception("Completion for argument %r raised an unexpected exception", params.argument.name) + raise MCPError( + code=INTERNAL_ERROR, message=f"Error completing argument {params.argument.name}" + ) from exc self._lowlevel_server.add_request_handler("completion/complete", CompleteRequestParams, handler) return func @@ -1293,7 +1328,7 @@ async def get_prompt( except MCPError: raise except Exception as e: - logger.exception(f"Error getting prompt {name}") + # 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 23248707a3..4a8bed792e 100644 --- a/src/mcp/server/mcpserver/tools/base.py +++ b/src/mcp/server/mcpserver/tools/base.py @@ -5,9 +5,15 @@ from typing import TYPE_CHECKING, Any from mcp_types import Icon, InputRequiredResult, ToolAnnotations -from pydantic import BaseModel, Field - -from mcp.server.mcpserver.exceptions import InvalidSignature, ToolError +from pydantic import BaseModel, Field, ValidationError + +from mcp.server.mcpserver.exceptions import ( + InvalidSignature, + ResourceError, + ToolError, + UnexpectedResourceError, + UnexpectedToolError, +) from mcp.server.mcpserver.resolve import ( build_resolver_plans, find_resolved_parameters, @@ -128,39 +134,52 @@ async def run( ) -> Any: """Run the tool with arguments. + Every failure other than `MCPError` is raised as a `ToolError` whose message + starts `Error executing tool ` and whose `__cause__` is what was raised. + An anticipated failure keeps its own text after the prefix. A crash does not, + so nothing from an unexpected exception reaches the client. + 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` 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: 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}") 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. return self.fn_metadata.convert_result(resolved) if convert_result else resolved pass_directly |= resolved - result = await self.fn_metadata.call_fn_with_arg_validation( - self.fn, - self.is_async, - arguments, - pass_directly or None, - pre_validated=pre_validated, - ) + result = await self.fn_metadata.call_fn(self.fn, self.is_async, validated, pass_directly) # 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 +196,15 @@ 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, 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. Its + # message is already the generic one, so it is safe to carry along. + raise UnexpectedToolError(f"Error executing tool {self.name}: {exc}") from exc + 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: + # A crash: the exception's own text stays on the server. + raise UnexpectedToolError(f"Error executing tool {self.name}") from exc diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index a4b7f4873e..5eab6efe59 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -23,7 +23,7 @@ ) from pydantic.fields import FieldInfo from pydantic.json_schema import GenerateJsonSchema, JsonSchemaWarningKind -from typing_extensions import NotRequired, ReadOnly, TypedDict, get_type_hints, is_typeddict +from typing_extensions import NotRequired, ReadOnly, TypedDict, deprecated, get_type_hints, is_typeddict from typing_inspection.introspection import ( UNKNOWN, AnnotationSource, @@ -35,6 +35,7 @@ from mcp.server.mcpserver.exceptions import InvalidSignature from mcp.server.mcpserver.utilities.logging import get_logger from mcp.server.mcpserver.utilities.types import Audio, Image +from mcp.shared.exceptions import MCPDeprecationWarning logger = get_logger(__name__) @@ -125,6 +126,28 @@ def validate_arguments(self, arguments_to_validate: dict[str, Any]) -> dict[str, arguments_parsed_model = self.arg_model.model_validate(arguments_pre_parsed) return arguments_parsed_model.model_dump_one_level() + async def call_fn( + self, + fn: Callable[..., Any | Awaitable[Any]], + fn_is_async: bool, + arguments: dict[str, Any], + arguments_to_pass_directly: dict[str, Any] | None = None, + ) -> Any: + """Call the function with already-validated `arguments` plus `arguments_to_pass_directly`. + + `arguments` is the output of `validate_arguments`. A sync function runs on a + worker thread. + """ + kwargs = arguments | (arguments_to_pass_directly or {}) + if fn_is_async: + return await fn(**kwargs) + return await anyio.to_thread.run_sync(functools.partial(fn, **kwargs)) + + @deprecated( + "FuncMetadata.call_fn_with_arg_validation() is deprecated and will be removed in 3.0; " + "call validate_arguments() and then call_fn() instead.", + category=MCPDeprecationWarning, + ) async def call_fn_with_arg_validation( self, fn: Callable[..., Any | Awaitable[Any]], @@ -133,25 +156,12 @@ async def call_fn_with_arg_validation( arguments_to_pass_directly: dict[str, Any] | None, pre_validated: dict[str, Any] | None = None, ) -> Any: - """Call the given function with arguments validated and injected. + """Validate `arguments_to_validate` (unless `pre_validated` is given) and call the function. - Arguments are first attempted to be parsed from JSON, then validated against - the argument model, before being passed to the function. Pass `pre_validated` - (the output of `validate_arguments`) to reuse an earlier validation pass - - validating twice can re-run `default_factory`/stateful validators and hand the - function different values than a caller already observed. + Deprecated: call `validate_arguments` and then `call_fn`. """ - # Copy so a caller-provided `pre_validated` dict is never mutated in place. - arguments_parsed_dict = dict( - pre_validated if pre_validated is not None else self.validate_arguments(arguments_to_validate) - ) - - arguments_parsed_dict |= arguments_to_pass_directly or {} - - if fn_is_async: - return await fn(**arguments_parsed_dict) - else: - return await anyio.to_thread.run_sync(functools.partial(fn, **arguments_parsed_dict)) + arguments = pre_validated if pre_validated is not None else self.validate_arguments(arguments_to_validate) + return await self.call_fn(fn, fn_is_async, arguments, arguments_to_pass_directly) def convert_result(self, result: Any) -> CallToolResult | InputRequiredResult: """Convert a function call result into a `CallToolResult`. diff --git a/tests/docs_src/test_client.py b/tests/docs_src/test_client.py index c8292d989b..d07ee4c9e4 100644 --- a/tests/docs_src/test_client.py +++ b/tests/docs_src/test_client.py @@ -85,7 +85,7 @@ async def test_call_tool_result_has_three_things_to_read() -> None: async def test_a_raising_tool_is_a_result_not_an_exception() -> None: - """tutorial003 `!!! check`: the exception's message comes back in content with is_error=True.""" + """tutorial003 `!!! check`: the ToolError's message comes back in content with is_error=True.""" async with Client(tutorial003.mcp) as client: result = await client.call_tool("lookup_book", {"title": "Solaris"}) assert result.is_error diff --git a/tests/docs_src/test_deprecated.py b/tests/docs_src/test_deprecated.py index 090ca61643..5d2afddb2e 100644 --- a/tests/docs_src/test_deprecated.py +++ b/tests/docs_src/test_deprecated.py @@ -8,6 +8,7 @@ so the prose cannot drift away from what the SDK does. """ +import logging import warnings import pytest @@ -117,20 +118,25 @@ def test_mcp_deprecation_warning_is_a_user_warning() -> None: @pytest.mark.filterwarnings("error::mcp.MCPDeprecationWarning") -async def test_error_filter_turns_the_deprecated_call_into_the_documented_tool_error() -> None: +async def test_error_filter_turns_the_deprecated_call_into_the_documented_tool_error( + caplog: pytest.LogCaptureFixture, +) -> None: """The `!!! check`: `"error::mcp.MCPDeprecationWarning"` makes `old_log` fail. - Under the error filter the warning becomes the raised exception, the tool manager - wraps it, and the result is exactly the tool error the page quotes. + Under the error filter the warning becomes the raised exception, the tool wrapper treats it as a + crash, and the result plus the logged warning are exactly what the page quotes. """ + caplog.set_level(logging.ERROR, logger="mcp.server.mcpserver.server") async with Client(mcp) as client: result = await client.call_tool("old_log", {}) assert result.is_error [content] = result.content assert isinstance(content, TextContent) - assert content.text == ( - "Error executing tool old_log: The logging capability is deprecated as of 2026-07-28 (SEP-2577)." - ) + assert content.text == "Error executing tool old_log" + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert record.exc_info is not None and isinstance(record.exc_info[1], BaseException) + assert str(record.exc_info[1].__cause__) == "The logging capability is deprecated as of 2026-07-28 (SEP-2577)." + assert type(record.exc_info[1].__cause__).__name__ == "MCPDeprecationWarning" async def test_filterwarnings_ignore_silences_the_whole_category() -> None: diff --git a/tests/docs_src/test_elicitation.py b/tests/docs_src/test_elicitation.py index 17933816bd..87d71571a6 100644 --- a/tests/docs_src/test_elicitation.py +++ b/tests/docs_src/test_elicitation.py @@ -1,5 +1,6 @@ """`docs/handlers/elicitation.md`: every claim the page makes, proved against the real SDK.""" +import logging from typing import Literal import pytest @@ -123,8 +124,7 @@ async def on_elicit(context: ClientRequestContext, params: ElicitRequestParams) async with Client(tutorial001.mcp, mode="legacy", elicitation_callback=on_elicit) as client: result = await client.call_tool("book_table", {"date": "2025-12-25", "party_size": 2}) assert result.is_error - assert isinstance(result.content[0], TextContent) - assert "does not match the requested schema" in result.content[0].text + assert result.content == [TextContent(type="text", text="Error executing tool book_table")] class Address(BaseModel): @@ -158,15 +158,21 @@ async def choose_seating(ctx: Context) -> str: return result.data.area -async def test_a_nested_model_is_rejected_before_anything_is_sent() -> None: - """`!!! warning`: a non-primitive field raises `TypeError` inside `ctx.elicit`, with this exact message.""" +async def test_a_nested_model_is_rejected_before_anything_is_sent(caplog: pytest.LogCaptureFixture) -> None: + """`!!! warning`: a non-primitive field raises `TypeError` inside `ctx.elicit` with this exact message, + which fails the call and lands in the server log rather than on the wire.""" + caplog.set_level(logging.ERROR, logger="mcp.server.mcpserver.server") async with Client(schema_gate_server, mode="legacy") as client: result = await client.call_tool("sign_up", {}) assert result.is_error - assert isinstance(result.content[0], TextContent) - assert result.content[0].text == ( - "Error executing tool sign_up: Elicitation schema field 'address' rendered as " - "{'$ref': '#/$defs/Address'}, which is not a valid PrimitiveSchemaDefinition" + assert result.content == [TextContent(type="text", text="Error executing tool sign_up")] + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert record.exc_info is not None and record.exc_info[1] is not None + cause = record.exc_info[1].__cause__ + assert isinstance(cause, TypeError) + assert str(cause) == ( + "Elicitation schema field 'address' rendered as {'$ref': '#/$defs/Address'}, " + "which is not a valid PrimitiveSchemaDefinition" ) diff --git a/tests/docs_src/test_handling_errors.py b/tests/docs_src/test_handling_errors.py index 0c2629169c..9824d90dc9 100644 --- a/tests/docs_src/test_handling_errors.py +++ b/tests/docs_src/test_handling_errors.py @@ -1,17 +1,19 @@ """`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. pytestmark = [pytest.mark.anyio, pytest.mark.filterwarnings("error::mcp.MCPDeprecationWarning")] -async def test_a_plain_exception_becomes_a_tool_error_the_model_reads() -> None: - """tutorial001: any non-`MCPError` exception comes back as `is_error=True` with the message in `content`.""" +async def test_tool_error_becomes_a_tool_error_the_model_reads() -> None: + """tutorial001: `ToolError` comes back as `is_error=True` with the message in `content`.""" async with Client(tutorial001.mcp) as client: result = await client.call_tool("get_author", {"title": "Nothing"}) assert result.is_error @@ -21,6 +23,16 @@ async def test_a_plain_exception_becomes_a_tool_error_the_model_reads() -> None: assert result.structured_content is None +async def test_tool_error_is_one_info_line(caplog: pytest.LogCaptureFixture) -> None: + """tutorial001: on the server a `ToolError` is one INFO record with no traceback.""" + caplog.set_level(logging.INFO) + async with Client(tutorial001.mcp) as client: + await client.call_tool("get_author", {"title": "Nothing"}) + 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_title_the_catalog_knows_is_an_ordinary_result() -> None: """tutorial001: the non-raising path is a plain `is_error=False` result.""" async with Client(tutorial001.mcp) as client: @@ -55,6 +67,21 @@ async def test_mcp_error_only_fires_on_the_raising_path() -> None: assert result.structured_content == {"result": "Frank Herbert"} +async def test_any_other_exception_is_a_crash_the_model_sees_generically(caplog: pytest.LogCaptureFixture) -> None: + """tutorial004, "Any other exception": the `KeyError` text stays on the server; the model gets only the + generic line, and the log gets one ERROR record with the traceback under the documented message.""" + 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")] + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert (record.levelno, record.getMessage()) == (logging.ERROR, "Tool 'get_author' raised an unexpected exception") + assert record.exc_info is not None + logged = record.exc_info[1] + assert logged is not None and isinstance(logged.__cause__, KeyError) + + async def test_resource_not_found_error_maps_to_invalid_params() -> None: """tutorial003: `ResourceNotFoundError` from a template handler is `-32602` with the URI in `data`.""" async with Client(tutorial003.mcp) as client: @@ -69,12 +96,10 @@ 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.""" - async with Client(tutorial001.mcp, raise_exceptions=True) as client: + async with Client(tutorial004.mcp, raise_exceptions=True) 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.") - ] + assert result.content == [TextContent(type="text", text="Error executing tool get_author")] async def test_a_title_the_template_knows_reads_normally() -> None: @@ -84,3 +109,14 @@ 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_bad_argument_is_an_info_line_not_a_crash(caplog: pytest.LogCaptureFixture) -> None: + """ "Errors you never raise": 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_structured_output.py b/tests/docs_src/test_structured_output.py index a12e6d2e7d..dcac214378 100644 --- a/tests/docs_src/test_structured_output.py +++ b/tests/docs_src/test_structured_output.py @@ -1,5 +1,7 @@ """`docs/servers/structured-output.md`: every claim the page makes, proved against the real SDK.""" +import logging + import pytest from inline_snapshot import snapshot from mcp_types import EmbeddedResource, ImageContent, TextContent, TextResourceContents @@ -151,15 +153,19 @@ async def test_dict_str_return_is_not_wrapped() -> None: assert result.structured_content == {"London": 16.2, "Reykjavik": 4.4} -async def test_return_value_is_validated_against_the_schema() -> None: - """tutorial007: a return value that does not match the output schema is a tool error, not a result.""" +async def test_return_value_is_validated_against_the_schema(caplog: pytest.LogCaptureFixture) -> None: + """tutorial007: a return value that does not match the output schema is a tool error, not a result; + the field name goes to the server log, not the client.""" + caplog.set_level(logging.ERROR, logger="mcp.server.mcpserver.server") async with Client(tutorial007.mcp) as client: result = await client.call_tool("get_weather", {"city": "London"}) assert result.is_error assert result.structured_content is None - assert isinstance(result.content[0], TextContent) - assert result.content[0].text.startswith("Error executing tool get_weather: 1 validation error for WeatherData") - assert "humidity\n Field required" in result.content[0].text + assert result.content == [TextContent(type="text", text="Error executing tool get_weather")] + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert record.getMessage() == "Tool 'get_weather' raised an unexpected exception" + assert record.exc_info is not None and record.exc_info[1] is not None + assert "1 validation error for WeatherData\nhumidity\n Field required" in str(record.exc_info[1].__cause__) async def test_structured_output_false_opts_out() -> None: diff --git a/tests/docs_src/test_troubleshooting.py b/tests/docs_src/test_troubleshooting.py index 9c94b643c1..1e1b5e15b8 100644 --- a/tests/docs_src/test_troubleshooting.py +++ b/tests/docs_src/test_troubleshooting.py @@ -83,6 +83,28 @@ async def test_a_failing_tool_returns_is_error_true_instead_of_raising() -> None ] +async def test_a_crashing_tool_is_the_bare_form_with_its_traceback_in_the_server_log( + caplog: pytest.LogCaptureFixture, +) -> None: + """The bare `Error executing tool `: the tool raised something other than ToolError, its text + is withheld, and the server log carries the exact ERROR message the page names.""" + mcp = MCPServer("Weather") + + @mcp.tool() + def forecast(city: str) -> str: + """Today's forecast for one city. Crashes: the upstream table is missing the key.""" + forecasts: dict[str, str] = {} + return forecasts[city] + + caplog.set_level(logging.ERROR, logger="mcp.server.mcpserver.server") + async with Client(mcp) as client: + result = await client.call_tool("forecast", {"city": "Atlantis"}) + assert result.content == [TextContent(type="text", text="Error executing tool forecast")] + (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/docs_src/test_uri_templates.py b/tests/docs_src/test_uri_templates.py index 03e12d8174..16744fd447 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 86725bcb4f..11bffba7b5 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -1016,8 +1016,9 @@ def __post_init__(self) -> None: "mcpserver:tool:handler-throws": Requirement( source="sdk", behavior=( - "An exception raised by a tool function (ToolError or otherwise) is caught and returned as a " - "tool result with isError true and the failure text in content; it does not become a JSON-RPC error." + "An exception raised by a tool function is caught and returned as a tool result with isError true, " + "never a JSON-RPC error; a ToolError carries its message in content, any other exception carries only " + "the generic 'Error executing tool '." ), ), "mcpserver:tool:input-validation": Requirement( @@ -1307,8 +1308,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_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") 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/interaction/mcpserver/test_tools.py b/tests/interaction/mcpserver/test_tools.py index a6418ac9c5..bff5b9282d 100644 --- a/tests/interaction/mcpserver/test_tools.py +++ b/tests/interaction/mcpserver/test_tools.py @@ -84,9 +84,9 @@ def place(mode: Literal["fast", "slow"], point: Point, count: Annotated[int, Fie async def test_call_tool_function_exception_becomes_error_result(connect: Connect, unstamped: Unstamp) -> None: """An exception raised by a tool function is returned as an is_error result, not a JSON-RPC error. - The function's `-> str` annotation gives the tool a derived output schema, but the error - result is built before any schema validation runs, so no validation failure is layered on - top of the original exception. + The exception's own text ("boom") is withheld: an unexpected exception is a crash, and only + ToolError carries a message to the client. The function's `-> str` annotation gives the tool + a derived output schema, but the error result is built before any schema validation runs. """ mcp = MCPServer("errors") @@ -98,7 +98,7 @@ def explode() -> str: result = await client.call_tool("explode", {}) assert unstamped(result) == snapshot( - CallToolResult(content=[TextContent(text="Error executing tool explode: boom")], is_error=True) + CallToolResult(content=[TextContent(text="Error executing tool explode")], is_error=True) ) @@ -245,7 +245,7 @@ def add(a: int, b: int) -> str: @requirement("mcpserver:output-schema:server-validate") @requirement("mcpserver:output-schema:missing-structured") async def test_tool_with_output_schema_returning_mismatched_structured_content_is_an_error_result( - connect: Connect, + connect: Connect, unstamped: Unstamp ) -> None: """Structured content that fails the tool's own output schema is rejected on the server side. @@ -273,16 +273,14 @@ def missing() -> Annotated[CallToolResult, Weather]: mismatched_result = await client.call_tool("mismatched", {}) missing_result = await client.call_tool("missing", {}) - # The body of each message is raw pydantic ValidationError output (model name, field paths, - # an errors.pydantic.dev URL) and changes across pydantic versions, so only the SDK's stable - # prefix is asserted. - assert mismatched_result.is_error is True - assert isinstance(mismatched_result.content[0], TextContent) - assert mismatched_result.content[0].text.startswith("Error executing tool mismatched: 2 validation errors") - - assert missing_result.is_error is True - assert isinstance(missing_result.content[0], TextContent) - assert missing_result.content[0].text.startswith("Error executing tool missing: 1 validation error") + # A return value that fails its own output schema is the tool's bug, so the pydantic detail + # goes to the server log and the client gets only the generic crash text. + assert unstamped(mismatched_result) == snapshot( + CallToolResult(content=[TextContent(text="Error executing tool mismatched")], is_error=True) + ) + assert unstamped(missing_result) == snapshot( + CallToolResult(content=[TextContent(text="Error executing tool missing")], is_error=True) + ) @requirement("mcpserver:tool:duplicate-name") diff --git a/tests/server/mcpserver/resources/test_file_resources.py b/tests/server/mcpserver/resources/test_file_resources.py index db9f73e935..8149f5bee0 100644 --- a/tests/server/mcpserver/resources/test_file_resources.py +++ b/tests/server/mcpserver/resources/test_file_resources.py @@ -178,7 +178,7 @@ async def test_missing_file_error(temp_file: Path): name="test", path=missing, ) - with pytest.raises(ValueError, match="Error reading file"): + with pytest.raises(FileNotFoundError): await resource.read() @@ -192,7 +192,7 @@ 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(PermissionError): await resource.read() 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..d38ddd840c 100644 --- a/tests/server/mcpserver/resources/test_function_resources.py +++ b/tests/server/mcpserver/resources/test_function_resources.py @@ -80,7 +80,7 @@ def get_data() -> dict[str, str]: @pytest.mark.anyio async def test_error_handling(self): - """Test error handling in FunctionResource.""" + """read() lets the function's own exception propagate; MCPServer.read_resource does the wrapping.""" def failing_func() -> str: raise ValueError("Test error") @@ -90,8 +90,9 @@ def failing_func() -> str: name="test", fn=failing_func, ) - with pytest.raises(ValueError, match="Error reading resource function://test"): + with pytest.raises(ValueError) as exc: await resource.read() + assert str(exc.value) == "Test error" @pytest.mark.anyio async def test_basemodel_conversion(self): @@ -258,6 +259,6 @@ def ask() -> InputRequiredResult: with pytest.raises(ValueError) 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" + "static resources cannot return InputRequiredResult; " + "only resource template functions participate in the multi-round-trip flow" ) diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index eff3479279..0dff88c268 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -14,6 +14,7 @@ from pydantic import BaseModel, Field, ValidationError from typing_extensions import NotRequired, ReadOnly, Required +from mcp import MCPDeprecationWarning from mcp.server.mcpserver import Audio, Image from mcp.server.mcpserver.exceptions import InvalidSignature from mcp.server.mcpserver.utilities.func_metadata import ArgModelBase, FuncMetadata, func_metadata @@ -102,33 +103,35 @@ async def test_complex_function_runtime_arg_validation_non_json(): meta = func_metadata(complex_arguments_fn) # Test with minimum required arguments - result = await meta.call_fn_with_arg_validation( + result = await meta.call_fn( complex_arguments_fn, fn_is_async=False, - arguments_to_validate={ - "an_int": 1, - "must_be_none": None, - "must_be_none_dumb_annotation": None, - "list_of_ints": [1, 2, 3], - "list_str_or_str": "hello", - "an_int_annotated_with_field": 42, - "an_int_annotated_with_field_and_others": 5, - "an_int_annotated_with_junk": 100, - "unannotated": "test", - "my_model_a": {}, - "my_model_a_forward_ref": {}, - "my_model_b": {"how_many_shrimp": 5, "ok": {"x": 1}, "y": None}, - }, + arguments=meta.validate_arguments( + { + "an_int": 1, + "must_be_none": None, + "must_be_none_dumb_annotation": None, + "list_of_ints": [1, 2, 3], + "list_str_or_str": "hello", + "an_int_annotated_with_field": 42, + "an_int_annotated_with_field_and_others": 5, + "an_int_annotated_with_junk": 100, + "unannotated": "test", + "my_model_a": {}, + "my_model_a_forward_ref": {}, + "my_model_b": {"how_many_shrimp": 5, "ok": {"x": 1}, "y": None}, + } + ), arguments_to_pass_directly=None, ) assert result == "ok!" # Test with invalid types with pytest.raises(ValueError): - await meta.call_fn_with_arg_validation( + await meta.call_fn( complex_arguments_fn, fn_is_async=False, - arguments_to_validate={"an_int": "not an int"}, + arguments=meta.validate_arguments({"an_int": "not an int"}), arguments_to_pass_directly=None, ) @@ -138,31 +141,33 @@ async def test_complex_function_runtime_arg_validation_with_json(): """Test that JSON string arguments are parsed and validated correctly""" meta = func_metadata(complex_arguments_fn) - result = await meta.call_fn_with_arg_validation( + result = await meta.call_fn( complex_arguments_fn, fn_is_async=False, - arguments_to_validate={ - "an_int": 1, - "must_be_none": None, - "must_be_none_dumb_annotation": None, - "list_of_ints": "[1, 2, 3]", # JSON string - "list_str_or_str": '["a", "b", "c"]', # JSON string - "an_int_annotated_with_field": 42, - "an_int_annotated_with_field_and_others": "5", # JSON string - "an_int_annotated_with_junk": 100, - "unannotated": "test", - "my_model_a": "{}", # JSON string - "my_model_a_forward_ref": "{}", # JSON string - "my_model_b": '{"how_many_shrimp": 5, "ok": {"x": 1}, "y": null}', - }, + arguments=meta.validate_arguments( + { + "an_int": 1, + "must_be_none": None, + "must_be_none_dumb_annotation": None, + "list_of_ints": "[1, 2, 3]", # JSON string + "list_str_or_str": '["a", "b", "c"]', # JSON string + "an_int_annotated_with_field": 42, + "an_int_annotated_with_field_and_others": "5", # JSON string + "an_int_annotated_with_junk": 100, + "unannotated": "test", + "my_model_a": "{}", # JSON string + "my_model_a_forward_ref": "{}", # JSON string + "my_model_b": '{"how_many_shrimp": 5, "ok": {"x": 1}, "y": null}', + } + ), arguments_to_pass_directly=None, ) assert result == "ok!" @pytest.mark.anyio -async def test_call_fn_does_not_mutate_pre_validated(): - """A caller-provided `pre_validated` dict must not be mutated by the call.""" +async def test_call_fn_does_not_mutate_the_arguments_dict(): + """The validated-arguments dict a caller passes to `call_fn` is not mutated when injected kwargs are merged.""" def fn(x: int, ctx: str) -> str: return f"{x}:{ctx}" @@ -171,17 +176,34 @@ def fn(x: int, ctx: str) -> str: pre_validated = meta.validate_arguments({"x": 1}) snapshot = dict(pre_validated) - result = await meta.call_fn_with_arg_validation( + result = await meta.call_fn( fn, fn_is_async=False, - arguments_to_validate={"x": 1}, + arguments=pre_validated, arguments_to_pass_directly={"ctx": "injected"}, - pre_validated=pre_validated, ) assert result == "1:injected" assert pre_validated == snapshot # `ctx` was not leaked into the caller's dict +@pytest.mark.anyio +async def test_call_fn_with_arg_validation_still_works_and_warns(): + """The pre-3.0 helper keeps validating-then-calling, and says it is deprecated (visible MCPDeprecationWarning).""" + + def fn(x: int, ctx: str) -> str: + return f"{x}:{ctx}" + + meta = func_metadata(fn, skip_names=["ctx"]) + with pytest.warns(MCPDeprecationWarning, match="call_fn_with_arg_validation"): + assert await meta.call_fn_with_arg_validation(fn, False, {"x": "2"}, {"ctx": "a"}) == "2:a" # pyright: ignore[reportDeprecated] + with pytest.warns(MCPDeprecationWarning): + validated = meta.validate_arguments({"x": 3}) + result = await meta.call_fn_with_arg_validation( # pyright: ignore[reportDeprecated] + fn, False, {}, {"ctx": "b"}, pre_validated=validated + ) + assert result == "3:b" + + def test_str_vs_list_str(): """Test handling of string vs list[str] type annotations. @@ -290,10 +312,10 @@ async def test_lambda_function(): } async def check_call(args): - return await meta.call_fn_with_arg_validation( + return await meta.call_fn( fn, fn_is_async=False, - arguments_to_validate=args, + arguments=meta.validate_arguments(args), arguments_to_pass_directly=None, ) @@ -555,10 +577,10 @@ def handle_json_payload(payload: str, strict_mode: bool = False) -> str: # Test with a JSON object string json_payload = '{"action": "create", "resource": "user", "data": {"name": "Test User"}}' - result = await meta.call_fn_with_arg_validation( + result = await meta.call_fn( handle_json_payload, fn_is_async=False, - arguments_to_validate={"payload": json_payload, "strict_mode": True}, + arguments=meta.validate_arguments({"payload": json_payload, "strict_mode": True}), arguments_to_pass_directly=None, ) @@ -568,10 +590,10 @@ def handle_json_payload(payload: str, strict_mode: bool = False) -> str: # Test with JSON array string json_array_payload = '["task1", "task2", "task3"]' - result = await meta.call_fn_with_arg_validation( + result = await meta.call_fn( handle_json_payload, fn_is_async=False, - arguments_to_validate={"payload": json_array_payload}, + arguments=meta.validate_arguments({"payload": json_array_payload}), arguments_to_pass_directly=None, ) @@ -1306,17 +1328,19 @@ def func_with_reserved_names( meta = func_metadata(func_with_reserved_names) # Test validation with reserved names - result = await meta.call_fn_with_arg_validation( + result = await meta.call_fn( func_with_reserved_names, fn_is_async=False, - arguments_to_validate={ - "model_dump": "test_dump", - "model_validate": 42, - "dict": ["a", "b", "c"], - "json": {"key": "value"}, - "validate": True, - "normal_param": "normal", - }, + arguments=meta.validate_arguments( + { + "model_dump": "test_dump", + "model_validate": 42, + "dict": ["a", "b", "c"], + "json": {"key": "value"}, + "validate": True, + "normal_param": "normal", + } + ), arguments_to_pass_directly=None, ) diff --git a/tests/server/mcpserver/test_resolve.py b/tests/server/mcpserver/test_resolve.py index aa5ced266a..49f0f1314f 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,16 @@ 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 + assert result.content[0].text == "Error executing tool sneaky" + (record,) = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert (record.levelname, record.getMessage()) == ("ERROR", "Tool 'sneaky' raised an unexpected exception") + assert record.exc_info is not None and record.exc_info[1] is not None + assert "the multi-round flow is driven either by resolvers or by the tool body" in str(record.exc_info[1].__cause__) 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 c22d0ca907..64b665d17f 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,17 +43,24 @@ TextContent, TextResourceContents, ) -from pydantic import BaseModel +from pydantic import AfterValidator, BaseModel, ValidationError from starlette.applications import Starlette from starlette.routing import Mount, Route from typing_extensions import NotRequired, TypedDict 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, @@ -289,7 +298,7 @@ async def test_tool_exception_handling(self): assert len(result.content) == 1 content = result.content[0] assert isinstance(content, TextContent) - assert "Test error" in content.text + assert content.text == "Error executing tool error_tool_fn" assert result.is_error is True async def test_tool_error_handling(self): @@ -300,7 +309,7 @@ async def test_tool_error_handling(self): assert len(result.content) == 1 content = result.content[0] assert isinstance(content, TextContent) - assert "Test error" in content.text + assert content.text == "Error executing tool error_tool_fn" assert result.is_error is True async def test_tool_error_details(self): @@ -312,7 +321,7 @@ async def test_tool_error_details(self): content = result.content[0] assert isinstance(content, TextContent) assert isinstance(content.text, str) - assert "Test error" in content.text + assert content.text == "Error executing tool error_tool_fn" assert result.is_error is True async def test_tool_return_value_conversion(self): @@ -1796,6 +1805,75 @@ async def handle_completion( assert result.completion.values == ["bold", "italic", "underline"] +async def test_completion_handler_crash_is_logged_and_reaches_the_client_generically( + caplog: pytest.LogCaptureFixture, +) -> None: + """SDK-defined: a crashing completion handler is one ERROR record with its traceback, and the client + gets -32603 naming only the argument, not the exception's text.""" + mcp = MCPServer() + raised = RuntimeError("index warmup failed on shard 3") + + @mcp.completion() + async def complete(ref: PromptReference, argument: CompletionArgument, context: CompletionContext | None): + raise raised + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.complete( + ref=PromptReference(type="ref/prompt", name="greet"), argument={"name": "style", "value": "b"} + ) + + assert exc.value.error == snapshot(ErrorData(code=INTERNAL_ERROR, message="Error completing argument style")) + assert _server_records(caplog) == snapshot( + [("ERROR", "Completion for argument 'style' raised an unexpected exception", True)] + ) + assert raised in _cause_chain(_logged_exception(caplog)) + + +async def test_completion_handler_returning_the_wrong_type_is_a_crash(caplog: pytest.LogCaptureFixture) -> None: + """SDK-defined: a completion handler whose return value isn't a Completion is the server's bug, so it is + logged as a crash and answered with the same generic -32603, not with 'Invalid request parameters'.""" + mcp = MCPServer() + + @mcp.completion() + async def complete(ref: PromptReference, argument: CompletionArgument, context: CompletionContext | None): + wrong: Any = ["bold", "italic"] + return wrong + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.complete( + ref=PromptReference(type="ref/prompt", name="greet"), argument={"name": "style", "value": "b"} + ) + + assert exc.value.error == snapshot(ErrorData(code=INTERNAL_ERROR, message="Error completing argument style")) + assert _server_records(caplog) == snapshot( + [("ERROR", "Completion for argument 'style' raised an unexpected exception", True)] + ) + assert isinstance(_cause_chain(_logged_exception(caplog))[-1], ValidationError) + + +async def test_completion_handler_raising_mcp_error_passes_through(caplog: pytest.LogCaptureFixture) -> None: + """SDK-defined: MCPError from a completion handler keeps its code and message and is not logged.""" + mcp = MCPServer() + + @mcp.completion() + async def complete(ref: PromptReference, argument: CompletionArgument, context: CompletionContext | None): + raise MCPError(code=INVALID_PARAMS, message="unknown argument") + + caplog.set_level(logging.INFO) + async with Client(mcp) as client: + with pytest.raises(MCPError) as exc: + await client.complete( + ref=PromptReference(type="ref/prompt", name="greet"), argument={"name": "style", "value": "b"} + ) + + assert exc.value.error == snapshot(ErrorData(code=INVALID_PARAMS, message="unknown argument")) + assert _server_records(caplog) == [] + + def test_streamable_http_no_redirect() -> None: """Test that streamable HTTP routes are correctly configured.""" mcp = MCPServer() @@ -2246,6 +2324,638 @@ def thing() -> str: assert exc.value.error.data == {"requiredCapabilities": ["elicitation"]} +def _cause_chain(exc: BaseException | None) -> list[BaseException]: + """`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__ + 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")] + 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, naming the fields but not the values.""" + 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) + # Field names only, repr-quoted: the rejected values are the caller's data and stay out of the log. + assert message == "Tool 'add' rejected arguments: ['a']" + 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_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")] + 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, +): + """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") + 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")] + 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] = [] diff --git a/tests/server/mcpserver/test_url_elicitation_error_throw.py b/tests/server/mcpserver/test_url_elicitation_error_throw.py index 29117e6936..6d2a659345 100644 --- a/tests/server/mcpserver/test_url_elicitation_error_throw.py +++ b/tests/server/mcpserver/test_url_elicitation_error_throw.py @@ -106,4 +106,4 @@ async def failing_tool(ctx: Context) -> str: assert result.is_error is True assert len(result.content) == 1 assert isinstance(result.content[0], types.TextContent) - assert "Something went wrong" in result.content[0].text + assert result.content[0].text == "Error executing tool failing_tool"