mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-09 07:09:11 +02:00
Merge remote-tracking branch 'origin/main' into feature/client-auto-default
This commit is contained in:
Jeremiah Lowin
commit
5f428aaced
5 changed files with 310 additions and 9 deletions
|
|
@ -407,6 +407,24 @@ A proxy is a server on its front and a client on its back, and the two eras have
|
|||
|
||||
*Verify:* `fastmcp_slim/fastmcp/server/providers/proxy.py` (`_mirror_front_era_mode`, the `_create_client_factory` non-Client branch, the era guard in `ProxyInitializeMiddleware.on_initialize`), `fastmcp_slim/fastmcp/client/transports/base.py` (`TransportOptions.backend_mode`), `fastmcp_slim/fastmcp/client/transports/config.py` (`MCPConfigTransport.connect_session` / `_create_proxy`), `fastmcp_slim/fastmcp/server/server.py` (`create_proxy` docstring), `tests/server/test_mrtr_guards.py` (`TestProxyEraMirroring`, `TestMultiServerConfigEraMirroring`).
|
||||
|
||||
### Resource and prompt errors survive the modern era — Absorbed (defect fix)
|
||||
|
||||
`_on_call_tool` returns a `ResourceError`-equivalent as an error result, but `_on_read_resource` and `_on_get_prompt` caught only `DisabledError`/`NotFoundError`, so a `ResourceError`, `PromptError`, or an argument-conversion failure on a resource template escaped as a raw handler exception. On the handshake eras that reached the wire as `str(exc)`, which is survivable; on `2026-07-28` the runner masks anything that is not an `MCPError` or `ValidationError` as a generic `"Internal server error"`, so a legitimate client-input error became indistinguishable from a server bug. Both handlers now translate a `FastMCPError` through `to_mcp_error` the way tools already do. Masking is unchanged — `mask_error_details` is still applied inside `read_resource`/`render_prompt`, so these paths leak no more than tools do.
|
||||
|
||||
*Verify:* `fastmcp_slim/fastmcp/server/mixins/mcp_operations.py` (`_on_read_resource`, `_on_get_prompt`), `tests/server/test_protocol_eras.py`.
|
||||
|
||||
### Proxies forward upstream instructions on the modern era — Absorbed (defect fix)
|
||||
|
||||
`ProxyInitializeMiddleware` forwards an upstream server's `instructions` by patching the `InitializeResult`, but `on_initialize` only fires for the handshake era. A modern client negotiates via `server/discover`, which the SDK builds from the low-level server's own `instructions`, so a proxy silently dropped its upstream's instructions for every modern client. `FastMCPProxy` now registers a `server/discover` handler (the same `add_request_handler` hook it already uses for `ping`, and a replacement the SDK explicitly sanctions) that delegates to the SDK's own implementation and fills in only the instructions that would otherwise be lost. The proxy's lazy-connect contract is unchanged: the backend is contacted when a client asks, never at construction. Because era mirroring pins a modern backend to an exact version — and a pinned version adopts a synthesized `DiscoverResult` rather than probing the wire — this read negotiates with `mode="auto"`; instructions are metadata with no back-channel, so they do not need the era consistency mirroring exists to protect.
|
||||
|
||||
*Verify:* `fastmcp_slim/fastmcp/server/providers/proxy.py` (`FastMCPProxy._setup_proxy_discover_handler`), `tests/server/providers/proxy/test_proxy_server.py` (`TestProxyModernEraInstructions`).
|
||||
|
||||
### Proxy list methods raise `MCPError` on backend failure — Breaking (in-process error type)
|
||||
|
||||
`ProxyProvider`'s four `_list_*` methods caught only `MCPError`, so a failed backend connection escaped as the `RuntimeError` the client wraps it in (or a raw `httpx2.ConnectError`). On the handshake eras that reached the wire as `str(exc)` and named the real failure; on `2026-07-28` it was masked as `"Internal server error"`, leaving a modern client unable to tell a dead backend from a server bug. The list methods now normalize transport failures through `_proxy_upstream_error`, matching `ProxyInitializeMiddleware.on_initialize`. Code calling a proxy's `list_tools()` (and friends) in-process must now catch `MCPError` rather than `RuntimeError`; the over-the-wire error type is unchanged.
|
||||
|
||||
*Verify:* `fastmcp_slim/fastmcp/server/providers/proxy.py` (`_PROXY_TRANSPORT_ERRORS` and the four `_list_*` methods), `tests/server/providers/proxy/test_proxy_server.py` (`TestProxyProviderTransportErrors`).
|
||||
|
||||
### The xfail register — Known gap
|
||||
|
||||
Roughly forty `xfail` markers across the test tree (concentrated in `tests/server/tasks/`, `tests/client/tasks/`, and `test_protocol_eras.py`) are the built-in beta tracker: each names the SDK gap it waits on. They are enumerated and mapped to sdk-feedback findings on the [Known Gaps](/development/v4-notes/known-gaps) page.
|
||||
|
|
|
|||
|
|
@ -318,6 +318,16 @@ class MCPOperationsMixin:
|
|||
raise to_mcp_error(
|
||||
NotFoundError(f"Resource not found: {str(uri)!r}")
|
||||
) from e
|
||||
except FastMCPError as e:
|
||||
# Resource-visible errors (ResourceError, ValidationError, ...)
|
||||
# must reach the wire as an MCPError. Resources have no
|
||||
# error-result shape the way tools do, so the equivalent of
|
||||
# _on_call_tool's error result is a translated MCPError: at
|
||||
# 2026-07-28 the runner only preserves MCPError/ValidationError
|
||||
# messages and masks anything else as "Internal server error",
|
||||
# which would hide a legitimate client-input error. Masking
|
||||
# already happened inside read_resource.
|
||||
raise to_mcp_error(e) from e
|
||||
|
||||
if isinstance(result, mcp_types.CreateTaskResult):
|
||||
return result
|
||||
|
|
@ -349,6 +359,13 @@ class MCPOperationsMixin:
|
|||
result = await self.render_prompt(name, arguments, version=version)
|
||||
except (DisabledError, NotFoundError) as e:
|
||||
raise to_mcp_error(NotFoundError(f"Unknown prompt: {name!r}")) from e
|
||||
except FastMCPError as e:
|
||||
# Prompt-visible errors (PromptError, ValidationError, ...) must
|
||||
# reach the wire as an MCPError for the same reason as
|
||||
# resources: at 2026-07-28 anything that is not an
|
||||
# MCPError/ValidationError is masked as "Internal server error".
|
||||
# Masking already happened inside render_prompt.
|
||||
raise to_mcp_error(e) from e
|
||||
|
||||
if isinstance(result, mcp_types.CreateTaskResult):
|
||||
return result
|
||||
|
|
|
|||
|
|
@ -94,6 +94,22 @@ PROXY_TRANSPORT_OPTIONS = TransportOptions(
|
|||
)
|
||||
|
||||
|
||||
#: Transport-level failures that can escape a backend connection attempt.
|
||||
#: `Client._connect` wraps most connect failures in a ``RuntimeError("Client
|
||||
#: failed to connect: ...")``, but a transport can also surface an httpx or
|
||||
#: anyio stream error directly. Every proxy entry point that opens a backend
|
||||
#: connection normalizes these into an ``MCPError`` so callers see a protocol
|
||||
#: error instead of a raw transport exception.
|
||||
_PROXY_TRANSPORT_ERRORS: tuple[type[Exception], ...] = (
|
||||
RuntimeError,
|
||||
TimeoutError,
|
||||
httpx2.HTTPError,
|
||||
anyio.ClosedResourceError,
|
||||
anyio.EndOfStream,
|
||||
anyio.BrokenResourceError,
|
||||
)
|
||||
|
||||
|
||||
def _proxy_upstream_error(error: Exception) -> MCPError:
|
||||
return MCPError(
|
||||
code=mcp_types.INTERNAL_ERROR,
|
||||
|
|
@ -163,14 +179,7 @@ class ProxyInitializeMiddleware(Middleware):
|
|||
upstream_instructions = init_result.instructions
|
||||
except MCPError:
|
||||
raise
|
||||
except (
|
||||
RuntimeError,
|
||||
TimeoutError,
|
||||
httpx2.HTTPError,
|
||||
anyio.ClosedResourceError,
|
||||
anyio.EndOfStream,
|
||||
anyio.BrokenResourceError,
|
||||
) as error:
|
||||
except _PROXY_TRANSPORT_ERRORS as error:
|
||||
raise _proxy_upstream_error(error) from error
|
||||
|
||||
result = await call_next(context)
|
||||
|
|
@ -739,6 +748,8 @@ class ProxyProvider(Provider):
|
|||
tools = []
|
||||
else:
|
||||
raise
|
||||
except _PROXY_TRANSPORT_ERRORS as error:
|
||||
raise _proxy_upstream_error(error) from error
|
||||
self._tools_cache = _CacheEntry(tools, time.monotonic())
|
||||
return tools
|
||||
|
||||
|
|
@ -776,6 +787,8 @@ class ProxyProvider(Provider):
|
|||
resources = []
|
||||
else:
|
||||
raise
|
||||
except _PROXY_TRANSPORT_ERRORS as error:
|
||||
raise _proxy_upstream_error(error) from error
|
||||
self._resources_cache = _CacheEntry(resources, time.monotonic())
|
||||
return resources
|
||||
|
||||
|
|
@ -813,6 +826,8 @@ class ProxyProvider(Provider):
|
|||
templates = []
|
||||
else:
|
||||
raise
|
||||
except _PROXY_TRANSPORT_ERRORS as error:
|
||||
raise _proxy_upstream_error(error) from error
|
||||
self._templates_cache = _CacheEntry(templates, time.monotonic())
|
||||
return templates
|
||||
|
||||
|
|
@ -850,6 +865,8 @@ class ProxyProvider(Provider):
|
|||
prompts = []
|
||||
else:
|
||||
raise
|
||||
except _PROXY_TRANSPORT_ERRORS as error:
|
||||
raise _proxy_upstream_error(error) from error
|
||||
self._prompts_cache = _CacheEntry(prompts, time.monotonic())
|
||||
return prompts
|
||||
|
||||
|
|
@ -1089,6 +1106,7 @@ class FastMCPProxy(FastMCP):
|
|||
self.add_provider(provider)
|
||||
self.middleware.append(ProxyInitializeMiddleware(self))
|
||||
self._setup_proxy_ping_handler()
|
||||
self._setup_proxy_discover_handler()
|
||||
|
||||
async def _get_client(self) -> Client:
|
||||
client = self.client_factory()
|
||||
|
|
@ -1110,6 +1128,63 @@ class FastMCPProxy(FastMCP):
|
|||
"ping", mcp_types.RequestParams, ping_remote
|
||||
)
|
||||
|
||||
def _setup_proxy_discover_handler(self) -> None:
|
||||
"""Forward the backend's instructions on the modern (`server/discover`) path.
|
||||
|
||||
`ProxyInitializeMiddleware` forwards upstream instructions by patching
|
||||
the `InitializeResult`, but `on_initialize` only fires for the legacy
|
||||
handshake. A modern client negotiates via `server/discover`, whose
|
||||
default SDK handler reads `self.instructions` off the low-level server
|
||||
directly, so a proxy would silently drop its upstream's instructions for
|
||||
every modern client.
|
||||
|
||||
The SDK sanctions replacing this handler wholesale, so we delegate to
|
||||
its own implementation for the rest of the result (supported versions,
|
||||
capabilities, server info) and only fill in the instructions we would
|
||||
otherwise lose. Resolving them here — at request time, from a live
|
||||
backend session — keeps the proxy's lazy-connect contract intact: the
|
||||
backend is contacted when a client actually asks, never at construction.
|
||||
"""
|
||||
build_default_result = self._mcp_server._handle_discover
|
||||
|
||||
async def discover_remote(
|
||||
ctx: ServerRequestContext[Any, Any],
|
||||
params: mcp_types.RequestParams | None,
|
||||
) -> mcp_types.DiscoverResult:
|
||||
result = await build_default_result(ctx, params)
|
||||
# A proxy with its own instructions keeps them, matching the
|
||||
# precedence `ProxyInitializeMiddleware` applies on the legacy path.
|
||||
if result.instructions is not None:
|
||||
return result
|
||||
client = await self._get_client()
|
||||
# `session.instructions` is era-neutral: it reads the backend's
|
||||
# `DiscoverResult` or `InitializeResult` depending on what the
|
||||
# backend negotiated, so a modern front can proxy a legacy backend.
|
||||
if client.is_connected():
|
||||
result.instructions = client.session.instructions
|
||||
return result
|
||||
# Era mirroring pins a modern backend to an exact version, and a
|
||||
# pinned version adopts a synthesized `DiscoverResult` instead of
|
||||
# probing the wire — so the pinned client would report no
|
||||
# instructions at all. Instructions are metadata with no
|
||||
# back-channel, so this read does not need the era consistency
|
||||
# mirroring exists to protect; negotiate with "auto" instead, which
|
||||
# probes `server/discover` and falls back to the handshake for a
|
||||
# legacy-only backend.
|
||||
client.mode = "auto"
|
||||
try:
|
||||
async with client:
|
||||
result.instructions = client.session.instructions
|
||||
except MCPError:
|
||||
raise
|
||||
except _PROXY_TRANSPORT_ERRORS as error:
|
||||
raise _proxy_upstream_error(error) from error
|
||||
return result
|
||||
|
||||
self._mcp_server.add_request_handler(
|
||||
"server/discover", mcp_types.RequestParams, discover_remote
|
||||
)
|
||||
|
||||
|
||||
# -----------------------------------------------------------------------------
|
||||
# ProxyClient and Related
|
||||
|
|
|
|||
|
|
@ -4,12 +4,14 @@ import time
|
|||
from typing import Any, cast
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import httpx2
|
||||
import mcp_types
|
||||
import pytest
|
||||
from anyio import create_task_group
|
||||
from dirty_equals import Contains
|
||||
from mcp import MCPError
|
||||
from mcp_types import Icon, TextContent, TextResourceContents
|
||||
from mcp_types.version import MODERN_PROTOCOL_VERSIONS
|
||||
from pydantic import AnyUrl
|
||||
|
||||
from fastmcp import FastMCP
|
||||
|
|
@ -311,13 +313,20 @@ async def test_proxy_initialize_forwards_remote_connection_error():
|
|||
|
||||
|
||||
async def test_proxy_list_tools_surfaces_remote_connection_error():
|
||||
"""A dead backend surfaces as an MCPError naming the connection failure.
|
||||
|
||||
The provider normalizes transport failures into `MCPError` (rather than
|
||||
letting the client's `RuntimeError` escape) so the error survives the
|
||||
modern era's wire boundary, which masks any non-MCPError as a generic
|
||||
"Internal server error".
|
||||
"""
|
||||
port = find_available_port()
|
||||
proxy = create_proxy(
|
||||
StreamableHttpTransport(f"http://127.0.0.1:{port}/mcp"),
|
||||
provider_error_strategy="raise",
|
||||
)
|
||||
|
||||
with pytest.raises(RuntimeError, match="Client failed to connect"):
|
||||
with pytest.raises(MCPError, match="Client failed to connect"):
|
||||
await proxy.list_tools()
|
||||
|
||||
|
||||
|
|
@ -1456,3 +1465,97 @@ class TestProxyForwardingAppliesToEveryBackendClient:
|
|||
|
||||
assert result.is_error is False
|
||||
assert result.structured_content == {"status": "weird"}
|
||||
|
||||
|
||||
class TestProxyModernEraInstructions:
|
||||
"""Upstream instructions must reach a client on the modern era too.
|
||||
|
||||
`ProxyInitializeMiddleware.on_initialize` only fires for the legacy
|
||||
handshake. A `mode="auto"` client negotiates via `server/discover`, which
|
||||
the SDK builds from the low-level server's own `instructions`, so without a
|
||||
discover-side hook the proxy drops its upstream's instructions entirely.
|
||||
"""
|
||||
|
||||
async def test_proxy_forwards_upstream_instructions_on_modern_era(self):
|
||||
upstream = FastMCP(name="upstream", instructions="USE_THIS_MARKER_123")
|
||||
proxy = create_proxy(upstream, name="proxy")
|
||||
|
||||
async with Client(proxy, mode="auto") as client:
|
||||
assert client.protocol_version in MODERN_PROTOCOL_VERSIONS
|
||||
assert client.session.instructions == "USE_THIS_MARKER_123"
|
||||
|
||||
async def test_proxy_own_instructions_take_precedence_on_modern_era(self):
|
||||
upstream = FastMCP(name="upstream", instructions="upstream instructions")
|
||||
proxy = create_proxy(upstream, name="proxy", instructions="proxy instructions")
|
||||
|
||||
async with Client(proxy, mode="auto") as client:
|
||||
assert client.session.instructions == "proxy instructions"
|
||||
|
||||
async def test_proxy_instructions_none_when_upstream_has_none_on_modern_era(self):
|
||||
upstream = FastMCP(name="upstream")
|
||||
proxy = create_proxy(upstream, name="proxy")
|
||||
|
||||
async with Client(proxy, mode="auto") as client:
|
||||
assert client.session.instructions is None
|
||||
|
||||
|
||||
class TestProxyProviderTransportErrors:
|
||||
"""A dead backend must surface as an MCPError, not a raw transport error.
|
||||
|
||||
`ProxyTool.run` and `ProxyInitializeMiddleware.on_initialize` normalize
|
||||
connection failures into `MCPError`; the provider's list methods caught
|
||||
only `MCPError`, so an `httpx2.ConnectError` (or the `RuntimeError` the
|
||||
client wraps a failed connect in) escaped unwrapped to the caller.
|
||||
"""
|
||||
|
||||
@pytest.fixture
|
||||
def unreachable_provider(self) -> ProxyProvider:
|
||||
port = find_available_port()
|
||||
return ProxyProvider(
|
||||
lambda: ProxyClient(f"http://127.0.0.1:{port}/mcp/"),
|
||||
cache_ttl=0,
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"method",
|
||||
["_list_tools", "_list_resources", "_list_resource_templates", "_list_prompts"],
|
||||
)
|
||||
async def test_list_method_wraps_connection_failure(
|
||||
self, unreachable_provider: ProxyProvider, method: str
|
||||
):
|
||||
with pytest.raises(MCPError):
|
||||
await getattr(unreachable_provider, method)()
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"method",
|
||||
["_list_tools", "_list_resources", "_list_resource_templates", "_list_prompts"],
|
||||
)
|
||||
async def test_list_method_wraps_raw_transport_error(self, method: str):
|
||||
"""A raw transport error raised mid-call is normalized, not leaked."""
|
||||
|
||||
def exploding_factory() -> Client:
|
||||
raise httpx2.ConnectError("backend refused the connection")
|
||||
|
||||
provider = ProxyProvider(exploding_factory, cache_ttl=0)
|
||||
with pytest.raises(MCPError, match="backend refused the connection"):
|
||||
await getattr(provider, method)()
|
||||
|
||||
@pytest.mark.parametrize("mode", ["legacy", "auto"])
|
||||
async def test_connection_error_reaches_client_on_both_eras(self, mode: str):
|
||||
"""The actual defect: the modern era masked the connection failure.
|
||||
|
||||
An unwrapped `RuntimeError` reaching the modern wire boundary is
|
||||
replaced with a generic "Internal server error", so a client on the
|
||||
newer protocol could not tell a dead backend from a server bug. On the
|
||||
legacy era the same exception reached the wire as `str(exc)`, which is
|
||||
why nothing caught this while tests pinned the older version.
|
||||
"""
|
||||
port = find_available_port()
|
||||
proxy = create_proxy(
|
||||
StreamableHttpTransport(f"http://127.0.0.1:{port}/mcp"),
|
||||
provider_error_strategy="raise",
|
||||
)
|
||||
|
||||
with pytest.raises(MCPError, match="Client failed to connect"):
|
||||
async with Client(proxy, mode=mode) as client:
|
||||
await client.list_tools()
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ from pydantic import FileUrl
|
|||
|
||||
from fastmcp import Client as FastMCPClient
|
||||
from fastmcp import Context, FastMCP, settings
|
||||
from fastmcp.exceptions import PromptError, ResourceError
|
||||
from fastmcp.server.elicitation import AcceptedElicitation
|
||||
from fastmcp.server.middleware import Middleware
|
||||
|
||||
|
|
@ -732,3 +733,90 @@ async def test_middleware_runs_on_both_eras():
|
|||
|
||||
# One invocation observed from each era.
|
||||
assert counter.count == 2
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Resource / prompt handler errors must survive both eras
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def erroring_server() -> FastMCP:
|
||||
"""A server whose resource and prompt handlers raise FastMCP errors."""
|
||||
mcp = FastMCP("erroring")
|
||||
|
||||
@mcp.resource("data://boom")
|
||||
def boom() -> str:
|
||||
raise ResourceError("resource detail marker")
|
||||
|
||||
@mcp.resource("data://items/{item_id}")
|
||||
def item(item_id: int) -> str:
|
||||
return f"item {item_id}"
|
||||
|
||||
@mcp.prompt
|
||||
def explode() -> str:
|
||||
raise PromptError("prompt detail marker")
|
||||
|
||||
return mcp
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ALL_MODES)
|
||||
async def test_resource_error_message_reaches_client(
|
||||
erroring_server: FastMCP, mode: str
|
||||
) -> None:
|
||||
"""A ResourceError's message must reach the wire on every era.
|
||||
|
||||
The modern runner masks any handler exception that is not an MCPError or a
|
||||
ValidationError as a generic "Internal server error", so a ResourceError
|
||||
that escapes the handler becomes indistinguishable from a server bug.
|
||||
"""
|
||||
async with FastMCPClient(erroring_server, mode=mode) as client:
|
||||
with pytest.raises(MCPError) as exc_info:
|
||||
await client.read_resource("data://boom")
|
||||
|
||||
assert "resource detail marker" in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ALL_MODES)
|
||||
async def test_prompt_error_message_reaches_client(
|
||||
erroring_server: FastMCP, mode: str
|
||||
) -> None:
|
||||
"""A PromptError's message must reach the wire on every era."""
|
||||
async with FastMCPClient(erroring_server, mode=mode) as client:
|
||||
with pytest.raises(MCPError) as exc_info:
|
||||
await client.get_prompt("explode")
|
||||
|
||||
assert "prompt detail marker" in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ALL_MODES)
|
||||
async def test_resource_template_conversion_error_reaches_client(
|
||||
erroring_server: FastMCP, mode: str
|
||||
) -> None:
|
||||
"""A bad template argument is a client-input error, not a server fault.
|
||||
|
||||
This is the path that originally exposed the masking: converting
|
||||
``item_id`` to an int fails, and the resulting error must name the problem
|
||||
rather than surface as a generic internal error.
|
||||
"""
|
||||
async with FastMCPClient(erroring_server, mode=mode) as client:
|
||||
with pytest.raises(MCPError) as exc_info:
|
||||
await client.read_resource("data://items/not-an-int")
|
||||
|
||||
assert "Internal server error" not in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ALL_MODES)
|
||||
async def test_resource_error_masked_when_masking_enabled(mode: str) -> None:
|
||||
"""Masking still applies: resources leak no more than tools already do."""
|
||||
mcp = FastMCP("masked", mask_error_details=True)
|
||||
|
||||
@mcp.resource("data://boom")
|
||||
def boom() -> str:
|
||||
raise ValueError("secret internal detail")
|
||||
|
||||
async with FastMCPClient(mcp, mode=mode) as client:
|
||||
with pytest.raises(MCPError) as exc_info:
|
||||
await client.read_resource("data://boom")
|
||||
|
||||
assert "secret internal detail" not in str(exc_info.value)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue