Skip to content

fix(agentgateway): raise AgentGatewayServerError on isError tool results - #327

Open
wangzhengzhuo05 wants to merge 1 commit into
SAP:mainfrom
wangzhengzhuo05:fix/raise-server-error-on-mcp-tool-iserror
Open

fix(agentgateway): raise AgentGatewayServerError on isError tool results#327
wangzhengzhuo05 wants to merge 1 commit into
SAP:mainfrom
wangzhengzhuo05:fix/raise-server-error-on-mcp-tool-iserror

Conversation

@wangzhengzhuo05

Copy link
Copy Markdown

What

AgentGatewayClient.call_mcp_tool silently swallowed MCP tool errors. When the MCP server returns a spec-compliant error response (isError: true, HTTP 200, SSE event: message with result.isError == true), both call_mcp_tool_lob (LoB flow) and call_mcp_tool_customer (customer flow) logged the error and returned the raw error text as if it were a successful tool result. Callers could not distinguish success from failure — any caller inspecting result.isError on the return value crashed with AttributeError (reported as 'NoneType' object has no attribute 'isError').

Why

The SDK already documents the intended contract in src/sap_cloud_sdk/agentgateway/exceptions.py: AgentGatewayServerError is raised when "A tool invocation returns an error result (isError=True)". The implementation violated its own documented contract — the exception was never raised on this path. Silently returning a value that crashes .isError accessors is never a valid outcome for a tool call that failed server-side.

How

  • src/sap_cloud_sdk/agentgateway/_lob.py: in call_mcp_tool_lob, replace the log-and-return block with raise AgentGatewayServerError(f"Tool '{tool.name}' on '{tool.url}' returned an error: {text}") when mcp_is_error(result) is true.
  • src/sap_cloud_sdk/agentgateway/_customer.py: identical fix in call_mcp_tool_customer so both flows behave consistently.
  • Success path unchanged (still returns the tool text); empty-content path unchanged (still returns "").
  • Tests: added test_raises_server_error_on_is_error to TestCallMcpToolLob and TestCallMcpToolCustomer; set explicit is_error=False/isError=False on the existing success-path mocks (a plain MagicMock is truthy, so those tests were implicitly passing through the error branch before).

Verification

  • Reproduced the bug at the wire level: a local HTTP server answering the exact SSE payload from the issue (event: message / data: {"result": {"content": [...], "isError": true}, "jsonrpc": "2.0", "id": 3}) against call_mcp_tool_lob returned the error text as a plain str and .isError raised AttributeError: 'str' object has no attribute 'isError'.
  • Targeted suite: pytest tests/agentgateway/unit/test_lob.py tests/agentgateway/unit/test_customer.py → 103 passed.
  • Full agentgateway unit suite: 220 passed, 24 failed — the 24 failures are in test_converters.py and pre-exist on a clean origin/main checkout (verified via a fresh worktree: same 24 failures, unrelated to this change).
  • Mutation check: reverted only the fix's behavior in _lob.py (restored log-and-return) → test_raises_server_error_on_is_error fails (1 failed); restored the fix → passes. The new test guards this behavior.

Notes

  • The reporter's exact stack ('NoneType' object has no attribute 'isError') comes from an older SDK version; on current main the returned value is a str, producing 'str' object has no attribute 'isError' — same class of defect, same fix.
  • No new dependencies; no formatting churn.

Fixes #326

AI-generated code disclosure

This change was implemented with AI assistance (OpenCode + muse-spark) and reviewed/verified for correctness, security, and license compatibility per SAP GenAI guidelines.

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 SAP#326
@wangzhengzhuo05
wangzhengzhuo05 requested a review from a team as a code owner September 11, 2026 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AgentGatewayClient.call_mcp_tool returns None instead of CallToolResult when MCP server responds with isError: true

1 participant