diff --git a/tests/client/client/test_client.py b/tests/client/client/test_client.py index e2a19ef40..3cd031c5a 100644 --- a/tests/client/client/test_client.py +++ b/tests/client/client/test_client.py @@ -275,19 +275,11 @@ async def test_client_serialization_error(): async def test_server_deserialization_error(): """Test server error when JSON string cannot be converted to expected type. - # TODO(defect): `_on_get_prompt` in - # fastmcp_slim/fastmcp/server/mixins/mcp_operations.py only catches - # `(DisabledError, NotFoundError)`, unlike `_on_call_tool` which catches - # `FastMCPError` broadly and returns the message as an `is_error` result. - # A `PromptError` raised during argument conversion therefore escapes as a - # raw exception. On the legacy path the v1-compat exception ladder puts - # `str(exc)` on the wire; on the modern path `modern_error_data` only - # special-cases `MCPError`/`ValidationError` and otherwise masks the - # exception as a generic "Internal server error" (deliberately, so handler - # internals don't leak) -- so this client-input error is indistinguishable - # from a real server bug on the modern protocol. Pinned to legacy until - # `_on_get_prompt` maps `FastMCPError` (or specifically `PromptError`) to a - # proper `MCPError` the same way tool errors are surfaced. + `_on_get_prompt` in fastmcp_slim/fastmcp/server/mixins/mcp_operations.py + catches `FastMCPError` broadly and translates it into an `MCPError` via + `to_mcp_error`, the same way `_on_call_tool` surfaces tool errors. The + `PromptError` raised during argument conversion reaches the client with + its message intact on both protocol eras. """ server = FastMCP("TestServer") @@ -297,7 +289,7 @@ async def test_server_deserialization_error(): """Expects list of integers but will receive invalid JSON.""" return f"Got {len(numbers)} numbers" - client = Client(transport=FastMCPTransport(server), mode="legacy") + client = Client(transport=FastMCPTransport(server)) async with client: with pytest.raises(MCPError, match="Could not convert argument"): diff --git a/tests/client/client/test_error_handling.py b/tests/client/client/test_error_handling.py index ac1316250..7bb500d8b 100644 --- a/tests/client/client/test_error_handling.py +++ b/tests/client/client/test_error_handling.py @@ -1,24 +1,13 @@ """Client error handling tests. -Resource, resource-template, and prompt error *detail* surfacing is a -handshake-era behavior: the legacy read/get path converts a `ResourceError` / -`PromptError` into a client-visible message, while the modern (2026-07-28) -server runner surfaces the raised exception as a generic "Internal server -error". Tests asserting the detailed message are pinned to `mode="legacy"`; -tool-error tests (which flow through an `isError` `CallToolResult`) are -era-neutral and run on the default `auto`. - -# TODO(defect): `_on_read_resource` / `_on_get_prompt` in -# fastmcp_slim/fastmcp/server/mixins/mcp_operations.py only catch -# `(DisabledError, NotFoundError)`, unlike `_on_call_tool` which catches -# `FastMCPError` broadly and returns the message as an `is_error` result. A -# `ResourceError`/`PromptError` therefore escapes as a raw exception; on the -# modern protocol `modern_error_data` only special-cases `MCPError`/ -# `ValidationError` and otherwise masks it as a generic "Internal server -# error" (deliberately, so handler internals don't leak) - so this happens -# regardless of `mask_error_details`, and a client-input error is -# indistinguishable from a real server bug. See the matching TODO on -# `test_server_deserialization_error` in `tests/client/client/test_client.py`. +Resource, resource-template, and prompt error *detail* surfacing is +era-neutral. `_on_read_resource` / `_on_get_prompt` in +`fastmcp_slim/fastmcp/server/mixins/mcp_operations.py` catch `FastMCPError` +broadly and translate it into an `MCPError` via `to_mcp_error`, mirroring how +`_on_call_tool` returns tool errors as an `isError` `CallToolResult`. The +detailed message (a `ResourceError`/`PromptError`, or the `ResourceError`/ +`PromptError` that wraps an arbitrary handler exception) reaches the client +on the default `auto` mode exactly as it does on `mode="legacy"`. """ import logging @@ -106,7 +95,7 @@ class TestErrorHandling: async def exception_resource(): raise ValueError("This is an internal error (sensitive)") - client = Client(transport=FastMCPTransport(mcp), mode="legacy") + client = Client(transport=FastMCPTransport(mcp)) async with client: with pytest.raises(Exception) as excinfo: @@ -122,7 +111,7 @@ class TestErrorHandling: async def exception_resource(): raise ValueError("This is an internal error (sensitive)") - client = Client(transport=FastMCPTransport(mcp), mode="legacy") + client = Client(transport=FastMCPTransport(mcp)) async with client: with pytest.raises(Exception) as excinfo: @@ -138,7 +127,7 @@ class TestErrorHandling: async def error_resource(): raise ResourceError("This is a resource error (xyz)") - client = Client(transport=FastMCPTransport(mcp), mode="legacy") + client = Client(transport=FastMCPTransport(mcp)) async with client: with pytest.raises(Exception) as excinfo: @@ -152,7 +141,7 @@ class TestErrorHandling: async def exception_resource(id: str): raise ValueError("This is an internal error (sensitive)") - client = Client(transport=FastMCPTransport(mcp), mode="legacy") + client = Client(transport=FastMCPTransport(mcp)) async with client: with pytest.raises(Exception) as excinfo: @@ -168,7 +157,7 @@ class TestErrorHandling: async def exception_resource(id: str): raise ValueError("This is an internal error (sensitive)") - client = Client(transport=FastMCPTransport(mcp), mode="legacy") + client = Client(transport=FastMCPTransport(mcp)) async with client: with pytest.raises(Exception) as excinfo: @@ -184,7 +173,7 @@ class TestErrorHandling: async def error_resource(id: str): raise ResourceError("This is a resource error (xyz)") - client = Client(transport=FastMCPTransport(mcp), mode="legacy") + client = Client(transport=FastMCPTransport(mcp)) async with client: with pytest.raises(Exception) as excinfo: @@ -347,7 +336,7 @@ class TestLogLevel: "Resource unavailable, try again later", log_level=logging.WARNING ) - async with Client(transport=FastMCPTransport(mcp), mode="legacy") as client: + async with Client(transport=FastMCPTransport(mcp)) as client: with caplog.at_level(logging.WARNING): with pytest.raises(Exception) as exc_info: await client.read_resource_mcp("test://custom") @@ -370,7 +359,7 @@ class TestLogLevel: def regular_resource(): raise ResourceError("Something went wrong") - async with Client(transport=FastMCPTransport(mcp), mode="legacy") as client: + async with Client(transport=FastMCPTransport(mcp)) as client: with caplog.at_level(logging.ERROR): with pytest.raises(Exception) as exc_info: await client.read_resource_mcp("test://regular") @@ -392,7 +381,7 @@ class TestLogLevel: "Insufficient context, provide more details", log_level=logging.WARNING ) - async with Client(transport=FastMCPTransport(mcp), mode="legacy") as client: + async with Client(transport=FastMCPTransport(mcp)) as client: with caplog.at_level(logging.WARNING): with pytest.raises(Exception) as exc_info: await client.get_prompt("custom_level_prompt") @@ -415,7 +404,7 @@ class TestLogLevel: def regular_prompt(): raise PromptError("Something went wrong") - async with Client(transport=FastMCPTransport(mcp), mode="legacy") as client: + async with Client(transport=FastMCPTransport(mcp)) as client: with caplog.at_level(logging.ERROR): with pytest.raises(Exception) as exc_info: await client.get_prompt("regular_prompt") diff --git a/tests/server/providers/proxy/test_proxy_server.py b/tests/server/providers/proxy/test_proxy_server.py index bb2a91b5a..184a63e9a 100644 --- a/tests/server/providers/proxy/test_proxy_server.py +++ b/tests/server/providers/proxy/test_proxy_server.py @@ -209,19 +209,17 @@ async def test_create_proxy_with_transport(fastmcp_server): async def test_proxy_forwards_upstream_instructions(): - """A proxy should surface the upstream server's instructions in the handshake.""" + """A proxy should surface the upstream server's instructions in the handshake. + + `FastMCPProxy` registers a `server/discover` handler that forwards the + upstream's instructions, mirroring what `ProxyInitializeMiddleware.on_initialize` + already does for the legacy handshake, so `client.session.instructions` + (era-neutral) resolves the same way on both protocol eras. + """ upstream = FastMCP(name="upstream", instructions="USE_THIS_MARKER_123") proxy = create_proxy(upstream, name="proxy") - # TODO(mode="legacy" pin): `ProxyInitializeMiddleware.on_initialize` is the - # only place that copies the upstream's instructions onto the proxy's own - # handshake result, and `on_initialize` only fires for the legacy handshake. - # A front client on `mode="auto"` negotiates `server/discover` instead, whose - # result the SDK builds straight from `self.instructions` with no equivalent - # hook — so upstream instructions silently never reach a modern-era client - # through a proxy. `client.session.instructions` (era-neutral) comes back - # None here instead of "USE_THIS_MARKER_123". Real defect, not a test bug. - async with Client(proxy, mode="legacy") as client: + async with Client(proxy) as client: assert client.session.instructions == "USE_THIS_MARKER_123" @@ -331,25 +329,22 @@ async def test_proxy_list_tools_surfaces_remote_connection_error(): async def test_proxy_list_tools_client_surfaces_remote_connection_error(): + """With a modern front, connecting succeeds (no eager backend probe — see + test_proxy_ping_surfaces_wrong_remote_path) and the failure only surfaces + once `list_tools()` actually hits the dead backend. `ProxyProvider._list_tools` + now normalizes the raw `httpx2.ConnectError` from the failed backend connect + into the `MCPError("Client failed to connect...")` this test expects, the + same way `ProxyInitializeMiddleware.on_initialize` and `ProxyTool.run` + already did. + """ port = find_available_port() proxy = create_proxy( StreamableHttpTransport(f"http://127.0.0.1:{port}/mcp"), provider_error_strategy="raise", ) - # TODO(mode="legacy" pin): with a modern front, connecting succeeds (no - # eager backend probe — see test_proxy_ping_surfaces_wrong_remote_path) - # and the failure only surfaces once `list_tools()` actually hits the - # dead backend. But `ProxyProvider._list_tools` only catches `MCPError`; - # a raw `httpx2.ConnectError` from the failed backend connect propagates - # unwrapped instead of becoming the `MCPError("Client failed to - # connect...")` this test expects. `ProxyInitializeMiddleware.on_initialize` - # and `ProxyTool.run` already normalize connection failures this way — - # the same `except (RuntimeError, TimeoutError, httpx2.HTTPError, ...)` - # handling appears to be missing from `ProxyProvider`'s list methods. - # Real defect, not a test bug. with pytest.raises(MCPError, match="Client failed to connect"): - async with Client(proxy, mode="legacy") as client: + async with Client(proxy) as client: await client.list_tools()