From 09fc6c384a728302bdc0c5e035c7cb4231711244 Mon Sep 17 00:00:00 2001 From: wangzhengzhuo05 <175673456+wangzhengzhuo05@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:01:36 +0800 Subject: [PATCH] fix(agentgateway): raise AgentGatewayServerError on isError tool results MCP tools that return a spec-compliant error response (isError=true) were silently flattened to the raw text, so callers checking result.isError crashed with AttributeError. Raise the documented AgentGatewayServerError with the server message instead, in both the LoB and customer call paths. Fixes #326 --- src/sap_cloud_sdk/agentgateway/_customer.py | 12 ++--- src/sap_cloud_sdk/agentgateway/_lob.py | 8 ++- tests/agentgateway/unit/test_customer.py | 59 ++++++++++++++++++++- tests/agentgateway/unit/test_lob.py | 50 +++++++++++++++++ 4 files changed, 117 insertions(+), 12 deletions(-) diff --git a/src/sap_cloud_sdk/agentgateway/_customer.py b/src/sap_cloud_sdk/agentgateway/_customer.py index f0ec1020..2a8d5d15 100644 --- a/src/sap_cloud_sdk/agentgateway/_customer.py +++ b/src/sap_cloud_sdk/agentgateway/_customer.py @@ -46,7 +46,10 @@ MCPToolFilter, ) from sap_cloud_sdk.agentgateway._token_cache import _TokenCache -from sap_cloud_sdk.agentgateway.exceptions import AgentGatewaySDKError +from sap_cloud_sdk.agentgateway.exceptions import ( + AgentGatewaySDKError, + AgentGatewayServerError, +) from sap_cloud_sdk.core.secret_resolver import resolve_base_mount logger = logging.getLogger(__name__) @@ -841,11 +844,8 @@ async def call_mcp_tool_customer( text = str(getattr(first, "text", "")) if mcp_is_error(result): - logger.error( - "Tool '%s' on '%s' returned an error: %s", - tool.name, - tool.url, - text, + raise AgentGatewayServerError( + f"Tool '{tool.name}' on '{tool.url}' returned an error: {text}" ) return text diff --git a/src/sap_cloud_sdk/agentgateway/_lob.py b/src/sap_cloud_sdk/agentgateway/_lob.py index 3813b998..8274f233 100644 --- a/src/sap_cloud_sdk/agentgateway/_lob.py +++ b/src/sap_cloud_sdk/agentgateway/_lob.py @@ -50,6 +50,7 @@ from sap_cloud_sdk.agentgateway._token_cache import _GatewayUrlCache, _TokenCache from sap_cloud_sdk.agentgateway.exceptions import ( AgentGatewaySDKError, + AgentGatewayServerError, MCPServerNotFoundError, ) @@ -526,11 +527,8 @@ async def call_mcp_tool_lob( text = str(getattr(first, "text", "")) if mcp_is_error(result): - logger.error( - "Tool '%s' on '%s' returned an error: %s", - tool.name, - tool.url, - text, + raise AgentGatewayServerError( + f"Tool '{tool.name}' on '{tool.url}' returned an error: {text}" ) return text diff --git a/tests/agentgateway/unit/test_customer.py b/tests/agentgateway/unit/test_customer.py index 512875be..1d603d0f 100644 --- a/tests/agentgateway/unit/test_customer.py +++ b/tests/agentgateway/unit/test_customer.py @@ -27,7 +27,10 @@ ) from sap_cloud_sdk.agentgateway._token_cache import _TokenCache from sap_cloud_sdk.agentgateway.config import ClientConfig -from sap_cloud_sdk.agentgateway.exceptions import AgentGatewaySDKError +from sap_cloud_sdk.agentgateway.exceptions import ( + AgentGatewaySDKError, + AgentGatewayServerError, +) # ============================================================ @@ -766,6 +769,8 @@ async def test_calls_tool_with_pre_fetched_token(self, credentials, mock_tool): mock_content = MagicMock() mock_content.text = "Order created successfully" mock_result.content = [mock_content] + mock_result.is_error = False + mock_result.isError = False mock_session.call_tool = AsyncMock(return_value=mock_result) mock_session_ctx = AsyncMock() mock_session_ctx.__aenter__ = AsyncMock(return_value=mock_session) @@ -824,6 +829,58 @@ async def test_returns_empty_string_when_no_content(self, credentials, mock_tool assert result == "" + @pytest.mark.asyncio + async def test_raises_server_error_on_is_error( + self, credentials, mock_tool + ): + """Raise AgentGatewayServerError when tool returns isError.""" + tool = MCPTool( + name="test-tool", + server_name="test-server", + description="Test tool", + input_schema={}, + url="https://example.com/mcp", + fragment_name="test-fragment", + ) + + with ( + patch( + "httpx.AsyncClient", + ), + patch( + "sap_cloud_sdk.agentgateway._customer.streamable_http_client", + ) as mock_stream, + patch( + "sap_cloud_sdk.agentgateway._customer.ClientSession", + ) as mock_session_class, + ): + mock_stream_ctx = AsyncMock() + mock_stream_ctx.__aenter__ = AsyncMock( + return_value=(AsyncMock(), AsyncMock(), None) + ) + mock_stream_ctx.__aexit__ = AsyncMock(return_value=None) + mock_stream.return_value = mock_stream_ctx + + mock_session = AsyncMock() + mock_session.initialize = AsyncMock() + mock_result = MagicMock() + mock_result.content = [MagicMock()] + mock_result.content[0].text = "backend exploded" + mock_result.is_error = True + mock_result.isError = True + mock_session.call_tool = AsyncMock(return_value=mock_result) + mock_session_ctx = AsyncMock() + mock_session_ctx.__aenter__ = AsyncMock(return_value=mock_session) + mock_session_ctx.__aexit__ = AsyncMock(return_value=None) + mock_session_class.return_value = mock_session_ctx + + with pytest.raises( + AgentGatewayServerError, match="backend exploded" + ): + await call_mcp_tool_customer(tool, "auth-token", 60.0) + + mock_session.call_tool.assert_called_once() + # ============================================================ # Test: detect_transparent_credentials diff --git a/tests/agentgateway/unit/test_lob.py b/tests/agentgateway/unit/test_lob.py index fd9396a9..6ae11117 100644 --- a/tests/agentgateway/unit/test_lob.py +++ b/tests/agentgateway/unit/test_lob.py @@ -41,6 +41,7 @@ from sap_cloud_sdk.destination import ConsumptionOptions, ConsumptionLevel from sap_cloud_sdk.agentgateway.exceptions import ( AgentGatewaySDKError, + AgentGatewayServerError, MCPServerNotFoundError, ) from sap_cloud_sdk.destination import ConsumptionLevel @@ -953,6 +954,8 @@ async def test_calls_tool_with_pre_fetched_token(self): mock_result = MagicMock() mock_result.content = [MagicMock()] mock_result.content[0].text = "Tool result" + mock_result.is_error = False + mock_result.isError = False with ( patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, @@ -1030,6 +1033,53 @@ async def test_returns_empty_string_when_no_content(self): assert result == "" + @pytest.mark.asyncio + async def test_raises_server_error_on_is_error(self): + """Raise AgentGatewayServerError when tool returns isError.""" + tool = MCPTool( + name="test-tool", + server_name="test-server", + description="Test tool", + input_schema={}, + url="https://example.com/mcp", + fragment_name="test-fragment", + ) + + mock_result = MagicMock() + mock_result.content = [MagicMock()] + mock_result.content[0].text = "backend exploded" + mock_result.is_error = True + mock_result.isError = True + + with ( + patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient"), + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + mock_stream.return_value.__aenter__.return_value = ( + AsyncMock(), + AsyncMock(), + None, + ) + + mock_session_instance = AsyncMock() + mock_session_instance.initialize = AsyncMock() + mock_session_instance.call_tool = AsyncMock( + return_value=mock_result + ) + mock_session.return_value.__aenter__.return_value = ( + mock_session_instance + ) + + with pytest.raises( + AgentGatewayServerError, match="backend exploded" + ): + await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + + mock_session_instance.call_tool.assert_called_once() + # ============================================================ # Test: list_a2a_fragments