-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(server): return -32602 for resource not found (SEP-2164) #2344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d065e9b
d4d62fb
cb93651
4784bfa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
|
|
||
| from pydantic import BaseModel, Field, validate_call | ||
|
|
||
| from mcp.server.mcpserver.exceptions import ResourceError | ||
| from mcp.server.mcpserver.resources.types import FunctionResource, Resource | ||
| from mcp.server.mcpserver.utilities.context_injection import find_context_parameter, inject_context | ||
| from mcp.server.mcpserver.utilities.func_metadata import func_metadata | ||
|
|
@@ -104,7 +105,7 @@ async def create_resource( | |
| """Create a resource from the template with the given parameters. | ||
|
|
||
| Raises: | ||
| ValueError: If creating the resource fails. | ||
| ResourceError: If creating the resource fails. | ||
| """ | ||
| try: | ||
| # Add context to params if needed | ||
|
|
@@ -126,5 +127,7 @@ async def create_resource( | |
| meta=self.meta, | ||
| fn=lambda: result, # Capture result in closure | ||
| ) | ||
| except ResourceError: | ||
| raise | ||
| except Exception as e: | ||
| raise ValueError(f"Error creating resource from template: {e}") | ||
| raise ResourceError(f"Error creating resource from template: {e}") | ||
|
claude[bot] marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We keep leaking info when adding the exception in the message itself. We should drop it out. This comment is not related to this PR. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,7 @@ | |
| from mcp.server.lowlevel.server import LifespanResultT, Server | ||
| from mcp.server.lowlevel.server import lifespan as default_lifespan | ||
| from mcp.server.mcpserver.context import Context | ||
| from mcp.server.mcpserver.exceptions import ResourceError | ||
| from mcp.server.mcpserver.exceptions import ResourceError, ResourceNotFoundError | ||
| from mcp.server.mcpserver.prompts import Prompt, PromptManager | ||
| from mcp.server.mcpserver.resources import FunctionResource, Resource, ResourceManager | ||
| from mcp.server.mcpserver.tools import Tool, ToolManager | ||
|
|
@@ -44,6 +44,8 @@ | |
| from mcp.server.transport_security import TransportSecuritySettings | ||
| from mcp.shared.exceptions import MCPError | ||
| from mcp.types import ( | ||
| INTERNAL_ERROR, | ||
| INVALID_PARAMS, | ||
| Annotations, | ||
| BlobResourceContents, | ||
| CallToolRequestParams, | ||
|
|
@@ -332,7 +334,12 @@ | |
| self, ctx: ServerRequestContext[LifespanResultT], params: ReadResourceRequestParams | ||
| ) -> ReadResourceResult: | ||
| context = Context(request_context=ctx, mcp_server=self) | ||
| results = await self.read_resource(params.uri, context) | ||
| try: | ||
| results = await self.read_resource(params.uri, context) | ||
| except ResourceNotFoundError as err: | ||
| raise MCPError(code=INVALID_PARAMS, message=str(err), data={"uri": str(params.uri)}) | ||
|
Check warning on line 340 in src/mcp/server/mcpserver/server.py
|
||
|
Comment on lines
+339
to
+340
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Edge case: if a template handler composes via Extended reasoning...What the bug isWhen an async resource-template handler internally calls Code path that triggers it
When that exception exits the handler's Why nothing prevents itBefore this PR, the inner failure was double-wrapped (templates.py Step-by-step proof
Client receives Impact and suggested fixNiche: only affects async template handlers that compose resources via |
||
| except ResourceError as err: | ||
| raise MCPError(code=INTERNAL_ERROR, message=str(err), data={"uri": str(params.uri)}) | ||
|
Comment on lines
+341
to
+342
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Minor: template-function exception text now reaches the client ( Extended reasoning...What changedBefore this PR, a template-function failure was (accidentally) sanitized: Step-by-step trace
Previously step 5 would have caught The inconsistencyWithin the same Addressing the counter-argumentIt's fair to note that (a) the previous sanitization was an accidental side-effect of the very bug this PR fixes, not a designed feature; (b) Suggested fixIf sanitization is preferred (matching the existing comment): in templates.py:133, raise
Comment on lines
+340
to
+342
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we have something like MCPError.from something?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only for inbound |
||
| contents: list[TextResourceContents | BlobResourceContents] = [] | ||
| for item in results: | ||
| if isinstance(item.content, bytes): | ||
|
|
@@ -436,10 +443,7 @@ | |
| """Read a resource by URI.""" | ||
| if context is None: | ||
| context = Context(mcp_server=self) | ||
| try: | ||
| resource = await self._resource_manager.get_resource(uri, context) | ||
| except ValueError: | ||
| raise ResourceError(f"Unknown resource: {uri}") | ||
| resource = await self._resource_manager.get_resource(uri, context) | ||
|
|
||
| try: | ||
| content = await resource.read() | ||
|
claude[bot] marked this conversation as resolved.
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.