mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-23 05:54:19 +02:00
Audit tests/client/ mode="legacy" pins
Remove 4 unjustified pins (proxy header passthrough, connect timeout, two response_title validation tests that fail before any request is dispatched). Keep 81 pins that genuinely exercise older-protocol-only behavior (ctx.elicit back-channel, sampling, roots, ping, session IDs, initialize handshake, client.set_logging_level). Flags a real defect: _on_read_resource/_on_get_prompt only catch (DisabledError, NotFoundError), unlike _on_call_tool which catches FastMCPError broadly. A ResourceError/PromptError escapes as a raw exception and the modern protocol's generic exception ladder masks it as "Internal server error", losing the detailed message tool errors still get. Left pinned with a TODO in test_client.py and test_error_handling.py rather than hidden.
This commit is contained in:
parent
bf352d8a4a
commit
ae43039d8e
6 changed files with 40 additions and 20 deletions
|
|
@ -275,9 +275,19 @@ async def test_client_serialization_error():
|
|||
async def test_server_deserialization_error():
|
||||
"""Test server error when JSON string cannot be converted to expected type.
|
||||
|
||||
Pinned to legacy: the detailed `PromptError` message is surfaced to the
|
||||
client only on the handshake era; the modern server runner reports the
|
||||
raised conversion error as a generic "Internal server error".
|
||||
# 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.
|
||||
"""
|
||||
|
||||
server = FastMCP("TestServer")
|
||||
|
|
|
|||
|
|
@ -7,6 +7,18 @@ 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`.
|
||||
"""
|
||||
|
||||
import logging
|
||||
|
|
|
|||
|
|
@ -222,9 +222,9 @@ async def test_elicitation_response_title_rejected_for_basemodel():
|
|||
async def elicitation_handler(message, response_type, params, ctx):
|
||||
return ElicitResult(action="accept", content={"name": "x"})
|
||||
|
||||
async with Client(
|
||||
mcp, mode="legacy", elicitation_handler=elicitation_handler
|
||||
) as client:
|
||||
# Not pinned: response_title is validated locally before any request is
|
||||
# dispatched, so this raises identically on every era.
|
||||
async with Client(mcp, elicitation_handler=elicitation_handler) as client:
|
||||
with pytest.raises(ToolError, match="response_title"):
|
||||
await client.call_tool("ask", {})
|
||||
|
||||
|
|
@ -245,9 +245,9 @@ async def test_elicitation_response_title_rejected_for_none():
|
|||
async def elicitation_handler(message, response_type, params, ctx):
|
||||
return ElicitResult(action="accept", content={})
|
||||
|
||||
async with Client(
|
||||
mcp, mode="legacy", elicitation_handler=elicitation_handler
|
||||
) as client:
|
||||
# Not pinned: response_title is validated locally before any request is
|
||||
# dispatched, so this raises identically on every era.
|
||||
async with Client(mcp, elicitation_handler=elicitation_handler) as client:
|
||||
with pytest.raises(ToolError, match="response_title"):
|
||||
await client.call_tool("ask", {})
|
||||
|
||||
|
|
@ -540,6 +540,10 @@ async def test_elicitation_handler_error():
|
|||
async def elicitation_handler(message, response_type, params, ctx):
|
||||
raise ValueError("Handler failed!")
|
||||
|
||||
# Pinned: the tool's broad `except Exception` means this would pass under
|
||||
# auto for the wrong reason (elicit() itself raising "unavailable on
|
||||
# 2026-07-28" rather than the handler's ValueError ever running). Legacy
|
||||
# pins the test to what it actually claims to exercise.
|
||||
async with Client(
|
||||
mcp, mode="legacy", elicitation_handler=elicitation_handler
|
||||
) as client:
|
||||
|
|
|
|||
|
|
@ -95,6 +95,8 @@ class TestSetLoggingLevel:
|
|||
async def test_set_logging_level(self, fastmcp_server: FastMCP):
|
||||
"""Client can set the minimum log level and lower-level messages are suppressed."""
|
||||
log_handler = LogHandler()
|
||||
# client.set_logging_level is a legacy-only RPC (deprecated per SEP-2577);
|
||||
# it does not exist on the modern protocol.
|
||||
async with Client(
|
||||
fastmcp_server, mode="legacy", log_handler=log_handler.handle_log
|
||||
) as client:
|
||||
|
|
|
|||
|
|
@ -196,11 +196,7 @@ async def test_client_headers_proxy(proxy_server: str):
|
|||
"""
|
||||
Test that client headers are passed through the proxy to the remove server.
|
||||
"""
|
||||
# The proxy backend forwards over the legacy handshake, so align the outer
|
||||
# client's era with it.
|
||||
async with Client(
|
||||
transport=StreamableHttpTransport(proxy_server), mode="legacy"
|
||||
) as client:
|
||||
async with Client(transport=StreamableHttpTransport(proxy_server)) as client:
|
||||
result = await client.read_resource("resource://get_headers_headers_get")
|
||||
assert isinstance(result[0], TextResourceContents)
|
||||
headers = json.loads(result[0].text)
|
||||
|
|
|
|||
|
|
@ -260,13 +260,9 @@ async def test_nested_streamable_http_server_resolves_correctly(nested_server: s
|
|||
class TestTimeout:
|
||||
async def test_timeout(self, streamable_http_server: ASGIServer):
|
||||
# note this transport behaves differently than others and raises
|
||||
# MCPError from the *client* context. Pinned to legacy: on a modern
|
||||
# (server/discover) connection a connect-time timeout surfaces as a raw
|
||||
# httpx.ReadTimeout from the probe rather than a wrapped MCPError.
|
||||
# MCPError from the *client* context
|
||||
with pytest.raises(MCPError, match="timed out"):
|
||||
async with streamable_http_server.client(
|
||||
timeout=0.02, mode="legacy"
|
||||
) as client:
|
||||
async with streamable_http_server.client(timeout=0.02) as client:
|
||||
await client.call_tool("sleep", {"seconds": 0.05})
|
||||
|
||||
async def test_timeout_tool_call(self, streamable_http_server: ASGIServer):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue