From a77e245a167d4909b2f03f635d4f5534630181b5 Mon Sep 17 00:00:00 2001 From: WilliamK112 <164879897+WilliamK112@users.noreply.github.com> Date: Tue, 8 Sep 2026 18:38:54 -0500 Subject: [PATCH] fix(core): raise on MCP tool error results --- .../src/toolbox_core/exceptions.py | 8 ++++++++ .../mcp_transport/transport_base.py | 13 ++++++++++--- .../mcp_transport/v20241105/mcp.py | 4 +++- .../mcp_transport/v20250326/mcp.py | 4 +++- .../mcp_transport/v20250618/mcp.py | 4 +++- .../mcp_transport/v20251125/mcp.py | 4 +++- .../mcp_transport/v20260728/mcp.py | 4 +++- .../tests/mcp_transport/test_v20241105.py | 19 ++++++++++++++++++- .../tests/mcp_transport/test_v20250326.py | 18 ++++++++++++++++++ .../tests/mcp_transport/test_v20250618.py | 19 ++++++++++++++++++- .../tests/mcp_transport/test_v20251125.py | 19 ++++++++++++++++++- .../tests/mcp_transport/test_v20260728.py | 18 ++++++++++++++++++ 12 files changed, 123 insertions(+), 11 deletions(-) diff --git a/packages/toolbox-core/src/toolbox_core/exceptions.py b/packages/toolbox-core/src/toolbox_core/exceptions.py index 384bcd52c..fead511ee 100644 --- a/packages/toolbox-core/src/toolbox_core/exceptions.py +++ b/packages/toolbox-core/src/toolbox_core/exceptions.py @@ -19,6 +19,14 @@ class ToolboxError(Exception): pass +class ToolInvocationError(ToolboxError): + """Raised when an MCP server reports that a tool invocation failed.""" + + def __init__(self, content: str): + self.content = content + super().__init__(content) + + class ProtocolNegotiationError(ToolboxError): """Raised when the server requires a different protocol version during a stateless request.""" diff --git a/packages/toolbox-core/src/toolbox_core/mcp_transport/transport_base.py b/packages/toolbox-core/src/toolbox_core/mcp_transport/transport_base.py index 781278c48..0a69ae557 100644 --- a/packages/toolbox-core/src/toolbox_core/mcp_transport/transport_base.py +++ b/packages/toolbox-core/src/toolbox_core/mcp_transport/transport_base.py @@ -21,6 +21,7 @@ from aiohttp import ClientSession from .. import version +from ..exceptions import ToolInvocationError from ..itransport import ITransport from ..protocol import ( AdditionalPropertiesSchema, @@ -98,20 +99,26 @@ async def _ensure_initialized( def base_url(self) -> str: return self._mcp_base_url - def _process_tool_result_content(self, content: list) -> str: + def _process_tool_result_content( + self, content: list, *, is_error: bool = False + ) -> str: """Processes the tool result content, handling multiple JSON objects.""" texts = [c.text for c in content if getattr(c, "type", "") == "text"] + result = "".join(texts) or "null" if len(texts) > 1: try: # Check if all chunks are valid JSON objects (dictionaries) if all(isinstance(json.loads(t), dict) for t in texts): - return f"[{','.join(texts)}]" + result = f"[{','.join(texts)}]" except (ValueError, TypeError): # Not valid JSON or not objects, fall back to simple concatenation pass - return "".join(texts) or "null" + if is_error: + raise ToolInvocationError(result) + + return result def _convert_parameter_schema( self, name: str, schema: dict, required_fields: list[str] diff --git a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20241105/mcp.py b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20241105/mcp.py index 8e1262821..18a4d98ca 100644 --- a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20241105/mcp.py +++ b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20241105/mcp.py @@ -393,7 +393,9 @@ async def tool_invoke( f"Failed to invoke tool '{tool_name}': No response from server." ) - return self._process_tool_result_content(result.content) + return self._process_tool_result_content( + result.content, is_error=result.isError + ) except Exception as e: error = e raise diff --git a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250326/mcp.py b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250326/mcp.py index aecff9ada..4b0918626 100644 --- a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250326/mcp.py +++ b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250326/mcp.py @@ -420,7 +420,9 @@ async def tool_invoke( f"Failed to invoke tool '{tool_name}': No response from server." ) - return self._process_tool_result_content(result.content) + return self._process_tool_result_content( + result.content, is_error=result.isError + ) except Exception as e: error = e raise diff --git a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250618/mcp.py b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250618/mcp.py index 7a9611990..bc55f675b 100644 --- a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250618/mcp.py +++ b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20250618/mcp.py @@ -401,7 +401,9 @@ async def tool_invoke( f"Failed to invoke tool '{tool_name}': No response from server." ) - return self._process_tool_result_content(result.content) + return self._process_tool_result_content( + result.content, is_error=result.isError + ) except Exception as e: error = e raise diff --git a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20251125/mcp.py b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20251125/mcp.py index ec78212f8..05bed1f72 100644 --- a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20251125/mcp.py +++ b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20251125/mcp.py @@ -401,7 +401,9 @@ async def tool_invoke( f"Failed to invoke tool '{tool_name}': No response from server." ) - return self._process_tool_result_content(result.content) + return self._process_tool_result_content( + result.content, is_error=result.isError + ) except Exception as e: error = e raise diff --git a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20260728/mcp.py b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20260728/mcp.py index af7eb334f..f91e60290 100644 --- a/packages/toolbox-core/src/toolbox_core/mcp_transport/v20260728/mcp.py +++ b/packages/toolbox-core/src/toolbox_core/mcp_transport/v20260728/mcp.py @@ -347,7 +347,9 @@ async def tool_invoke( f"Failed to invoke tool '{tool_name}': No response from server." ) - return self._process_tool_result_content(result.content) + return self._process_tool_result_content( + result.content, is_error=result.isError + ) except Exception as e: error = e raise diff --git a/packages/toolbox-core/tests/mcp_transport/test_v20241105.py b/packages/toolbox-core/tests/mcp_transport/test_v20241105.py index 23bbbf059..f50325921 100644 --- a/packages/toolbox-core/tests/mcp_transport/test_v20241105.py +++ b/packages/toolbox-core/tests/mcp_transport/test_v20241105.py @@ -18,7 +18,7 @@ import pytest_asyncio from aiohttp import ClientSession -from toolbox_core.exceptions import ProtocolNegotiationError +from toolbox_core.exceptions import ProtocolNegotiationError, ToolInvocationError from toolbox_core.mcp_transport.v20241105 import types from toolbox_core.mcp_transport.v20241105.mcp import McpHttpTransportV20241105 from toolbox_core.protocol import ManifestSchema, Protocol @@ -461,6 +461,23 @@ async def test_tool_invoke_success(self, transport, mocker): result = await transport.tool_invoke("tool", {}, {}) assert result == "Result" + async def test_tool_invoke_error_result(self, transport, mocker): + mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) + mocker.patch.object( + transport, + "_send_request", + new_callable=AsyncMock, + return_value=types.CallToolResult( + content=[types.TextContent(type="text", text="tool failed")], + isError=True, + ), + ) + + with pytest.raises(ToolInvocationError, match="tool failed") as exc_info: + await transport.tool_invoke("tool", {}, {}) + + assert exc_info.value.content == "tool failed" + async def test_tool_get_success(self, transport, mocker): mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) mocker.patch.object( diff --git a/packages/toolbox-core/tests/mcp_transport/test_v20250326.py b/packages/toolbox-core/tests/mcp_transport/test_v20250326.py index a17ad2d80..9fc24c71c 100644 --- a/packages/toolbox-core/tests/mcp_transport/test_v20250326.py +++ b/packages/toolbox-core/tests/mcp_transport/test_v20250326.py @@ -18,6 +18,7 @@ import pytest_asyncio from aiohttp import ClientSession +from toolbox_core.exceptions import ToolInvocationError from toolbox_core.mcp_transport.v20250326 import types from toolbox_core.mcp_transport.v20250326.mcp import McpHttpTransportV20250326 from toolbox_core.protocol import ManifestSchema, Protocol @@ -461,6 +462,23 @@ async def test_tool_invoke_success(self, transport, mocker): result = await transport.tool_invoke("tool", {}, {}) assert result == "Result" + async def test_tool_invoke_error_result(self, transport, mocker): + mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) + mocker.patch.object( + transport, + "_send_request", + new_callable=AsyncMock, + return_value=types.CallToolResult( + content=[types.TextContent(type="text", text="tool failed")], + isError=True, + ), + ) + + with pytest.raises(ToolInvocationError, match="tool failed") as exc_info: + await transport.tool_invoke("tool", {}, {}) + + assert exc_info.value.content == "tool failed" + async def test_tool_get_success(self, transport, mocker): mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) mocker.patch.object( diff --git a/packages/toolbox-core/tests/mcp_transport/test_v20250618.py b/packages/toolbox-core/tests/mcp_transport/test_v20250618.py index 077876476..4cff95a69 100644 --- a/packages/toolbox-core/tests/mcp_transport/test_v20250618.py +++ b/packages/toolbox-core/tests/mcp_transport/test_v20250618.py @@ -18,7 +18,7 @@ import pytest_asyncio from aiohttp import ClientSession -from toolbox_core.exceptions import ProtocolNegotiationError +from toolbox_core.exceptions import ProtocolNegotiationError, ToolInvocationError from toolbox_core.mcp_transport.v20250618 import types from toolbox_core.mcp_transport.v20250618.mcp import McpHttpTransportV20250618 from toolbox_core.protocol import ManifestSchema, Protocol, TelemetryAttributes @@ -464,6 +464,23 @@ async def test_tool_invoke_success(self, transport, mocker): result = await transport.tool_invoke("tool", {}, {}) assert result == "Result" + async def test_tool_invoke_error_result(self, transport, mocker): + mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) + mocker.patch.object( + transport, + "_send_request", + new_callable=AsyncMock, + return_value=types.CallToolResult( + content=[types.TextContent(type="text", text="tool failed")], + isError=True, + ), + ) + + with pytest.raises(ToolInvocationError, match="tool failed") as exc_info: + await transport.tool_invoke("tool", {}, {}) + + assert exc_info.value.content == "tool failed" + async def test_tool_get_success(self, transport, mocker): mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) mocker.patch.object( diff --git a/packages/toolbox-core/tests/mcp_transport/test_v20251125.py b/packages/toolbox-core/tests/mcp_transport/test_v20251125.py index f90a3d808..7e3cf4c93 100644 --- a/packages/toolbox-core/tests/mcp_transport/test_v20251125.py +++ b/packages/toolbox-core/tests/mcp_transport/test_v20251125.py @@ -18,7 +18,7 @@ import pytest_asyncio from aiohttp import ClientSession -from toolbox_core.exceptions import ProtocolNegotiationError +from toolbox_core.exceptions import ProtocolNegotiationError, ToolInvocationError from toolbox_core.mcp_transport.v20251125 import types from toolbox_core.mcp_transport.v20251125.mcp import McpHttpTransportV20251125 from toolbox_core.protocol import ManifestSchema, Protocol @@ -491,6 +491,23 @@ async def test_tool_invoke_success(self, transport, mocker): result = await transport.tool_invoke("tool", {}, {}) assert result == "Result" + async def test_tool_invoke_error_result(self, transport, mocker): + mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) + mocker.patch.object( + transport, + "_send_request", + new_callable=AsyncMock, + return_value=types.CallToolResult( + content=[types.TextContent(type="text", text="tool failed")], + isError=True, + ), + ) + + with pytest.raises(ToolInvocationError, match="tool failed") as exc_info: + await transport.tool_invoke("tool", {}, {}) + + assert exc_info.value.content == "tool failed" + async def test_tool_get_success(self, transport, mocker): mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) mocker.patch.object( diff --git a/packages/toolbox-core/tests/mcp_transport/test_v20260728.py b/packages/toolbox-core/tests/mcp_transport/test_v20260728.py index 55925d553..1b31ea6f9 100644 --- a/packages/toolbox-core/tests/mcp_transport/test_v20260728.py +++ b/packages/toolbox-core/tests/mcp_transport/test_v20260728.py @@ -19,6 +19,7 @@ from aiohttp import ClientSession from aioresponses import aioresponses +from toolbox_core.exceptions import ToolInvocationError from toolbox_core.mcp_transport.v20260728 import types from toolbox_core.mcp_transport.v20260728.mcp import McpHttpTransportV20260728 from toolbox_core.protocol import ManifestSchema, Protocol @@ -393,6 +394,23 @@ async def test_tool_invoke_success(self, transport, mocker): result = await transport.tool_invoke("tool", {}, {}) assert result == "Result" + async def test_tool_invoke_error_result(self, transport, mocker): + mocker.patch.object(transport, "_ensure_initialized", new_callable=AsyncMock) + mocker.patch.object( + transport, + "_send_request", + new_callable=AsyncMock, + return_value=types.CallToolResult( + content=[types.TextContent(type="text", text="tool failed")], + isError=True, + ), + ) + + with pytest.raises(ToolInvocationError, match="tool failed") as exc_info: + await transport.tool_invoke("tool", {}, {}) + + assert exc_info.value.content == "tool failed" + async def test_send_request_400_with_json_rpc_error(self, transport): # Test that an HTTP 400 with a non-negotiation JSON-RPC error is parsed properly. mock_response = AsyncMock()