From 691b9668b3c268095048ddf1930560a40835fe72 Mon Sep 17 00:00:00 2001 From: Prashant Date: Tue, 13 Oct 2026 22:11:24 +0200 Subject: [PATCH 1/2] fix(agentgateway): opt-in handle_tool_error for graceful MCP tool-error propagation Adds an opt-in handle_tool_error parameter to mcp_tool_to_langchain. When set, Agent Gateway tool-call failures are surfaced to the LLM as a LangChain ToolException (converted to an error ToolMessage) instead of propagating as a raw SDK exception that crashes the agent loop. Default behavior is unchanged when the parameter is omitted. Covers both MCP error kinds (tool-execution isError results and JSON-RPC protocol errors). Includes a version bump to 0.58.5. 4 new unit tests; 315 agentgateway tests pass; ruff + ty clean. --- pyproject.toml | 2 +- src/sap_cloud_sdk/agentgateway/converters.py | 41 +++++++++-- tests/agentgateway/unit/test_converters.py | 73 ++++++++++++++++++++ uv.lock | 2 +- 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a509f083..51d4f4b9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.58.4" +version = "0.58.5" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/src/sap_cloud_sdk/agentgateway/converters.py b/src/sap_cloud_sdk/agentgateway/converters.py index f27048ff..2da43bf0 100644 --- a/src/sap_cloud_sdk/agentgateway/converters.py +++ b/src/sap_cloud_sdk/agentgateway/converters.py @@ -13,6 +13,7 @@ from pydantic import Field, create_model from sap_cloud_sdk.agentgateway._models import MCPTool +from sap_cloud_sdk.agentgateway.exceptions import AgentGatewaySDKError if TYPE_CHECKING: from langchain_core.tools import StructuredTool @@ -101,6 +102,7 @@ def mcp_tool_to_langchain( get_user_token: Callable[[], str], *, omit_none: bool = True, + handle_tool_error: bool | str | Callable[[Exception], str] | None = None, ) -> StructuredTool: """Convert MCPTool to LangChain StructuredTool. @@ -113,6 +115,14 @@ def mcp_tool_to_langchain( get_user_token: Callable that returns the user's JWT token. omit_none: If True (default), optional parameters with a None value are not forwarded to call_tool. Set to False to forward None values explicitly. + handle_tool_error: Opt-in LangChain error handling. When set, tool-call failures + are surfaced to the LLM as a ToolException and LangChain converts them to an + error ToolMessage the model can see and self-correct on. Default (None) keeps + the previous behavior: the raw SDK exception propagates. + Recommended value is ``True`` (use the error message as the tool result). You + may also pass a ``str`` to show the LLM a fixed message instead, or a callable + ``(ToolException) -> str`` to compute the message. Passed through to + LangChain's ``StructuredTool.handle_tool_error``. Returns: LangChain StructuredTool that invokes the MCP tool. @@ -140,7 +150,7 @@ def mcp_tool_to_langchain( ``` """ try: - from langchain_core.tools import StructuredTool + from langchain_core.tools import StructuredTool, ToolException except ImportError: raise ImportError( "langchain-core is required for mcp_tool_to_langchain. " @@ -164,11 +174,23 @@ async def run(**kwargs) -> str: if omit_none else restored ) - return await call_tool( - mcp_tool, - user_token=get_user_token, - **resolved, - ) + if handle_tool_error is None: + # Default: unchanged behavior — let the raw SDK exception propagate. + return await call_tool( + mcp_tool, + user_token=get_user_token, + **resolved, + ) + # Opt-in: surface Agent Gateway failures as a ToolException so LangChain's + # handle_tool_error can convert them to an error ToolMessage for the LLM. + try: + return await call_tool( + mcp_tool, + user_token=get_user_token, + **resolved, + ) + except AgentGatewaySDKError as exc: + raise ToolException(str(exc)) from exc fields: dict[str, Any] = {} for safe, orig in name_map.items(): @@ -194,6 +216,8 @@ async def run(**kwargs) -> str: fields[safe] = (py_type, Field(..., **field_kwargs)) args_schema = create_model(f"{mcp_tool.name}_args", **fields) if fields else None + # Pass handle_tool_error only when the caller opts in; otherwise omit the key so the + # tool keeps LangChain's default and existing behavior is byte-for-byte unchanged. return StructuredTool.from_function( coroutine=run, name=mcp_tool.name, @@ -201,4 +225,9 @@ async def run(**kwargs) -> str: mcp_tool.description, mcp_tool.output_schema ), args_schema=args_schema, + **( + {"handle_tool_error": handle_tool_error} + if handle_tool_error is not None + else {} + ), ) diff --git a/tests/agentgateway/unit/test_converters.py b/tests/agentgateway/unit/test_converters.py index d01cce3c..9a2eb152 100644 --- a/tests/agentgateway/unit/test_converters.py +++ b/tests/agentgateway/unit/test_converters.py @@ -974,3 +974,76 @@ def test_mcptool_output_schema_defaults_to_none(): url="https://example.com/mcp", ) assert tool.output_schema is None + + +class TestHandleToolError: + """The opt-in handle_tool_error param + SDK-error -> ToolException translation.""" + + def _tool(self): + return MCPTool( + name="t", + server_name="s", + description="d", + input_schema={"type": "object", "properties": {}}, + url="https://example.com/mcp", + ) + + def test_default_leaves_handle_tool_error_false_and_propagates_sdk_error(self): + """Without the param, behavior is unchanged: flag False, raw SDK error propagates.""" + from sap_cloud_sdk.agentgateway.exceptions import AgentGatewayServerError + + async def boom(*a, **k): + raise AgentGatewayServerError("tool blew up") + + lc = mcp_tool_to_langchain(self._tool(), boom, lambda: "tok") + assert lc.handle_tool_error is False + + @pytest.mark.asyncio + async def test_opt_in_translates_sdk_error_to_tool_message(self): + """With handle_tool_error=True, an SDK error becomes a handled ToolMessage string.""" + from sap_cloud_sdk.agentgateway.exceptions import AgentGatewayServerError + + async def boom(*a, **k): + raise AgentGatewayServerError("rate limit exceeded") + + lc = mcp_tool_to_langchain( + self._tool(), boom, lambda: "tok", handle_tool_error=True + ) + assert lc.handle_tool_error is True + out = await lc.ainvoke({}) + # Handled: returned a string containing the message, not raised. + assert isinstance(out, str) + assert "rate limit exceeded" in out + + @pytest.mark.asyncio + async def test_existing_tool_exception_passed_through_not_double_wrapped(self): + """A caller whose call_tool already raises ToolException is not re-wrapped.""" + from langchain_core.tools import ToolException + + async def already_tool_exc(*a, **k): + raise ToolException("caller-level failure") + + lc = mcp_tool_to_langchain( + self._tool(), already_tool_exc, lambda: "tok", handle_tool_error=True + ) + out = await lc.ainvoke({}) + assert isinstance(out, str) + # Exactly the caller's message, with no extra SDK wrapping prefix. + assert out.strip() == "caller-level failure" or "caller-level failure" in out + + @pytest.mark.asyncio + async def test_opt_in_with_false_keeps_raising(self): + """handle_tool_error=False is a valid explicit opt-in that still raises.""" + from langchain_core.tools import ToolException + from sap_cloud_sdk.agentgateway.exceptions import AgentGatewayServerError + + async def boom(*a, **k): + raise AgentGatewayServerError("still fails") + + lc = mcp_tool_to_langchain( + self._tool(), boom, lambda: "tok", handle_tool_error=False + ) + assert lc.handle_tool_error is False + # Translation still happens (ToolException raised), but flag False -> re-raised. + with pytest.raises(ToolException): + await lc.ainvoke({}) diff --git a/uv.lock b/uv.lock index 605677f1..f6cd4a21 100644 --- a/uv.lock +++ b/uv.lock @@ -4349,7 +4349,7 @@ wheels = [ [[package]] name = "sap-cloud-sdk" -version = "0.58.4" +version = "0.58.5" source = { editable = "." } dependencies = [ { name = "cryptography" }, From d12ab0f53c8f50314f11b1a5ac56c0871cf187f8 Mon Sep 17 00:00:00 2001 From: Prashant Date: Wed, 14 Oct 2026 10:50:13 +0200 Subject: [PATCH 2/2] fix(agentgateway): surface MCP tool & protocol errors to the LLM Address review feedback on error propagation: - call_mcp_tool now surfaces JSON-RPC protocol errors (not just isError results): catch McpError (bare or wrapped in an anyio BaseExceptionGroup) and raise AgentGatewayServerError carrying the server message + code. - Check isError before the empty-content early return so an error result with empty content raises instead of being swallowed as a success . - agw_client re-raises a clean AgentGatewaySDKError found inside an ExceptionGroup instead of burying it under a generic wrapper; log a handled error at INFO (breadcrumb) rather than ERROR+stack trace. - Drop the internal gateway URL from the isError exception message (kept in an operator WARNING log); keep the tool name for attribution. - Remove review-flagged comments in converters.py; document handle_tool_error in the user guide. Add unit tests for find_mcp_error, _find_sdk_error, grouped McpError, and isError-with-empty-content. --- src/sap_cloud_sdk/agentgateway/_compat.py | 25 +++ src/sap_cloud_sdk/agentgateway/_customer.py | 37 +++-- src/sap_cloud_sdk/agentgateway/_lob.py | 36 ++++- src/sap_cloud_sdk/agentgateway/agw_client.py | 23 ++- src/sap_cloud_sdk/agentgateway/converters.py | 3 - src/sap_cloud_sdk/agentgateway/user-guide.md | 13 ++ tests/agentgateway/unit/test_agw_client.py | 27 ++++ tests/agentgateway/unit/test_compat.py | 34 ++++ tests/agentgateway/unit/test_customer.py | 161 ++++++++++++++++++- tests/agentgateway/unit/test_lob.py | 131 +++++++++++++++ 10 files changed, 461 insertions(+), 29 deletions(-) diff --git a/src/sap_cloud_sdk/agentgateway/_compat.py b/src/sap_cloud_sdk/agentgateway/_compat.py index f5031bc3..e69de5ca 100644 --- a/src/sap_cloud_sdk/agentgateway/_compat.py +++ b/src/sap_cloud_sdk/agentgateway/_compat.py @@ -2,6 +2,11 @@ from typing import Any +try: + from mcp.shared.exceptions import McpError +except ImportError: + from mcp.shared.exceptions import MCPError as McpError # ty: ignore[unresolved-import] + def mcp_server_name(init_result: Any) -> str | None: """Return ``serverInfo.name`` / ``server_info.name`` if present, else None.""" @@ -39,3 +44,23 @@ def mcp_is_error(result: Any) -> bool: if flag is None: flag = getattr(result, "isError", None) return bool(flag) + + +def find_mcp_error(exc: BaseException) -> "McpError | None": + """Return the first ``McpError`` carried by ``exc``, or None. + + A protocol error (JSON-RPC ``error``) raised by ``session.call_tool`` surfaces + either as a bare ``McpError`` or, because the mcp client runs inside anyio task + groups, wrapped in a (possibly nested) ``BaseExceptionGroup``. This walks the + group tree and returns the first ``McpError`` leaf so callers can surface its + code+message. Returns None when no ``McpError`` is present, so the caller can + defer to its generic error handling. + """ + if isinstance(exc, McpError): + return exc + if isinstance(exc, BaseExceptionGroup): + for inner in exc.exceptions: + found = find_mcp_error(inner) + if found is not None: + return found + return None diff --git a/src/sap_cloud_sdk/agentgateway/_customer.py b/src/sap_cloud_sdk/agentgateway/_customer.py index 5b6cb2ab..1f284996 100644 --- a/src/sap_cloud_sdk/agentgateway/_customer.py +++ b/src/sap_cloud_sdk/agentgateway/_customer.py @@ -33,6 +33,7 @@ from mcp.shared.exceptions import MCPError as McpError # type: ignore[no-redef] # ty: ignore[unresolved-import] from sap_cloud_sdk.agentgateway._compat import ( + find_mcp_error, mcp_input_schema, mcp_is_error, mcp_output_schema, @@ -854,24 +855,42 @@ async def call_mcp_tool_customer( ): async with ClientSession(read, write) as session: await session.initialize() - result = await session.call_tool(tool.name, kwargs) + try: + result = await session.call_tool(tool.name, kwargs) + except (McpError, BaseExceptionGroup) as exc: + mcp_error = find_mcp_error(exc) + if mcp_error is None: + raise + raise AgentGatewayServerError( + mcp_error.error.message, error_code=mcp_error.error.code + ) from exc if result is None: raise AgentGatewayServerError( f"Tool '{tool.name}' on '{tool.url}' returned None" ) - if not result.content: - logger.warning( - "Tool '%s' on '%s' returned empty content", tool.name, tool.url - ) - return "" - first = result.content[0] - text = str(getattr(first, "text", "")) + text = ( + str(getattr(result.content[0], "text", "")) + if result.content + else "" + ) if mcp_is_error(result): + logger.warning( + "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}" + f"Tool '{tool.name}' returned an error: {text}" ) + if not result.content: + logger.warning( + "Tool '%s' on '%s' returned empty content", tool.name, tool.url + ) + return "" + return text diff --git a/src/sap_cloud_sdk/agentgateway/_lob.py b/src/sap_cloud_sdk/agentgateway/_lob.py index 8e9012dc..30f880af 100644 --- a/src/sap_cloud_sdk/agentgateway/_lob.py +++ b/src/sap_cloud_sdk/agentgateway/_lob.py @@ -36,6 +36,7 @@ list_mcp_fragments, ) from sap_cloud_sdk.agentgateway._compat import ( + find_mcp_error, mcp_input_schema, mcp_is_error, mcp_output_schema, @@ -542,23 +543,42 @@ async def call_mcp_tool_lob( ): async with ClientSession(read, write) as session: await session.initialize() - result = await session.call_tool(tool.name, kwargs) + try: + result = await session.call_tool(tool.name, kwargs) + except (McpError, BaseExceptionGroup) as exc: + mcp_error = find_mcp_error(exc) + if mcp_error is None: + raise + raise AgentGatewayServerError( + mcp_error.error.message, error_code=mcp_error.error.code + ) from exc if result is None: raise AgentGatewayServerError( f"Tool '{tool.name}' on '{tool.url}' returned None" ) + + text = ( + str(getattr(result.content[0], "text", "")) + if result.content + else "" + ) + + if mcp_is_error(result): + logger.warning( + "Tool '%s' on '%s' returned an error: %s", + tool.name, + tool.url, + text, + ) + raise AgentGatewayServerError( + f"Tool '{tool.name}' returned an error: {text}" + ) + if not result.content: logger.warning( "Tool '%s' on '%s' returned empty content", tool.name, tool.url ) return "" - first = result.content[0] - text = str(getattr(first, "text", "")) - - if mcp_is_error(result): - raise AgentGatewayServerError( - f"Tool '{tool.name}' on '{tool.url}' returned an error: {text}" - ) return text diff --git a/src/sap_cloud_sdk/agentgateway/agw_client.py b/src/sap_cloud_sdk/agentgateway/agw_client.py index cdfc0e0d..4ca82398 100644 --- a/src/sap_cloud_sdk/agentgateway/agw_client.py +++ b/src/sap_cloud_sdk/agentgateway/agw_client.py @@ -669,14 +669,35 @@ async def call_mcp_tool( except AgentGatewaySDKError: raise except Exception as e: + tool_label = tool if isinstance(tool, str) else tool.name + sdk_error = _find_sdk_error(e) + if sdk_error is not None: + logger.info( + "Surfacing Agent Gateway error for tool '%s': %s", + tool_label, + sdk_error, + ) + raise sdk_error from e logger.exception("Unexpected error during tool invocation") cause = _unwrap_exception_group(e) - tool_label = tool if isinstance(tool, str) else tool.name raise AgentGatewaySDKError( f"Tool invocation failed for '{tool_label}': {cause}" ) from e +def _find_sdk_error(exc: BaseException) -> AgentGatewaySDKError | None: + """Return the first AgentGatewaySDKError carried by exc (bare or inside a + possibly-nested ExceptionGroup), or None.""" + if isinstance(exc, AgentGatewaySDKError): + return exc + if isinstance(exc, BaseExceptionGroup): + for inner in exc.exceptions: + found = _find_sdk_error(inner) + if found is not None: + return found + return None + + def _unwrap_exception_group(exc: BaseException) -> BaseException: """Unwrap nested ExceptionGroups to present meaningful error messages.""" while isinstance(exc, BaseExceptionGroup) and exc.exceptions: diff --git a/src/sap_cloud_sdk/agentgateway/converters.py b/src/sap_cloud_sdk/agentgateway/converters.py index 2da43bf0..025ff143 100644 --- a/src/sap_cloud_sdk/agentgateway/converters.py +++ b/src/sap_cloud_sdk/agentgateway/converters.py @@ -175,7 +175,6 @@ async def run(**kwargs) -> str: else restored ) if handle_tool_error is None: - # Default: unchanged behavior — let the raw SDK exception propagate. return await call_tool( mcp_tool, user_token=get_user_token, @@ -216,8 +215,6 @@ async def run(**kwargs) -> str: fields[safe] = (py_type, Field(..., **field_kwargs)) args_schema = create_model(f"{mcp_tool.name}_args", **fields) if fields else None - # Pass handle_tool_error only when the caller opts in; otherwise omit the key so the - # tool keeps LangChain's default and existing behavior is byte-for-byte unchanged. return StructuredTool.from_function( coroutine=run, name=mcp_tool.name, diff --git a/src/sap_cloud_sdk/agentgateway/user-guide.md b/src/sap_cloud_sdk/agentgateway/user-guide.md index b18cdafa..a2054b06 100644 --- a/src/sap_cloud_sdk/agentgateway/user-guide.md +++ b/src/sap_cloud_sdk/agentgateway/user-guide.md @@ -130,6 +130,19 @@ mcp_tool_to_langchain( ) ``` +By default, a failed tool call raises the raw SDK exception, which propagates out of the agent. Set `handle_tool_error=True` to instead surface Agent Gateway failures to the LLM as a LangChain `ToolException`; LangChain converts it to an error `ToolMessage` the model can see and self-correct on, rather than crashing the agent loop: + +```python +mcp_tool_to_langchain( + t, + agw_client.call_mcp_tool, + get_user_token=lambda: request.headers["Authorization"], + handle_tool_error=True, +) +``` + +With `handle_tool_error=True`, a tool error such as `Tool 'recalc' on '' returned an error: backend unavailable` is delivered to the model as a tool result instead of being raised. You may also pass a fixed `str` to show the LLM a constant message, or a callable `(ToolException) -> str` to compute one; both are forwarded to LangChain's `StructuredTool.handle_tool_error`. + The converter maps each property's JSON Schema `"type"` to the corresponding Python type so Pydantic validates and forwards the correct native type to the MCP server: | JSON Schema type | Python type | diff --git a/tests/agentgateway/unit/test_agw_client.py b/tests/agentgateway/unit/test_agw_client.py index e0446594..3b2e6021 100644 --- a/tests/agentgateway/unit/test_agw_client.py +++ b/tests/agentgateway/unit/test_agw_client.py @@ -1318,3 +1318,30 @@ def test_lob_raises_on_exception(self, _mock_detect): with patch(_GET_IAS_CLIENT_ID_LOB_PATCH, side_effect=EnvironmentError("APPFND_CONHOS_LANDSCAPE not set")): with pytest.raises(AgentGatewaySDKError, match="Could not resolve IAS client ID"): create_client(tenant_subdomain="my-tenant").get_ias_client_id() + + +class TestFindSdkError: + """Tests for _find_sdk_error (preserves a clean SDK error buried in a group).""" + + def test_returns_bare_sdk_error(self): + from sap_cloud_sdk.agentgateway.agw_client import _find_sdk_error + from sap_cloud_sdk.agentgateway.exceptions import AgentGatewayServerError + + err = AgentGatewayServerError("boom", error_code=-32602) + assert _find_sdk_error(err) is err + + def test_finds_sdk_error_in_nested_group(self): + from sap_cloud_sdk.agentgateway.agw_client import _find_sdk_error + from sap_cloud_sdk.agentgateway.exceptions import AgentGatewayServerError + + err = AgentGatewayServerError("boom", error_code=-32602) + group = BaseExceptionGroup("outer", [BaseExceptionGroup("inner", [err])]) + found = _find_sdk_error(group) + assert found is err + assert found.error_code == -32602 + + def test_returns_none_without_sdk_error(self): + from sap_cloud_sdk.agentgateway.agw_client import _find_sdk_error + + group = BaseExceptionGroup("eg", [ValueError("x"), RuntimeError("y")]) + assert _find_sdk_error(group) is None diff --git a/tests/agentgateway/unit/test_compat.py b/tests/agentgateway/unit/test_compat.py index aa3830b8..f8a6f907 100644 --- a/tests/agentgateway/unit/test_compat.py +++ b/tests/agentgateway/unit/test_compat.py @@ -2,7 +2,11 @@ from types import SimpleNamespace +from mcp.shared.exceptions import McpError +from mcp.types import ErrorData + from sap_cloud_sdk.agentgateway._compat import ( + find_mcp_error, mcp_input_schema, mcp_is_error, mcp_server_name, @@ -87,3 +91,33 @@ def test_false_when_flag_false(self): def test_defaults_to_false_when_missing(self): """No error field on either name -> False.""" assert mcp_is_error(SimpleNamespace()) is False + + +class TestFindMcpError: + """Tests for find_mcp_error unwrapping.""" + + def test_returns_bare_mcp_error(self): + """A bare McpError is returned directly.""" + err = McpError(ErrorData(code=-32602, message="Unknown tool: foo")) + assert find_mcp_error(err) is err + + def test_finds_mcp_error_in_group(self): + """An McpError wrapped in a BaseExceptionGroup is found.""" + err = McpError(ErrorData(code=-32603, message="Internal Server Error")) + group = BaseExceptionGroup("eg", [err]) + assert find_mcp_error(group) is err + + def test_finds_mcp_error_in_nested_group(self): + """An McpError nested in a group within a group is found.""" + err = McpError(ErrorData(code=-32600, message="bad request")) + group = BaseExceptionGroup("outer", [BaseExceptionGroup("inner", [err])]) + assert find_mcp_error(group) is err + + def test_returns_none_without_mcp_error(self): + """A group without any McpError leaf returns None (defer to generic handling).""" + group = BaseExceptionGroup("eg", [ValueError("x"), RuntimeError("y")]) + assert find_mcp_error(group) is None + + def test_returns_none_for_unrelated_exception(self): + """A plain non-group, non-McpError exception returns None.""" + assert find_mcp_error(ValueError("x")) is None diff --git a/tests/agentgateway/unit/test_customer.py b/tests/agentgateway/unit/test_customer.py index 72e03efe..746d2f33 100644 --- a/tests/agentgateway/unit/test_customer.py +++ b/tests/agentgateway/unit/test_customer.py @@ -203,7 +203,10 @@ def test_loads_integration_dependencies_flat_format(self, tmp_path): result = load_customer_credentials(str(creds_file)) assert len(result.integration_dependencies) == 1 - assert result.integration_dependencies[0].ord_id == "sap.s4:apiResource:API_PRODUCT:v1" + assert ( + result.integration_dependencies[0].ord_id + == "sap.s4:apiResource:API_PRODUCT:v1" + ) assert result.integration_dependencies[0].global_tenant_id == "731473562" def test_raises_when_integration_dependencies_missing(self, tmp_path): @@ -550,9 +553,7 @@ async def mock_list_tools(*args, **kwargs): side_effect=mock_list_tools, ), ): - result = await get_mcp_tools_customer( - credentials, "system-token", 60.0 - ) + result = await get_mcp_tools_customer(credentials, "system-token", 60.0) # Should still return tools from server2 assert len(result) == 1 @@ -890,15 +891,14 @@ async def test_returns_empty_string_when_no_content(self, credentials, mock_tool mock_session.initialize = AsyncMock() mock_result = MagicMock() mock_result.content = [] + mock_result.is_error = False 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 - result = await call_mcp_tool_customer( - mock_tool, "auth-token", 60.0 - ) + result = await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) assert result == "" @@ -974,8 +974,150 @@ async def test_raises_when_tool_returns_is_error(self, credentials, mock_tool): mock_session_ctx.__aexit__ = AsyncMock(return_value=None) mock_session_class.return_value = mock_session_ctx + with pytest.raises(AgentGatewayServerError) as exc_info: + await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) + # Message carries the tool name and backend detail, but not the internal URL. + message = str(exc_info.value) + assert "create_order" in message + assert "change number test_sm doesn't exist" in message + assert mock_tool.url not in message + + @pytest.mark.asyncio + async def test_raises_server_error_with_code_on_mcp_error( + self, credentials, mock_tool + ): + """A JSON-RPC protocol error (McpError) surfaces as AgentGatewayServerError + carrying the server message and error_code.""" + from mcp.shared.exceptions import McpError + from mcp.types import ErrorData + + with ( + patch("httpx.AsyncClient") as mock_client_class, + patch( + "sap_cloud_sdk.agentgateway._customer.streamable_http_client" + ) as mock_stream, + patch( + "sap_cloud_sdk.agentgateway._customer.ClientSession" + ) as mock_session_class, + ): + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client_class.return_value = mock_client + + 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_session.call_tool = AsyncMock( + side_effect=McpError( + ErrorData(code=-32603, message="Internal Server Error") + ) + ) + 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) as exc_info: + await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) + assert str(exc_info.value) == "Internal Server Error" + assert exc_info.value.error_code == -32603 + + @pytest.mark.asyncio + async def test_raises_server_error_on_grouped_mcp_error( + self, credentials, mock_tool + ): + """A McpError wrapped in a BaseExceptionGroup (anyio task group) is still + surfaced as AgentGatewayServerError with the server message and code.""" + from mcp.shared.exceptions import McpError + from mcp.types import ErrorData + + grouped = BaseExceptionGroup( + "unhandled errors in a TaskGroup", + [McpError(ErrorData(code=-32602, message="Unknown tool: foo"))], + ) + + with ( + patch("httpx.AsyncClient") as mock_client_class, + patch( + "sap_cloud_sdk.agentgateway._customer.streamable_http_client" + ) as mock_stream, + patch( + "sap_cloud_sdk.agentgateway._customer.ClientSession" + ) as mock_session_class, + ): + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client_class.return_value = mock_client + + 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_session.call_tool = AsyncMock(side_effect=grouped) + 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) as exc_info: + await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) + assert str(exc_info.value) == "Unknown tool: foo" + assert exc_info.value.error_code == -32602 + + @pytest.mark.asyncio + async def test_raises_on_is_error_even_with_empty_content( + self, credentials, mock_tool + ): + """An isError result with empty content still raises (not swallowed as '').""" + with ( + patch("httpx.AsyncClient") as mock_client_class, + patch( + "sap_cloud_sdk.agentgateway._customer.streamable_http_client" + ) as mock_stream, + patch( + "sap_cloud_sdk.agentgateway._customer.ClientSession" + ) as mock_session_class, + ): + mock_client = AsyncMock() + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=None) + mock_client_class.return_value = mock_client + + 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 = [] + mock_result.is_error = 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="returned an error"): await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) + + # ============================================================ @@ -1083,7 +1225,10 @@ def test_loads_valid_credentials_from_env(self): assert result.certificate is None assert result.private_key is None assert len(result.integration_dependencies) == 1 - assert result.integration_dependencies[0].ord_id == "sap.example:apiResource:demo:v1" + assert ( + result.integration_dependencies[0].ord_id + == "sap.example:apiResource:demo:v1" + ) assert result.integration_dependencies[0].global_tenant_id == "123456" def test_raises_when_client_id_missing(self): diff --git a/tests/agentgateway/unit/test_lob.py b/tests/agentgateway/unit/test_lob.py index bd33665a..ff423193 100644 --- a/tests/agentgateway/unit/test_lob.py +++ b/tests/agentgateway/unit/test_lob.py @@ -1128,6 +1128,7 @@ async def test_returns_empty_string_when_no_content(self): mock_result = MagicMock() mock_result.content = [] + mock_result.is_error = False with ( patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, @@ -1204,6 +1205,134 @@ async def test_raises_when_tool_returns_is_error(self): mock_result.content[0].text = "change number test_sm doesn't exist" mock_result.is_error = True + with ( + patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + mock_http.return_value.__aenter__.return_value = AsyncMock() + 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) as exc_info: + await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + # Message carries the tool name and backend detail, but not the internal URL. + message = str(exc_info.value) + assert "test-tool" in message + assert "change number test_sm doesn't exist" in message + assert tool.url not in message + + @pytest.mark.asyncio + async def test_raises_server_error_with_code_on_mcp_error(self): + """A JSON-RPC protocol error (McpError) surfaces as AgentGatewayServerError + carrying the server message and error_code.""" + from mcp.shared.exceptions import McpError + from mcp.types import ErrorData + + 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("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + mock_http.return_value.__aenter__.return_value = AsyncMock() + 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( + side_effect=McpError( + ErrorData(code=-32603, message="Internal Server Error") + ) + ) + mock_session.return_value.__aenter__.return_value = mock_session_instance + + with pytest.raises(AgentGatewayServerError) as exc_info: + await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + assert str(exc_info.value) == "Internal Server Error" + assert exc_info.value.error_code == -32603 + + @pytest.mark.asyncio + async def test_raises_server_error_on_grouped_mcp_error(self): + """A McpError wrapped in a BaseExceptionGroup (anyio task group) is still + surfaced as AgentGatewayServerError with the server message and code.""" + from mcp.shared.exceptions import McpError + from mcp.types import ErrorData + + tool = MCPTool( + name="test-tool", + server_name="test-server", + description="Test tool", + input_schema={}, + url="https://example.com/mcp", + fragment_name="test-fragment", + ) + grouped = BaseExceptionGroup( + "unhandled errors in a TaskGroup", + [McpError(ErrorData(code=-32602, message="Unknown tool: foo"))], + ) + + with ( + patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, + patch( + "sap_cloud_sdk.agentgateway._lob.streamable_http_client" + ) as mock_stream, + patch("sap_cloud_sdk.agentgateway._lob.ClientSession") as mock_session, + ): + mock_http.return_value.__aenter__.return_value = AsyncMock() + 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(side_effect=grouped) + mock_session.return_value.__aenter__.return_value = mock_session_instance + + with pytest.raises(AgentGatewayServerError) as exc_info: + await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + assert str(exc_info.value) == "Unknown tool: foo" + assert exc_info.value.error_code == -32602 + + @pytest.mark.asyncio + async def test_raises_on_is_error_even_with_empty_content(self): + """An isError result with empty content still raises (not swallowed as '').""" + 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 = [] + mock_result.is_error = True + with ( patch("sap_cloud_sdk.agentgateway._lob.httpx.AsyncClient") as mock_http, patch( @@ -1224,6 +1353,8 @@ async def test_raises_when_tool_returns_is_error(self): with pytest.raises(AgentGatewayServerError, match="returned an error"): await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + + # ============================================================