diff --git a/docs/migration.md b/docs/migration.md index 3b47f9aad..b5ced3a71 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -343,6 +343,10 @@ async def my_tool(x: int, ctx: Context) -> str: The internal layers (`ToolManager.call_tool`, `Tool.run`, `Prompt.render`, `ResourceTemplate.create_resource`, etc.) now require `context` as a positional argument. +### `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/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 6bf17376d..cf24ebba8 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 @@ -92,12 +93,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 2d612657c..40950637a 100644 --- a/src/mcp/server/mcpserver/resources/templates.py +++ b/src/mcp/server/mcpserver/resources/templates.py @@ -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}") diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index 2a7a58117..fc40a58fe 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, @@ -332,7 +334,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): @@ -436,10 +443,7 @@ async def read_resource( """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() diff --git a/tests/server/mcpserver/resources/test_resource_manager.py b/tests/server/mcpserver/resources/test_resource_manager.py index 724b57997..34164c46e 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 @@ -114,7 +115,7 @@ def greet(name: str) -> str: async def test_get_unknown_resource(self): """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()) def test_list_resources(self, temp_file: Path): diff --git a/tests/server/mcpserver/resources/test_resource_template.py b/tests/server/mcpserver/resources/test_resource_template.py index 640cfe803..049f43701 100644 --- a/tests/server/mcpserver/resources/test_resource_template.py +++ b/tests/server/mcpserver/resources/test_resource_template.py @@ -5,6 +5,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 @@ -86,7 +87,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 3ef06d038..39fc72a9f 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, @@ -692,13 +694,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() @@ -711,6 +716,36 @@ def failing_resource(): with pytest.raises(MCPError, match="Error reading resource resource://failing"): await client.read_resource("resource://failing") + async def test_read_resource_template_error(self): + """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(self): + """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"} + async def test_binary_resource(self): mcp = MCPServer()