Return JSON-RPC error for unknown experiment resource - #24
Open
hhuo24 wants to merge 1 commit into
Open
Conversation
Author
Problem: get_experiment_resource returned json.dumps({"error": ...}) for a
missing experiment, so resources/read reported a *successful* result carrying
an in-band error body. The MCP 2026-07-28 Resources spec (Error Handling)
states a server MUST return JSON-RPC error -32602 for a non-existent resource
and MUST NOT return content that misrepresents a missing resource as a
successful read. Clients cannot otherwise distinguish a missing experiment
from a real one whose payload contains an "error" key.
Solution: Resolve experiment lookups through a single _resolve_experiment
helper that raises McpError(INVALID_PARAMS) on a miss (matching run_cli's
raise-on-failure contract). Templated resources are read via create_resource,
which wraps any raise into a generic ValueError and loses the code, so add a
thin read_resource override -- mirroring the existing list_resources override,
which likewise delegates to the original handler -- that validates the id and,
only on a miss, raises McpError so it reaches the client intact. Every hit and
every other URI is served by the unchanged original handler.
Impact: A missing experiment://{id} now returns JSON-RPC error -32602 instead
of a false-success body; successful reads are unchanged.
This change is made with the assistance of Claude Opus 4.8.
Signed-off-by: hhuo24 <hhuo2000@gmail.com>
hhuo24
force-pushed
the
fix/resource-not-found-errorcode
branch
from
August 13, 2026 21:05
abb43be to
ce27cfd
Compare
kavehshahedi
requested changes
Aug 14, 2026
kavehshahedi
left a comment
kavehshahedi
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the contribution! Overall, it's very well.
Just some minor comments to be addressed.
Comment on lines
+365
to
+366
| from mcp.shared.exceptions import McpError | ||
| from mcp.types import ErrorData, INVALID_PARAMS |
Contributor
There was a problem hiding this comment.
I'm not 100% sure if using lazy imports would be the optimal solution, but I guess for now it's fine. Please keep in mind to globalize these imports in case of reusage.
Comment on lines
+409
to
+416
| # Override read_resource for the same reason list_resources is overridden above: | ||
| # the handler reference is bound at construction time. A missing experiment MUST | ||
| # surface as JSON-RPC -32602, but FastMCP reads templated resources | ||
| # (experiment://{id}) through create_resource, which wraps ANY raise from the | ||
| # handler into a generic ValueError, collapsing the error code to 0. We therefore | ||
| # validate the id here and, only on a miss, raise McpError so it reaches the | ||
| # client intact; the actual read (and every other URI) is delegated unchanged to | ||
| # the original handler. |
Contributor
There was a problem hiding this comment.
These comments may be either summarized into one sentence or removed. It's not a vital piece of information/explanation to be kept in the codebase :D
Contributor
|
Btw, @hhuo24, you’ll need to sign the Eclipse ECA to be authorized to submit commits to the project. If you’ve already signed it with your Ericsson account, you can use those credentials. Otherwise, please sign the ECA with your Gmail account. :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: get_experiment_resource returned json.dumps({"error": ...}) for a missing experiment, so resources/read reported a successful result carrying an in-band error body. The MCP 2026-07-28 Resources spec (Error Handling) states a server MUST return JSON-RPC error -32602 for a non-existent resource.
Now raise the error if the experiment doesnt exist.
Impact: Missing experiment://{id} now returns JSON-RPC error -32602 instead of a false-success body; successful reads are unchanged.
What it does
How to test
Follow-ups
Review checklist