From 0840a9f3058d2743210aa6bede518f285dda0b47 Mon Sep 17 00:00:00 2001 From: James Yang Date: Tue, 11 Aug 2026 13:20:36 -0400 Subject: [PATCH 1/2] docs: clarify when Client(raise_exceptions=True) actually raises. Document that the flag only unsanitises unexpected in-memory handler crashes (still MCPError, with message/__cause__), leaves tool is_error results alone, and is ignored for URL/transport clients. Fixes #3287. --- docs/advanced/low-level-server.md | 2 +- docs/client/index.md | 7 +++- docs/get-started/testing.md | 58 ++++++++++++++++++++++++------- docs/troubleshooting.md | 9 +++++ docs_src/testing/tutorial002.py | 33 ++++++++++++++++++ src/mcp/client/client.py | 19 ++++++++-- tests/docs_src/test_testing.py | 31 ++++++++++++++--- 7 files changed, 138 insertions(+), 21 deletions(-) create mode 100644 docs_src/testing/tutorial002.py diff --git a/docs/advanced/low-level-server.md b/docs/advanced/low-level-server.md index 083e03cd61..7fb3acbe0e 100644 --- a/docs/advanced/low-level-server.md +++ b/docs/advanced/low-level-server.md @@ -72,7 +72,7 @@ The same text the `@mcp.tool()` version produced. Two honest differences: MCPError: Internal server error ``` - A JSON-RPC error, code `-32603`, with a deliberately generic message: the SDK won't leak your traceback to a remote caller. The model never finds out what it did wrong, so it can't retry. (In a test, `raise_exceptions=True` surfaces the real exception instead; see **[Testing](../get-started/testing.md)**.) + A JSON-RPC error, code `-32603`, with a deliberately generic message: the SDK won't leak your traceback to a remote caller. The model never finds out what it did wrong, so it can't retry. (In a test, `Client(server, raise_exceptions=True)` keeps the `MCPError` but puts the real message on it and chains the original as `__cause__`; see **[Testing](../get-started/testing.md)**.) That generalises. An exception raised from a low-level handler is **always** a protocol error, never an `is_error=True` tool result. If you want the model to read the failure and recover, validate `params.arguments` yourself and return `CallToolResult(content=[TextContent(...)], is_error=True)`. The two kinds of failure are the subject of **[Handling errors](../servers/handling-errors.md)**. diff --git a/docs/client/index.md b/docs/client/index.md index 1e1df3c01b..d62793a730 100644 --- a/docs/client/index.md +++ b/docs/client/index.md @@ -197,7 +197,12 @@ This loop is correct against every server. `MCPServer` returns everything in one `Client(mcp)` with no process and no port is already a test harness for your server. -There is one constructor flag built for that: `Client(mcp, raise_exceptions=True)`. It only has an effect on in-memory connections, and **[Testing](../get-started/testing.md)** is the page that explains it and builds the whole pattern around it. +There is one constructor flag built for that: `Client(mcp, raise_exceptions=True)`. It only has an +effect on in-memory connections (ignored for URL strings and transports). On the modern +in-process path it does **not** make the original exception raise in place of `MCPError` — it +unsanitises an unexpected handler crash so the `MCPError` message is `str(original)` and +`__cause__` is the original. Tool `is_error=True` results are unchanged. **[Testing](../get-started/testing.md)** +builds the whole pattern around it. ## Recap diff --git a/docs/get-started/testing.md b/docs/get-started/testing.md index 9abd281ceb..dadf69d88a 100644 --- a/docs/get-started/testing.md +++ b/docs/get-started/testing.md @@ -78,18 +78,52 @@ There you go! You can now extend your tests to cover more scenarios. Two different things can go wrong, and this flag only touches one of them. -An exception inside one of **your tools** is not a protocol failure. It becomes a normal result with -`is_error=True`, and the model reads the message. `raise_exceptions` doesn't change that: with or -without it, `call_tool` returns the same `is_error=True` result. There's a whole page on it: -**[Handling errors](../servers/handling-errors.md)**. - -A failure **outside** a tool body is different. On the connection `Client(mcp)` gives you, the -server sanitises it into a generic `"Internal server error"` before the client sees it. You should -never leak the details of an unexpected crash to a remote caller. In a test that is exactly what -you *don't* want, and it is what `raise_exceptions=True` changes: your test sees the real message -instead of the sanitised one. - -Leave it on in tests. It has no meaning in production code. +An exception inside one of **your `@mcp.tool()` functions** is not a protocol failure. It becomes a +normal result with `is_error=True`, and the model reads the message. `raise_exceptions` doesn't +change that: with or without it, `call_tool` returns the same `is_error=True` result. There's a +whole page on it: **[Handling errors](../servers/handling-errors.md)**. + +An **unexpected exception** that escapes a request handler as a bare Python exception is +different. On an in-memory `Client(server)` connection the SDK still turns that into an +`MCPError` (`-32603`) — the call raises; it does not become an `is_error` result — but by default +it sanitises the message to `"Internal server error"` and drops the original exception. You should +never leak a traceback to a remote caller. In a test that is exactly what you *don't* want. + +`raise_exceptions=True` keeps the `MCPError`, but puts `str(original)` in the message and chains +the original as `__cause__`. Catch it *inside* the `async with` so anyio does not wrap it in an +`ExceptionGroup` (**[Troubleshooting](../troubleshooting.md)**): + +```python title="test_buggy_handler.py" +import pytest +from mcp import Client, MCPError +from mcp.types import INTERNAL_ERROR + +from server import server # a low-level Server whose handler can KeyError + + +@pytest.mark.anyio +async def test_missing_argument_surfaces_the_real_key_error(): + async with Client(server, raise_exceptions=True) as client: + with pytest.raises(MCPError) as exc_info: + await client.call_tool("search_books", {"query": "dune"}) # no limit + assert exc_info.value.error.code == INTERNAL_ERROR + assert isinstance(exc_info.value.__cause__, KeyError) +``` + +The server that makes that failure visible is a low-level `Server` (it does not validate +`input_schema` before calling you): + +```python title="server.py" +--8<-- "docs_src/testing/tutorial002.py" +``` + +Without the flag, the same call raises `MCPError: Internal server error` with no `__cause__`. +With a high-level `MCPServer`, most handler failures are already converted into tool +`is_error` results or intentional `MCPError`s before this flag can act — so the difference shows +up mainly for unmapped crashes (and for low-level `Server` handlers). + +Leave it on in tests that use the in-memory client. It is ignored for `Client("https://...")` +and for a user-supplied `Transport`: those paths never see the flag. ## In-process by default diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 75a6652ecc..b80f925b43 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -56,6 +56,14 @@ async def main() -> None: down this page) escapes from `async with` itself, so there is no "inside" to catch it in. For those, read the bottom of the group. +!!! tip + Seeing only `MCPError: Internal server error` in an in-memory test? That is the sanitised + form of an unexpected handler crash. `Client(mcp, raise_exceptions=True)` keeps the + `MCPError` but puts the real message on it and chains the original as `__cause__` — still + catch `MCPError` inside the block. The flag is ignored for URL/transport clients, does not + turn a tool's `is_error=True` into an exception, and should be dropped on + `mode="legacy"`. **[Testing](get-started/testing.md)** is the full story. + ## `RuntimeError: Client must be used within an async context manager` `Client(...)` only builds the object. Nothing connects until `async with`, so every method refuses: @@ -404,6 +412,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. +* In-memory `MCPError: Internal server error` is a sanitised handler crash; `raise_exceptions=True` unsanitises the message and `__cause__` (see **[Testing](get-started/testing.md)**). * `call_tool` does not raise for a failing tool. `Error executing tool ...` and `Unknown tool: ...` are results: check `result.is_error`. * `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. diff --git a/docs_src/testing/tutorial002.py b/docs_src/testing/tutorial002.py new file mode 100644 index 0000000000..34948909ef --- /dev/null +++ b/docs_src/testing/tutorial002.py @@ -0,0 +1,33 @@ +from mcp.server import Server, ServerRequestContext +from mcp.types import ( + CallToolRequestParams, + CallToolResult, + ListToolsResult, + PaginatedRequestParams, + TextContent, + Tool, +) + +BUGGY = Tool( + name="search_books", + description="Search the catalog by title or author.", + input_schema={ + "type": "object", + "properties": {"query": {"type": "string"}, "limit": {"type": "integer"}}, + "required": ["query", "limit"], + }, +) + + +async def list_tools(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult: + return ListToolsResult(tools=[BUGGY]) + + +async def call_tool(ctx: ServerRequestContext, params: CallToolRequestParams) -> CallToolResult: + args = params.arguments or {} + # Missing `limit` reaches the handler: low-level Server does not validate input_schema. + text = f"Found 3 books matching {args['query']!r} (showing up to {args['limit']})." + return CallToolResult(content=[TextContent(type="text", text=text)]) + + +server = Server("Bookshop", on_list_tools=list_tools, on_call_tool=call_tool) diff --git a/src/mcp/client/client.py b/src/mcp/client/client.py index ed7c40f123..9f0846fa04 100644 --- a/src/mcp/client/client.py +++ b/src/mcp/client/client.py @@ -293,9 +293,24 @@ async def main(): _: KW_ONLY - # TODO(Marcelo): When do `raise_exceptions=True` actually raises? raise_exceptions: bool = False - """Whether to raise exceptions from the server.""" + """Unsanitize unexpected in-process handler failures (tests only). + + Only has an effect for in-memory ``Client(server)`` connections. Ignored for + URL strings and user-supplied ``Transport`` instances. + + On the default modern in-process path, an unmapped handler exception still + surfaces to the caller as ``MCPError`` either way. With ``False`` (the + default) the message is the opaque ``"Internal server error"`` and there is + no ``__cause__``. With ``True`` the message is ``str(original)`` and the + original exception is chained as ``__cause__``. + + Does **not** turn a tool's ``is_error=True`` result into an exception, and + does not change intentional ``MCPError`` raised by a handler. Catch + ``MCPError`` *inside* ``async with Client(...)`` so anyio does not wrap it + in an ``ExceptionGroup``; see **Testing** and **Troubleshooting** in the + docs. + """ read_timeout_seconds: float | None = None """Timeout for read operations.""" diff --git a/tests/docs_src/test_testing.py b/tests/docs_src/test_testing.py index a6104840fc..01b45f804e 100644 --- a/tests/docs_src/test_testing.py +++ b/tests/docs_src/test_testing.py @@ -6,10 +6,10 @@ import pytest from inline_snapshot import snapshot -from mcp_types import CallToolResult, TextContent +from mcp_types import INTERNAL_ERROR, CallToolResult, TextContent -from docs_src.testing.tutorial001 import mcp -from mcp import Client +from docs_src.testing import tutorial001, tutorial002 +from mcp import Client, MCPError from tests.docs_src._helpers import strip_server_info # See test_index.py for why this is a per-module mark and not a conftest hook. @@ -17,9 +17,30 @@ async def test_call_add_tool() -> None: - async with Client(mcp, raise_exceptions=True) as client: + """tutorial001: the page's fixture-shaped happy path with `raise_exceptions=True`.""" + async with Client(tutorial001.mcp, raise_exceptions=True) as client: result = await client.call_tool("add", {"a": 1, "b": 2}) - result = strip_server_info(result, mcp) + result = strip_server_info(result, tutorial001.mcp) assert result == snapshot( CallToolResult(content=[TextContent(type="text", text="3")], structured_content={"result": 3}) ) + + +async def test_raise_exceptions_true_chains_the_original_handler_error() -> None: + """The `Why raise_exceptions=True?` section: still `MCPError`, but message and `__cause__` are real.""" + async with Client(tutorial002.server, raise_exceptions=True) as client: + with pytest.raises(MCPError) as exc_info: + await client.call_tool("search_books", {"query": "dune"}) + assert exc_info.value.error.code == INTERNAL_ERROR + assert isinstance(exc_info.value.__cause__, KeyError) + assert exc_info.value.__cause__.args == ("limit",) + + +async def test_raise_exceptions_false_sanitises_the_handler_error() -> None: + """Without the flag, the same low-level crash is the opaque `"Internal server error"`.""" + async with Client(tutorial002.server, raise_exceptions=False) as client: + with pytest.raises(MCPError) as exc_info: + await client.call_tool("search_books", {"query": "dune"}) + assert exc_info.value.error.code == INTERNAL_ERROR + assert exc_info.value.error.message == "Internal server error" + assert exc_info.value.__cause__ is None From 2d123e7018bbc39c052961a511fce97a5605e39d Mon Sep 17 00:00:00 2001 From: James Yang Date: Tue, 11 Aug 2026 13:27:16 -0400 Subject: [PATCH 2/2] Drop troubleshooting raise_exceptions tip duplication. Keep the full semantics on the Testing page and the low-level-server cross-link; the troubleshooting tip restated the same material. --- docs/troubleshooting.md | 9 --------- 1 file changed, 9 deletions(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b80f925b43..75a6652ecc 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -56,14 +56,6 @@ async def main() -> None: down this page) escapes from `async with` itself, so there is no "inside" to catch it in. For those, read the bottom of the group. -!!! tip - Seeing only `MCPError: Internal server error` in an in-memory test? That is the sanitised - form of an unexpected handler crash. `Client(mcp, raise_exceptions=True)` keeps the - `MCPError` but puts the real message on it and chains the original as `__cause__` — still - catch `MCPError` inside the block. The flag is ignored for URL/transport clients, does not - turn a tool's `is_error=True` into an exception, and should be dropped on - `mode="legacy"`. **[Testing](get-started/testing.md)** is the full story. - ## `RuntimeError: Client must be used within an async context manager` `Client(...)` only builds the object. Nothing connects until `async with`, so every method refuses: @@ -412,7 +404,6 @@ 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. -* In-memory `MCPError: Internal server error` is a sanitised handler crash; `raise_exceptions=True` unsanitises the message and `__cause__` (see **[Testing](get-started/testing.md)**). * `call_tool` does not raise for a failing tool. `Error executing tool ...` and `Unknown tool: ...` are results: check `result.is_error`. * `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.