Skip to content

Commit 14744c6

Browse files
authored
Add tests for MCPServer.read_resource exception handling (#1957)
1 parent 6f0e8e7 commit 14744c6

File tree

6 files changed

+165
-254
lines changed

6 files changed

+165
-254
lines changed

src/mcp/server/experimental/request_context.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ def validate_task_mode(
9191
error = ErrorData(code=METHOD_NOT_FOUND, message="This tool does not support task-augmented invocation")
9292

9393
if error is not None and raise_error:
94-
raise MCPError(code=error.code, message=error.message)
94+
raise MCPError.from_error_data(error)
9595

9696
return error
9797

src/mcp/server/lowlevel/server.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -795,12 +795,7 @@ async def _handle_request(
795795

796796
await message.respond(response)
797797
else: # pragma: no cover
798-
await message.respond(
799-
types.ErrorData(
800-
code=types.METHOD_NOT_FOUND,
801-
message="Method not found",
802-
)
803-
)
798+
await message.respond(types.ErrorData(code=types.METHOD_NOT_FOUND, message="Method not found"))
804799

805800
logger.debug("Response sent")
806801

src/mcp/server/mcpserver/resources/resource_manager.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,8 @@ def add_template(
8282
return template
8383

8484
async def get_resource(
85-
self,
86-
uri: AnyUrl | str,
87-
context: Context[ServerSessionT, LifespanContextT, RequestT] | None = None,
88-
) -> Resource | None:
85+
self, uri: AnyUrl | str, context: Context[ServerSessionT, LifespanContextT, RequestT] | None = None
86+
) -> Resource:
8987
"""Get resource by URI, checking concrete resources first, then templates."""
9088
uri_str = str(uri)
9189
logger.debug("Getting resource", extra={"uri": uri_str})

src/mcp/server/mcpserver/server.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -347,16 +347,18 @@ async def read_resource(self, uri: AnyUrl | str) -> Iterable[ReadResourceContent
347347
"""Read a resource by URI."""
348348

349349
context = self.get_context()
350-
resource = await self._resource_manager.get_resource(uri, context=context)
351-
if not resource: # pragma: no cover
350+
try:
351+
resource = await self._resource_manager.get_resource(uri, context=context)
352+
except ValueError:
352353
raise ResourceError(f"Unknown resource: {uri}")
353354

354355
try:
355356
content = await resource.read()
356357
return [ReadResourceContents(content=content, mime_type=resource.mime_type, meta=resource.meta)]
357-
except Exception as e: # pragma: no cover
358-
logger.exception(f"Error reading resource {uri}")
359-
raise ResourceError(str(e))
358+
except Exception as exc:
359+
logger.exception(f"Error getting resource {uri}")
360+
# If an exception happens when reading the resource, we should not leak the exception to the client.
361+
raise ResourceError(f"Error reading resource {uri}") from exc
360362

361363
def add_tool(
362364
self,

tests/issues/test_141_resource_templates.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from mcp import Client
44
from mcp.server.mcpserver import MCPServer
5+
from mcp.server.mcpserver.exceptions import ResourceError
56
from mcp.types import (
67
ListResourceTemplatesResult,
78
TextResourceContents,
@@ -54,10 +55,10 @@ def get_user_profile_missing(user_id: str) -> str: # pragma: no cover
5455
assert result_list[0].mime_type == "text/plain"
5556

5657
# Verify invalid parameters raise error
57-
with pytest.raises(ValueError, match="Unknown resource"):
58+
with pytest.raises(ResourceError, match="Unknown resource"):
5859
await mcp.read_resource("resource://users/123/posts") # Missing post_id
5960

60-
with pytest.raises(ValueError, match="Unknown resource"):
61+
with pytest.raises(ResourceError, match="Unknown resource"):
6162
await mcp.read_resource("resource://users/123/posts/456/extra") # Extra path component
6263

6364

0 commit comments

Comments
 (0)