-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
3007354
bcbb7ac
36e5268
0172696
6bc310e
8ded8de
fc40588
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 |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -341,7 +343,12 @@ async def _handle_read_resource( | |
| 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)}) | ||
|
Comment on lines
+348
to
+349
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
+350
to
+351
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
+349
to
+351
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
claude[bot] marked this conversation as resolved.
|
||
| contents: list[TextResourceContents | BlobResourceContents] = [] | ||
| for item in results: | ||
| if isinstance(item.content, bytes): | ||
|
|
@@ -442,13 +449,15 @@ async def list_resource_templates(self) -> list[MCPResourceTemplate]: | |
| async def read_resource( | ||
| self, uri: AnyUrl | str, context: Context[LifespanResultT, Any] | None = None | ||
| ) -> Iterable[ReadResourceContents]: | ||
| """Read a resource by URI.""" | ||
| """Read a resource by URI. | ||
|
|
||
| Raises: | ||
| ResourceNotFoundError: If no resource or template matches the URI. | ||
| ResourceError: If template creation or resource reading fails. | ||
| """ | ||
|
claude[bot] marked this conversation as resolved.
|
||
| 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.