Spaces:
Running
Running
Jeremiah Lowin commited on
Commit ·
678e6ab
1
Parent(s): 2c75133
Handle template errors
Browse files- src/fastmcp/resources/resource_manager.py +4 -0
- src/fastmcp/resources/template.py +16 -17
- src/fastmcp/resources/types.py +17 -24
- tests/client/test_client.py +30 -0
- tests/resources/test_function_resources.py +1 -1
- tests/resources/test_resource_manager.py +2 -28
- tests/resources/test_resource_template.py +0 -15
src/fastmcp/resources/resource_manager.py
CHANGED
|
@@ -244,7 +244,11 @@ class ResourceManager:
|
|
| 244 |
uri_str,
|
| 245 |
params=params,
|
| 246 |
)
|
|
|
|
|
|
|
|
|
|
| 247 |
except Exception as e:
|
|
|
|
| 248 |
raise ValueError(f"Error creating resource from template: {e}")
|
| 249 |
|
| 250 |
raise NotFoundError(f"Unknown resource: {uri_str}")
|
|
|
|
| 244 |
uri_str,
|
| 245 |
params=params,
|
| 246 |
)
|
| 247 |
+
except ResourceError as e:
|
| 248 |
+
logger.error(f"Error creating resource from template: {e}")
|
| 249 |
+
raise e
|
| 250 |
except Exception as e:
|
| 251 |
+
logger.error(f"Error creating resource from template: {e}")
|
| 252 |
raise ValueError(f"Error creating resource from template: {e}")
|
| 253 |
|
| 254 |
raise NotFoundError(f"Unknown resource: {uri_str}")
|
src/fastmcp/resources/template.py
CHANGED
|
@@ -171,28 +171,27 @@ class ResourceTemplate(BaseModel):
|
|
| 171 |
"""Create a resource from the template with the given parameters."""
|
| 172 |
from fastmcp.server.context import Context
|
| 173 |
|
| 174 |
-
|
| 175 |
-
|
| 176 |
-
|
| 177 |
-
|
| 178 |
-
|
| 179 |
-
kwargs[context_kwarg] = get_context()
|
| 180 |
|
|
|
|
| 181 |
# Call function and check if result is a coroutine
|
| 182 |
result = self.fn(**kwargs)
|
| 183 |
if inspect.iscoroutine(result):
|
| 184 |
result = await result
|
| 185 |
-
|
| 186 |
-
|
| 187 |
-
|
| 188 |
-
|
| 189 |
-
|
| 190 |
-
|
| 191 |
-
|
| 192 |
-
|
| 193 |
-
|
| 194 |
-
|
| 195 |
-
raise ValueError(f"Error creating resource from template: {e}")
|
| 196 |
|
| 197 |
def __eq__(self, other: object) -> bool:
|
| 198 |
if not isinstance(other, ResourceTemplate):
|
|
|
|
| 171 |
"""Create a resource from the template with the given parameters."""
|
| 172 |
from fastmcp.server.context import Context
|
| 173 |
|
| 174 |
+
# Add context to parameters if needed
|
| 175 |
+
kwargs = params.copy()
|
| 176 |
+
context_kwarg = find_kwarg_by_type(self.fn, kwarg_type=Context)
|
| 177 |
+
if context_kwarg and context_kwarg not in kwargs:
|
| 178 |
+
kwargs[context_kwarg] = get_context()
|
|
|
|
| 179 |
|
| 180 |
+
async def resource_read_fn() -> str | bytes:
|
| 181 |
# Call function and check if result is a coroutine
|
| 182 |
result = self.fn(**kwargs)
|
| 183 |
if inspect.iscoroutine(result):
|
| 184 |
result = await result
|
| 185 |
+
return result
|
| 186 |
+
|
| 187 |
+
return FunctionResource(
|
| 188 |
+
uri=AnyUrl(uri), # Explicitly convert to AnyUrl
|
| 189 |
+
name=self.name,
|
| 190 |
+
description=self.description,
|
| 191 |
+
mime_type=self.mime_type,
|
| 192 |
+
fn=resource_read_fn,
|
| 193 |
+
tags=self.tags,
|
| 194 |
+
)
|
|
|
|
| 195 |
|
| 196 |
def __eq__(self, other: object) -> bool:
|
| 197 |
if not isinstance(other, ResourceTemplate):
|
src/fastmcp/resources/types.py
CHANGED
|
@@ -63,30 +63,23 @@ class FunctionResource(Resource):
|
|
| 63 |
"""Read the resource by calling the wrapped function."""
|
| 64 |
from fastmcp.server.context import Context
|
| 65 |
|
| 66 |
-
|
| 67 |
-
|
| 68 |
-
|
| 69 |
-
|
| 70 |
-
|
| 71 |
-
|
| 72 |
-
|
| 73 |
-
|
| 74 |
-
|
| 75 |
-
|
| 76 |
-
|
| 77 |
-
|
| 78 |
-
|
| 79 |
-
|
| 80 |
-
|
| 81 |
-
|
| 82 |
-
|
| 83 |
-
return pydantic_core.to_json(result, fallback=str, indent=2).decode()
|
| 84 |
-
except ResourceError as e:
|
| 85 |
-
logger.exception(f"Error reading resource {self.uri}: {e}")
|
| 86 |
-
raise e
|
| 87 |
-
except Exception as e:
|
| 88 |
-
logger.exception(f"Error reading resource {self.uri}: {e}")
|
| 89 |
-
raise ValueError(f"Error reading resource {self.uri}.") from e
|
| 90 |
|
| 91 |
|
| 92 |
class FileResource(Resource):
|
|
|
|
| 63 |
"""Read the resource by calling the wrapped function."""
|
| 64 |
from fastmcp.server.context import Context
|
| 65 |
|
| 66 |
+
kwargs = {}
|
| 67 |
+
context_kwarg = find_kwarg_by_type(self.fn, kwarg_type=Context)
|
| 68 |
+
if context_kwarg is not None:
|
| 69 |
+
kwargs[context_kwarg] = get_context()
|
| 70 |
+
|
| 71 |
+
result = self.fn(**kwargs)
|
| 72 |
+
if inspect.iscoroutinefunction(self.fn):
|
| 73 |
+
result = await result
|
| 74 |
+
|
| 75 |
+
if isinstance(result, Resource):
|
| 76 |
+
return await result.read()
|
| 77 |
+
elif isinstance(result, bytes):
|
| 78 |
+
return result
|
| 79 |
+
elif isinstance(result, str):
|
| 80 |
+
return result
|
| 81 |
+
else:
|
| 82 |
+
return pydantic_core.to_json(result, fallback=str, indent=2).decode()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 83 |
|
| 84 |
|
| 85 |
class FileResource(Resource):
|
tests/client/test_client.py
CHANGED
|
@@ -469,3 +469,33 @@ class TestErrorHandling:
|
|
| 469 |
with pytest.raises(Exception) as excinfo:
|
| 470 |
await client.read_resource(AnyUrl("error://resource"))
|
| 471 |
assert "This is a resource error (xyz)" in str(excinfo.value)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 469 |
with pytest.raises(Exception) as excinfo:
|
| 470 |
await client.read_resource(AnyUrl("error://resource"))
|
| 471 |
assert "This is a resource error (xyz)" in str(excinfo.value)
|
| 472 |
+
|
| 473 |
+
async def test_general_template_exceptions_are_masked(self):
|
| 474 |
+
mcp = FastMCP("TestServer")
|
| 475 |
+
|
| 476 |
+
@mcp.resource(uri="exception://resource/{id}")
|
| 477 |
+
async def exception_resource(id: str):
|
| 478 |
+
raise ValueError("This is an internal error (sensitive)")
|
| 479 |
+
|
| 480 |
+
client = Client(transport=FastMCPTransport(mcp))
|
| 481 |
+
|
| 482 |
+
async with client:
|
| 483 |
+
with pytest.raises(Exception) as excinfo:
|
| 484 |
+
await client.read_resource(AnyUrl("exception://resource/123"))
|
| 485 |
+
assert "Error reading resource" in str(excinfo.value)
|
| 486 |
+
assert "sensitive" not in str(excinfo.value)
|
| 487 |
+
assert "internal error" not in str(excinfo.value)
|
| 488 |
+
|
| 489 |
+
async def test_template_errors_are_sent_to_client(self):
|
| 490 |
+
mcp = FastMCP("TestServer")
|
| 491 |
+
|
| 492 |
+
@mcp.resource(uri="error://resource/{id}")
|
| 493 |
+
async def error_resource(id: str):
|
| 494 |
+
raise ResourceError("This is a resource error (xyz)")
|
| 495 |
+
|
| 496 |
+
client = Client(transport=FastMCPTransport(mcp))
|
| 497 |
+
|
| 498 |
+
async with client:
|
| 499 |
+
with pytest.raises(Exception) as excinfo:
|
| 500 |
+
await client.read_resource(AnyUrl("error://resource/123"))
|
| 501 |
+
assert "This is a resource error (xyz)" in str(excinfo.value)
|
tests/resources/test_function_resources.py
CHANGED
|
@@ -80,7 +80,7 @@ class TestFunctionResource:
|
|
| 80 |
name="test",
|
| 81 |
fn=failing_func,
|
| 82 |
)
|
| 83 |
-
with pytest.raises(ValueError, match="
|
| 84 |
await resource.read()
|
| 85 |
|
| 86 |
async def test_basemodel_conversion(self):
|
|
|
|
| 80 |
name="test",
|
| 81 |
fn=failing_func,
|
| 82 |
)
|
| 83 |
+
with pytest.raises(ValueError, match="Test error"):
|
| 84 |
await resource.read()
|
| 85 |
|
| 86 |
async def test_basemodel_conversion(self):
|
tests/resources/test_resource_manager.py
CHANGED
|
@@ -600,8 +600,7 @@ class TestResourceErrorHandling:
|
|
| 600 |
)
|
| 601 |
manager.add_template(template)
|
| 602 |
|
| 603 |
-
|
| 604 |
-
with pytest.raises(ValueError) as excinfo:
|
| 605 |
await manager.read_resource("error://test")
|
| 606 |
|
| 607 |
# The original error message should be included in the ValueError
|
|
@@ -623,30 +622,5 @@ class TestResourceErrorHandling:
|
|
| 623 |
manager.add_template(template)
|
| 624 |
|
| 625 |
# First, the template creation will fail with ValueError
|
| 626 |
-
with pytest.raises(
|
| 627 |
await manager.read_resource("buggy://test")
|
| 628 |
-
|
| 629 |
-
# Let's test with a template that returns a resource that fails
|
| 630 |
-
def create_failing_resource(param: str):
|
| 631 |
-
async def failing_resource():
|
| 632 |
-
raise ValueError(f"Resource from template fails with {param}")
|
| 633 |
-
|
| 634 |
-
return FunctionResource(
|
| 635 |
-
uri=AnyUrl(f"failing://{param}"),
|
| 636 |
-
name=f"failing_{param}",
|
| 637 |
-
fn=failing_resource,
|
| 638 |
-
)
|
| 639 |
-
|
| 640 |
-
template = ResourceTemplate.from_function(
|
| 641 |
-
fn=create_failing_resource,
|
| 642 |
-
uri_template="failing://{param}",
|
| 643 |
-
name="failing_template",
|
| 644 |
-
)
|
| 645 |
-
manager.add_template(template)
|
| 646 |
-
|
| 647 |
-
with pytest.raises(ResourceError) as excinfo:
|
| 648 |
-
await manager.read_resource("failing://test")
|
| 649 |
-
|
| 650 |
-
# Exception should contain resource URI but not internal details
|
| 651 |
-
assert "Error reading resource 'failing://test'" in str(excinfo.value)
|
| 652 |
-
assert "Resource from template fails with test" not in str(excinfo.value)
|
|
|
|
| 600 |
)
|
| 601 |
manager.add_template(template)
|
| 602 |
|
| 603 |
+
with pytest.raises(ResourceError) as excinfo:
|
|
|
|
| 604 |
await manager.read_resource("error://test")
|
| 605 |
|
| 606 |
# The original error message should be included in the ValueError
|
|
|
|
| 622 |
manager.add_template(template)
|
| 623 |
|
| 624 |
# First, the template creation will fail with ValueError
|
| 625 |
+
with pytest.raises(ResourceError, match="Error reading resource"):
|
| 626 |
await manager.read_resource("buggy://test")
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
tests/resources/test_resource_template.py
CHANGED
|
@@ -186,21 +186,6 @@ class TestResourceTemplate:
|
|
| 186 |
data = json.loads(content)
|
| 187 |
assert data == {"key": "foo", "value": 123}
|
| 188 |
|
| 189 |
-
async def test_template_error(self):
|
| 190 |
-
"""Test error handling in template resource creation."""
|
| 191 |
-
|
| 192 |
-
def failing_func(x: str) -> str:
|
| 193 |
-
raise ValueError("Test error")
|
| 194 |
-
|
| 195 |
-
template = ResourceTemplate.from_function(
|
| 196 |
-
fn=failing_func,
|
| 197 |
-
uri_template="fail://{x}",
|
| 198 |
-
name="fail",
|
| 199 |
-
)
|
| 200 |
-
|
| 201 |
-
with pytest.raises(ValueError, match="Error creating resource from template"):
|
| 202 |
-
await template.create_resource("fail://test", {"x": "test"})
|
| 203 |
-
|
| 204 |
async def test_async_text_resource(self):
|
| 205 |
"""Test creating a text resource from async function."""
|
| 206 |
|
|
|
|
| 186 |
data = json.loads(content)
|
| 187 |
assert data == {"key": "foo", "value": 123}
|
| 188 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 189 |
async def test_async_text_resource(self):
|
| 190 |
"""Test creating a text resource from async function."""
|
| 191 |
|