Surface resource, prompt, and proxy errors on the modern protocol - #4579
Conversation
Three defects hidden by tests pinned to the handshake era, where a raw exception reaches the wire as str(exc). At 2026-07-28 the runner masks anything that is not an MCPError/ValidationError as "Internal server error". - _on_read_resource / _on_get_prompt now translate FastMCPError through to_mcp_error, mirroring _on_call_tool. Masking is unchanged. - FastMCPProxy registers a server/discover handler so upstream instructions reach modern clients; on_initialize only fires for the handshake. - ProxyProvider's list methods normalize transport failures into MCPError.
marvin-context-protocol
Bot
added
breaking change
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43fe00744d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 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" |
There was a problem hiding this comment.
When the supplied backend is a disconnected Client pinned to a modern version with prior_discover=..., forcing mode to "auto" makes _negotiate() ignore that result even though it may already contain the upstream instructions. This introduces an unnecessary server/discover probe and can make proxy negotiation fail or fall back to the legacy handshake for a backend that was intentionally configured for direct pinned adoption. Use the pinned client's prior discovery when available rather than overriding its negotiation mode.
Useful? React with 👍 / 👎.
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.
1 participant
Auditing the test suite for the
mode="auto"default surfaced three latent defects, all with the same root cause: on the modern protocol (2026-07-28), the SDK's error handling preserves onlyMCPErrorandValidationErrorand replaces everything else with a generic"Internal server error". On the older handshake protocol the same exception reached the wire as its real message. So any code path that raised a non-MCPErrortoward the wire was silently protocol-version-dependent, and tests pinned to the older protocol could never see it.Resource and prompt errors were masked.
_on_call_toolcaughtFastMCPErrorbroadly, but_on_read_resourceand_on_get_promptcaught only(DisabledError, NotFoundError). AResourceErrororPromptError— for example from bad argument conversion on a resource template — escaped, and on the modern protocol became indistinguishable from a genuine server bug. Both handlers now translateFastMCPErrortoMCPError, mirroring how tools already surface these, withmask_error_detailsrespected exactly as before.A proxy dropped its upstream's instructions for modern clients. Instructions were copied onto the proxy's result only inside the legacy
initializehandler. A modern client negotiatesserver/discover, which had no equivalent hook, so the upstream server's instructions silently never arrived. The proxy now registers aserver/discoverhandler that fills in the upstream instructions, without changing when the proxy first contacts its backend — proxies still connect lazily.Proxy list methods leaked masked errors.
ProxyProvider's list methods caught onlyMCPError, so a backend connection failure surfaced as"Internal server error"on the modern protocol instead of the clear"Client failed to connect..."the older protocol produced. They now normalize transport errors the same way the proxy's other paths do.Each fix has a regression test verified to fail against
mainand split precisely on protocol version.Note one behavior change for reviewers: a proxy's in-process
list_tools()now raisesMCPErrorrather thanRuntimeErroron a backend connection failure, matching the over-the-wire contract. This is recorded in the change register as breaking.Label: bugs