diff --git a/docs/migration.md b/docs/migration.md index 8b70885e8..8b3a5030e 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -487,6 +487,10 @@ await ctx.log(level="info", data="hello") Positional calls (`await ctx.info("hello")`) are unaffected. +### `ResourceManager.get_resource()` and `ResourceTemplate.create_resource()` raise typed exceptions + +`ResourceManager.get_resource()` now raises `ResourceNotFoundError` (instead of `ValueError`) when no resource or template matches the URI. `ResourceTemplate.create_resource()` now raises `ResourceError` (instead of `ValueError`) when the template function fails. Neither subclasses `ValueError`, so callers catching `ValueError` should switch to `ResourceNotFoundError` / `ResourceError` (both importable from `mcp.server.mcpserver.exceptions`). `MCPServer.read_resource()` continues to raise `ResourceError` and is unaffected. + ### Replace `RootModel` by union types with `TypeAdapter` validation The following union types are no longer `RootModel` subclasses: diff --git a/src/mcp/server/mcpserver/context.py b/src/mcp/server/mcpserver/context.py index e87388eee..dec8268c0 100644 --- a/src/mcp/server/mcpserver/context.py +++ b/src/mcp/server/mcpserver/context.py @@ -113,6 +113,10 @@ async def read_resource(self, uri: str | AnyUrl) -> Iterable[ReadResourceContent Returns: The resource content as either text or bytes + + Raises: + ResourceNotFoundError: If no resource or template matches the URI. + ResourceError: If template creation or resource reading fails. """ assert self._mcp_server is not None, "Context is not available outside of a request" return await self._mcp_server.read_resource(uri, self) diff --git a/src/mcp/server/mcpserver/exceptions.py b/src/mcp/server/mcpserver/exceptions.py index dd1b75e82..1b93c1914 100644 --- a/src/mcp/server/mcpserver/exceptions.py +++ b/src/mcp/server/mcpserver/exceptions.py @@ -13,6 +13,15 @@ class ResourceError(MCPServerError): """Error in resource operations.""" +class ResourceNotFoundError(ResourceError): + """Resource does not exist. + + Raise this from a resource template handler to signal that the requested + instance does not exist; clients receive ``-32602`` (invalid params) per + SEP-2164. + """ + + class ToolError(MCPServerError): """Error in tool operations.""" diff --git a/src/mcp/server/mcpserver/resources/resource_manager.py b/src/mcp/server/mcpserver/resources/resource_manager.py index 766cf51ae..cff41495e 100644 --- a/src/mcp/server/mcpserver/resources/resource_manager.py +++ b/src/mcp/server/mcpserver/resources/resource_manager.py @@ -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}") def list_resources(self) -> list[Resource]: """List all registered resources.""" diff --git a/src/mcp/server/mcpserver/resources/templates.py b/src/mcp/server/mcpserver/resources/templates.py index f1ee29a37..1fcb228f5 100644 --- a/src/mcp/server/mcpserver/resources/templates.py +++ b/src/mcp/server/mcpserver/resources/templates.py @@ -11,12 +11,16 @@ import anyio.to_thread 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 +from mcp.server.mcpserver.utilities.logging import get_logger from mcp.shared._callable_inspection import is_async_callable from mcp.types import Annotations, Icon +logger = get_logger(__name__) + if TYPE_CHECKING: from mcp.server.context import LifespanContextT, RequestT from mcp.server.mcpserver.context import Context @@ -106,7 +110,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 @@ -129,5 +133,8 @@ 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}") + logger.exception("Error creating resource from template") + raise ResourceError(f"Error creating resource from template: {e}") from e diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index be77705da..85c70efe7 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -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)}) + except ResourceError as err: + raise MCPError(code=INTERNAL_ERROR, message=str(err), data={"uri": str(params.uri)}) 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. + """ 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() diff --git a/tests/server/mcpserver/resources/test_resource_manager.py b/tests/server/mcpserver/resources/test_resource_manager.py index b91c71581..bbb7de7eb 100644 --- a/tests/server/mcpserver/resources/test_resource_manager.py +++ b/tests/server/mcpserver/resources/test_resource_manager.py @@ -5,6 +5,7 @@ from pydantic import AnyUrl from mcp.server.mcpserver import Context +from mcp.server.mcpserver.exceptions import ResourceNotFoundError from mcp.server.mcpserver.resources import FileResource, FunctionResource, ResourceManager, ResourceTemplate @@ -101,7 +102,7 @@ def greet(name: str) -> str: async def test_get_unknown_resource(): """Test getting a non-existent resource.""" manager = ResourceManager() - with pytest.raises(ValueError, match="Unknown resource"): + with pytest.raises(ResourceNotFoundError, match="Unknown resource"): await manager.get_resource(AnyUrl("unknown://test"), Context()) diff --git a/tests/server/mcpserver/resources/test_resource_template.py b/tests/server/mcpserver/resources/test_resource_template.py index 2a7ba8d50..0e8121b99 100644 --- a/tests/server/mcpserver/resources/test_resource_template.py +++ b/tests/server/mcpserver/resources/test_resource_template.py @@ -6,6 +6,7 @@ from pydantic import BaseModel from mcp.server.mcpserver import Context, MCPServer +from mcp.server.mcpserver.exceptions import ResourceError from mcp.server.mcpserver.resources import FunctionResource, ResourceTemplate from mcp.types import Annotations @@ -87,7 +88,7 @@ def failing_func(x: str) -> str: name="fail", ) - with pytest.raises(ValueError, match="Error creating resource from template"): + with pytest.raises(ResourceError, match="Error creating resource from template"): await template.create_resource("fail://test", {"x": "test"}, Context()) @pytest.mark.anyio diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 3457ec944..ece83cc09 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -13,13 +13,15 @@ from mcp.server.context import ServerRequestContext from mcp.server.experimental.request_context import Experimental from mcp.server.mcpserver import Context, MCPServer -from mcp.server.mcpserver.exceptions import ToolError +from mcp.server.mcpserver.exceptions import ResourceNotFoundError, ToolError from mcp.server.mcpserver.prompts.base import Message, UserMessage from mcp.server.mcpserver.resources import FileResource, FunctionResource from mcp.server.mcpserver.utilities.types import Audio, Image from mcp.server.transport_security import TransportSecuritySettings from mcp.shared.exceptions import MCPError from mcp.types import ( + INTERNAL_ERROR, + INVALID_PARAMS, AudioContent, BlobResourceContents, Completion, @@ -727,13 +729,16 @@ def get_text(): assert result.contents[0].text == "Hello, world!" async def test_read_unknown_resource(self): - """Test that reading an unknown resource raises MCPError.""" + """Test that reading an unknown resource returns -32602 with uri in data (SEP-2164).""" mcp = MCPServer() async with Client(mcp) as client: - with pytest.raises(MCPError, match="Unknown resource: unknown://missing"): + with pytest.raises(MCPError, match="Unknown resource: unknown://missing") as exc_info: await client.read_resource("unknown://missing") + assert exc_info.value.error.code == INVALID_PARAMS + assert exc_info.value.error.data == {"uri": "unknown://missing"} + async def test_read_resource_error(self): """Test that resource read errors are properly wrapped in MCPError.""" mcp = MCPServer() @@ -1516,3 +1521,35 @@ async def test_report_progress_passes_related_request_id(): message="halfway", related_request_id="req-abc-123", ) + + +async def test_read_resource_template_error(): + """Template-creation failure must surface as INTERNAL_ERROR, not INVALID_PARAMS (not-found).""" + mcp = MCPServer() + + @mcp.resource("resource://item/{item_id}") + def get_item(item_id: str) -> str: + raise RuntimeError("backend unavailable") + + async with Client(mcp) as client: + with pytest.raises(MCPError, match="Error creating resource from template") as exc_info: + await client.read_resource("resource://item/42") + + assert exc_info.value.error.code == INTERNAL_ERROR + assert exc_info.value.error.code != INVALID_PARAMS + + +async def test_read_resource_template_not_found(): + """A template handler raising ResourceNotFoundError must surface as INVALID_PARAMS per SEP-2164.""" + mcp = MCPServer() + + @mcp.resource("resource://users/{user_id}") + def get_user(user_id: str) -> str: + raise ResourceNotFoundError(f"no user {user_id}") + + async with Client(mcp) as client: + with pytest.raises(MCPError, match="no user 999") as exc_info: + await client.read_resource("resource://users/999") + + assert exc_info.value.error.code == INVALID_PARAMS + assert exc_info.value.error.data == {"uri": "resource://users/999"}