Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
3007354bcbb7ac36e526801726966bc310e8ded8defc40588File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -7,6 +7,7 @@ | ||
| from pydantic import AnyUrl | ||
| from mcp.server.mcpserver.exceptions import ResourceNotFoundError | ||
| from mcp.server.mcpserver.resources.base import Resource | ||
| from mcp.server.mcpserver.resources.templates import ResourceTemplate | ||
| from mcp.server.mcpserver.utilities.logging import get_logger | ||
| @@ -79,7 +80,12 @@ def add_template( | ||
| return template | ||
| async def get_resource(self, uri: AnyUrl | str, context: Context[LifespanContextT, RequestT]) -> Resource: | ||
| """Get resource by URI, checking concrete resources first, then templates.""" | ||
| """Get resource by URI, checking concrete resources first, then templates. | ||
| Raises: | ||
| ResourceNotFoundError: If no resource or template matches the URI. | ||
| ResourceError: If a matching template fails to create the resource. | ||
| """ | ||
| uri_str = str(uri) | ||
| logger.debug("Getting resource", extra={"uri": uri_str}) | ||
| @@ -90,12 +96,9 @@ async def get_resource(self, uri: AnyUrl | str, context: Context[LifespanContext | ||
| # Then check templates | ||
| for template in self._templates.values(): | ||
| if params := template.matches(uri_str): | ||
| try: | ||
| return await template.create_resource(uri_str, params, context=context) | ||
| except Exception as e: # pragma: no cover | ||
| raise ValueError(f"Error creating resource from template: {e}") | ||
| return await template.create_resource(uri_str, params, context=context) | ||
| raise ValueError(f"Unknown resource: {uri}") | ||
| raise ResourceNotFoundError(f"Unknown resource: {uri}") | ||
claude[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| def list_resources(self) -> list[Resource]: | ||
| """List all registered resources.""" | ||
| 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? ContributorAuthor 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.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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.
Uh oh!There was an error while loading. Please reload this page. | ||
| 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. | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.