mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 15:19:10 +02:00
Unpin tests whose defects #4579 fixed
Resource/prompt error detail and proxy instructions/connection-error surfacing now work on the modern protocol era, so the tests pinned to mode="legacy" with a TODO(defect)/TODO(mode="legacy" pin) marker run on the default auto mode again.
This commit is contained in:
parent
5f428aaced
commit
e01c5932bf
3 changed files with 41 additions and 65 deletions
|
|
@ -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"):
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue