mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-23 14:04:18 +02:00
Handle template errors
This commit is contained in:
parent
6fa7c1704a
commit
e7e301815f
7 changed files with 70 additions and 85 deletions
|
|
@ -244,7 +244,11 @@ class ResourceManager:
|
|||
uri_str,
|
||||
params=params,
|
||||
)
|
||||
except ResourceError as e:
|
||||
logger.error(f"Error creating resource from template: {e}")
|
||||
raise e
|
||||
except Exception as e:
|
||||
logger.error(f"Error creating resource from template: {e}")
|
||||
raise ValueError(f"Error creating resource from template: {e}")
|
||||
|
||||
raise NotFoundError(f"Unknown resource: {uri_str}")
|
||||
|
|
|
|||
|
|
@ -171,28 +171,27 @@ class ResourceTemplate(BaseModel):
|
|||
"""Create a resource from the template with the given parameters."""
|
||||
from fastmcp.server.context import Context
|
||||
|
||||
try:
|
||||
# Add context to parameters if needed
|
||||
kwargs = params.copy()
|
||||
context_kwarg = find_kwarg_by_type(self.fn, kwarg_type=Context)
|
||||
if context_kwarg and context_kwarg not in kwargs:
|
||||
kwargs[context_kwarg] = get_context()
|
||||
# Add context to parameters if needed
|
||||
kwargs = params.copy()
|
||||
context_kwarg = find_kwarg_by_type(self.fn, kwarg_type=Context)
|
||||
if context_kwarg and context_kwarg not in kwargs:
|
||||
kwargs[context_kwarg] = get_context()
|
||||
|
||||
async def resource_read_fn() -> str | bytes:
|
||||
# Call function and check if result is a coroutine
|
||||
result = self.fn(**kwargs)
|
||||
if inspect.iscoroutine(result):
|
||||
result = await result
|
||||
return result
|
||||
|
||||
return FunctionResource(
|
||||
uri=AnyUrl(uri), # Explicitly convert to AnyUrl
|
||||
name=self.name,
|
||||
description=self.description,
|
||||
mime_type=self.mime_type,
|
||||
fn=lambda **kwargs: result, # Capture result in closure
|
||||
tags=self.tags,
|
||||
)
|
||||
except Exception as e:
|
||||
raise ValueError(f"Error creating resource from template: {e}")
|
||||
return FunctionResource(
|
||||
uri=AnyUrl(uri), # Explicitly convert to AnyUrl
|
||||
name=self.name,
|
||||
description=self.description,
|
||||
mime_type=self.mime_type,
|
||||
fn=resource_read_fn,
|
||||
tags=self.tags,
|
||||
)
|
||||
|
||||
def __eq__(self, other: object) -> bool:
|
||||
if not isinstance(other, ResourceTemplate):
|
||||
|
|
|
|||
|
|
@ -63,30 +63,23 @@ class FunctionResource(Resource):
|
|||
"""Read the resource by calling the wrapped function."""
|
||||
from fastmcp.server.context import Context
|
||||
|
||||
try:
|
||||
kwargs = {}
|
||||
context_kwarg = find_kwarg_by_type(self.fn, kwarg_type=Context)
|
||||
if context_kwarg is not None:
|
||||
kwargs[context_kwarg] = get_context()
|
||||
kwargs = {}
|
||||
context_kwarg = find_kwarg_by_type(self.fn, kwarg_type=Context)
|
||||
if context_kwarg is not None:
|
||||
kwargs[context_kwarg] = get_context()
|
||||
|
||||
result = self.fn(**kwargs)
|
||||
if inspect.iscoroutinefunction(self.fn):
|
||||
result = await result
|
||||
result = self.fn(**kwargs)
|
||||
if inspect.iscoroutinefunction(self.fn):
|
||||
result = await result
|
||||
|
||||
if isinstance(result, Resource):
|
||||
return await result.read()
|
||||
elif isinstance(result, bytes):
|
||||
return result
|
||||
elif isinstance(result, str):
|
||||
return result
|
||||
else:
|
||||
return pydantic_core.to_json(result, fallback=str, indent=2).decode()
|
||||
except ResourceError as e:
|
||||
logger.exception(f"Error reading resource {self.uri}: {e}")
|
||||
raise e
|
||||
except Exception as e:
|
||||
logger.exception(f"Error reading resource {self.uri}: {e}")
|
||||
raise ValueError(f"Error reading resource {self.uri}.") from e
|
||||
if isinstance(result, Resource):
|
||||
return await result.read()
|
||||
elif isinstance(result, bytes):
|
||||
return result
|
||||
elif isinstance(result, str):
|
||||
return result
|
||||
else:
|
||||
return pydantic_core.to_json(result, fallback=str, indent=2).decode()
|
||||
|
||||
|
||||
class FileResource(Resource):
|
||||
|
|
|
|||
|
|
@ -469,3 +469,33 @@ class TestErrorHandling:
|
|||
with pytest.raises(Exception) as excinfo:
|
||||
await client.read_resource(AnyUrl("error://resource"))
|
||||
assert "This is a resource error (xyz)" in str(excinfo.value)
|
||||
|
||||
async def test_general_template_exceptions_are_masked(self):
|
||||
mcp = FastMCP("TestServer")
|
||||
|
||||
@mcp.resource(uri="exception://resource/{id}")
|
||||
async def exception_resource(id: str):
|
||||
raise ValueError("This is an internal error (sensitive)")
|
||||
|
||||
client = Client(transport=FastMCPTransport(mcp))
|
||||
|
||||
async with client:
|
||||
with pytest.raises(Exception) as excinfo:
|
||||
await client.read_resource(AnyUrl("exception://resource/123"))
|
||||
assert "Error reading resource" in str(excinfo.value)
|
||||
assert "sensitive" not in str(excinfo.value)
|
||||
assert "internal error" not in str(excinfo.value)
|
||||
|
||||
async def test_template_errors_are_sent_to_client(self):
|
||||
mcp = FastMCP("TestServer")
|
||||
|
||||
@mcp.resource(uri="error://resource/{id}")
|
||||
async def error_resource(id: str):
|
||||
raise ResourceError("This is a resource error (xyz)")
|
||||
|
||||
client = Client(transport=FastMCPTransport(mcp))
|
||||
|
||||
async with client:
|
||||
with pytest.raises(Exception) as excinfo:
|
||||
await client.read_resource(AnyUrl("error://resource/123"))
|
||||
assert "This is a resource error (xyz)" in str(excinfo.value)
|
||||
|
|
|
|||
|
|
@ -80,7 +80,7 @@ class TestFunctionResource:
|
|||
name="test",
|
||||
fn=failing_func,
|
||||
)
|
||||
with pytest.raises(ValueError, match="Error reading resource function://test"):
|
||||
with pytest.raises(ValueError, match="Test error"):
|
||||
await resource.read()
|
||||
|
||||
async def test_basemodel_conversion(self):
|
||||
|
|
|
|||
|
|
@ -600,8 +600,7 @@ class TestResourceErrorHandling:
|
|||
)
|
||||
manager.add_template(template)
|
||||
|
||||
# ResourceErrors in templates are wrapped in ValueError
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
with pytest.raises(ResourceError) as excinfo:
|
||||
await manager.read_resource("error://test")
|
||||
|
||||
# The original error message should be included in the ValueError
|
||||
|
|
@ -623,30 +622,5 @@ class TestResourceErrorHandling:
|
|||
manager.add_template(template)
|
||||
|
||||
# First, the template creation will fail with ValueError
|
||||
with pytest.raises(ValueError):
|
||||
with pytest.raises(ResourceError, match="Error reading resource"):
|
||||
await manager.read_resource("buggy://test")
|
||||
|
||||
# Let's test with a template that returns a resource that fails
|
||||
def create_failing_resource(param: str):
|
||||
async def failing_resource():
|
||||
raise ValueError(f"Resource from template fails with {param}")
|
||||
|
||||
return FunctionResource(
|
||||
uri=AnyUrl(f"failing://{param}"),
|
||||
name=f"failing_{param}",
|
||||
fn=failing_resource,
|
||||
)
|
||||
|
||||
template = ResourceTemplate.from_function(
|
||||
fn=create_failing_resource,
|
||||
uri_template="failing://{param}",
|
||||
name="failing_template",
|
||||
)
|
||||
manager.add_template(template)
|
||||
|
||||
with pytest.raises(ResourceError) as excinfo:
|
||||
await manager.read_resource("failing://test")
|
||||
|
||||
# Exception should contain resource URI but not internal details
|
||||
assert "Error reading resource 'failing://test'" in str(excinfo.value)
|
||||
assert "Resource from template fails with test" not in str(excinfo.value)
|
||||
|
|
|
|||
|
|
@ -186,21 +186,6 @@ class TestResourceTemplate:
|
|||
data = json.loads(content)
|
||||
assert data == {"key": "foo", "value": 123}
|
||||
|
||||
async def test_template_error(self):
|
||||
"""Test error handling in template resource creation."""
|
||||
|
||||
def failing_func(x: str) -> str:
|
||||
raise ValueError("Test error")
|
||||
|
||||
template = ResourceTemplate.from_function(
|
||||
fn=failing_func,
|
||||
uri_template="fail://{x}",
|
||||
name="fail",
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match="Error creating resource from template"):
|
||||
await template.create_resource("fail://test", {"x": "test"})
|
||||
|
||||
async def test_async_text_resource(self):
|
||||
"""Test creating a text resource from async function."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue