mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-21 21:14:17 +02:00
Let ToolResult return an error result via is_error (#4217)
This commit is contained in:
parent
8382569027
commit
e242abee7d
6 changed files with 104 additions and 11 deletions
|
|
@ -82,6 +82,7 @@ class CachableToolResult(FastMCPBaseModel):
|
|||
content: list[mcp.types.ContentBlock]
|
||||
structured_content: dict[str, Any] | None
|
||||
meta: dict[str, Any] | None
|
||||
is_error: bool = False
|
||||
|
||||
@classmethod
|
||||
def wrap(cls, value: ToolResult) -> Self:
|
||||
|
|
@ -89,6 +90,7 @@ class CachableToolResult(FastMCPBaseModel):
|
|||
content=value.content,
|
||||
structured_content=value.structured_content,
|
||||
meta=value.meta,
|
||||
is_error=value.is_error,
|
||||
)
|
||||
|
||||
def unwrap(self) -> ToolResult:
|
||||
|
|
@ -96,6 +98,7 @@ class CachableToolResult(FastMCPBaseModel):
|
|||
content=self.content,
|
||||
structured_content=self.structured_content,
|
||||
meta=self.meta,
|
||||
is_error=self.is_error,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ from fastmcp.client.logging import LogMessage
|
|||
from fastmcp.client.roots import RootsList
|
||||
from fastmcp.client.telemetry import client_span
|
||||
from fastmcp.client.transports import ClientTransportT
|
||||
from fastmcp.exceptions import ResourceError, ToolError
|
||||
from fastmcp.exceptions import ResourceError
|
||||
from fastmcp.mcp_config import MCPConfig
|
||||
from fastmcp.prompts import Message, Prompt, PromptResult
|
||||
from fastmcp.prompts.base import PromptArgument
|
||||
|
|
@ -160,19 +160,16 @@ class ProxyTool(Tool):
|
|||
result = await client.call_tool_mcp(
|
||||
name=backend_name, arguments=arguments, meta=meta
|
||||
)
|
||||
if result.isError:
|
||||
first = result.content[0] if result.content else None
|
||||
if isinstance(first, mcp.types.TextContent):
|
||||
raise ToolError(first.text)
|
||||
elif first is None:
|
||||
raise ToolError("Tool returned an error with no content")
|
||||
else:
|
||||
raise ToolError(f"Tool returned an error ({type(first).__name__})")
|
||||
# Pass an upstream error result through faithfully rather than
|
||||
# collapsing it into a raised ToolError — this preserves the
|
||||
# backend's content (including non-text and structured content),
|
||||
# and the client still raises on isError by default.
|
||||
# Preserve backend's meta (includes task metadata for background tasks)
|
||||
return ToolResult(
|
||||
content=result.content,
|
||||
structured_content=result.structuredContent,
|
||||
meta=result.meta,
|
||||
is_error=result.isError,
|
||||
)
|
||||
|
||||
def get_span_attributes(self) -> dict[str, Any]:
|
||||
|
|
|
|||
|
|
@ -76,12 +76,19 @@ class ToolResult(BaseModel):
|
|||
meta: dict[str, Any] | None = Field(
|
||||
default=None, description="Runtime metadata about the tool execution"
|
||||
)
|
||||
is_error: bool = Field(
|
||||
default=False,
|
||||
description="Whether this result represents a tool execution error. "
|
||||
"When True, it maps to CallToolResult.isError so the error is returned "
|
||||
"to the client rather than raised.",
|
||||
)
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
content: list[ContentBlock] | Any | None = None,
|
||||
structured_content: dict[str, Any] | Any | None = None,
|
||||
meta: dict[str, Any] | None = None,
|
||||
is_error: bool = False,
|
||||
):
|
||||
if content is None and structured_content is None:
|
||||
raise ValueError("Either content or structured_content must be provided")
|
||||
|
|
@ -118,7 +125,10 @@ class ToolResult(BaseModel):
|
|||
)
|
||||
|
||||
super().__init__(
|
||||
content=converted_content, structured_content=structured_content, meta=meta
|
||||
content=converted_content,
|
||||
structured_content=structured_content,
|
||||
meta=meta,
|
||||
is_error=is_error,
|
||||
)
|
||||
|
||||
def to_mcp_result(
|
||||
|
|
@ -126,10 +136,13 @@ class ToolResult(BaseModel):
|
|||
) -> (
|
||||
list[ContentBlock] | tuple[list[ContentBlock], dict[str, Any]] | CallToolResult
|
||||
):
|
||||
if self.meta is not None:
|
||||
# An error result must round-trip through CallToolResult so isError
|
||||
# reaches the client; the plain content/tuple returns can't carry it.
|
||||
if self.meta is not None or self.is_error:
|
||||
return CallToolResult(
|
||||
structuredContent=self.structured_content,
|
||||
content=self.content,
|
||||
isError=self.is_error,
|
||||
_meta=self.meta, # type: ignore[call-arg] # _meta is Pydantic alias for meta field # ty:ignore[unknown-argument]
|
||||
)
|
||||
if self.structured_content is None:
|
||||
|
|
|
|||
|
|
@ -543,6 +543,13 @@ class TestCachableToolResult:
|
|||
assert cached_tool_result.structured_content == tool_result.structured_content
|
||||
assert cached_tool_result.meta == tool_result.meta
|
||||
|
||||
def test_wrap_and_unwrap_preserves_is_error(self):
|
||||
tool_result = ToolResult("boom", is_error=True)
|
||||
|
||||
cached_tool_result = CachableToolResult.wrap(tool_result).unwrap()
|
||||
|
||||
assert cached_tool_result.is_error is True
|
||||
|
||||
|
||||
class TestCachingWithImportedServerPrefixes:
|
||||
"""Test that caching preserves prefixes from imported servers.
|
||||
|
|
|
|||
|
|
@ -360,6 +360,28 @@ class TestTools:
|
|||
async with Client(proxy_server) as client:
|
||||
await client.call_tool("error_tool", {})
|
||||
|
||||
async def test_error_tool_passthrough_preserves_content(self, proxy_server):
|
||||
"""Upstream error results pass through with content intact, not flattened."""
|
||||
error_result = mcp_types.CallToolResult(
|
||||
content=[
|
||||
mcp_types.ImageContent(
|
||||
type="image", data="abc123", mimeType="image/png"
|
||||
)
|
||||
],
|
||||
structuredContent={"detail": "boom"},
|
||||
isError=True,
|
||||
)
|
||||
with patch.object(
|
||||
Client, "call_tool_mcp", new_callable=AsyncMock, return_value=error_result
|
||||
):
|
||||
async with Client(proxy_server) as client:
|
||||
result = await client.call_tool("error_tool", {}, raise_on_error=False)
|
||||
|
||||
assert result.is_error is True
|
||||
assert isinstance(result.content[0], mcp_types.ImageContent)
|
||||
assert result.content[0].data == "abc123"
|
||||
assert result.structured_content == {"detail": "boom"}
|
||||
|
||||
async def test_call_tool_forwards_meta(self, fastmcp_server, proxy_server):
|
||||
"""Test that metadata from proxied tool results is properly forwarded."""
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ from dataclasses import dataclass
|
|||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from mcp.types import CallToolResult, TextContent
|
||||
|
||||
from fastmcp.tools.base import Tool, ToolResult
|
||||
|
||||
|
|
@ -70,6 +71,56 @@ class TestToolResultCasting:
|
|||
assert result.meta == {"some": "metadata"}
|
||||
|
||||
|
||||
class TestToolResultIsError:
|
||||
"""A tool can return an error result (isError) instead of raising."""
|
||||
|
||||
def test_to_mcp_result_sets_iserror_and_preserves_content(self):
|
||||
result = ToolResult(
|
||||
content="boom", structured_content={"code": 42}, is_error=True
|
||||
)
|
||||
mcp_result = result.to_mcp_result()
|
||||
assert isinstance(mcp_result, CallToolResult)
|
||||
assert mcp_result.isError is True
|
||||
assert isinstance(mcp_result.content[0], TextContent)
|
||||
assert mcp_result.content[0].text == "boom"
|
||||
assert mcp_result.structuredContent == {"code": 42}
|
||||
|
||||
def test_default_is_not_error(self):
|
||||
result = ToolResult(content="ok")
|
||||
assert result.is_error is False
|
||||
|
||||
async def test_returned_error_raises_on_client_by_default(self):
|
||||
from fastmcp import FastMCP
|
||||
from fastmcp.client import Client
|
||||
from fastmcp.exceptions import ToolError
|
||||
|
||||
mcp = FastMCP()
|
||||
|
||||
@mcp.tool
|
||||
def failing() -> ToolResult:
|
||||
return ToolResult(content="upstream boom", is_error=True)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
with pytest.raises(ToolError):
|
||||
await client.call_tool("failing", {})
|
||||
|
||||
async def test_returned_error_preserves_content_when_not_raising(self):
|
||||
from fastmcp import FastMCP
|
||||
from fastmcp.client import Client
|
||||
|
||||
mcp = FastMCP()
|
||||
|
||||
@mcp.tool
|
||||
def failing() -> ToolResult:
|
||||
return ToolResult(content="upstream boom", is_error=True)
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool("failing", {}, raise_on_error=False)
|
||||
|
||||
assert result.is_error is True
|
||||
assert result.content[0].text == "upstream boom"
|
||||
|
||||
|
||||
class TestUnionReturnTypes:
|
||||
"""Tests for tools with union return types."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue