From 5f27117ee5e069407b3a6d36aad27ee2576b20b7 Mon Sep 17 00:00:00 2001 From: Prashant Date: Tue, 20 Oct 2026 23:25:50 +0200 Subject: [PATCH 1/2] feat(agentgateway): support mcp 2.x and guard the malformed-result crash --- pyproject.toml | 2 +- src/sap_cloud_sdk/agentgateway/_compat.py | 56 ++++++++ src/sap_cloud_sdk/agentgateway/_customer.py | 49 ++++++- tests/agentgateway/unit/test_agw_client.py | 56 ++++++++ tests/agentgateway/unit/test_compat.py | 50 ++++++- tests/agentgateway/unit/test_customer.py | 147 ++++++++++++++++++-- tests/agentgateway/unit/test_lob.py | 13 +- uv.lock | 2 +- 8 files changed, 344 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8105b98c..09517c7c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ dependencies = [ "opentelemetry-instrumentation-sqlalchemy~=0.65b0", "opentelemetry-instrumentation-django~=0.65b0", "opentelemetry-instrumentation-flask~=0.65b0", - "mcp>=1.1.0,<2", + "mcp>=1.1.0,<3", "python-odata>=0.7.0", "cryptography>=48.0.1,<51", ] diff --git a/src/sap_cloud_sdk/agentgateway/_compat.py b/src/sap_cloud_sdk/agentgateway/_compat.py index e69de5ca..f809a1bd 100644 --- a/src/sap_cloud_sdk/agentgateway/_compat.py +++ b/src/sap_cloud_sdk/agentgateway/_compat.py @@ -8,6 +8,33 @@ from mcp.shared.exceptions import MCPError as McpError # ty: ignore[unresolved-import] +def make_mcp_error(code: int, message: str) -> "McpError": + """Construct an ``McpError`` with ``.error.code`` / ``.error.message`` set, across + mcp 1.x and 2.x. + + The constructor signature changed between majors: + + - **mcp 1.x:** ``McpError(ErrorData(code=..., message=...))`` — a single ``ErrorData``. + - **mcp 2.x:** ``MCPError(code, message, data=None)`` — positional ``code``/``message`` + (and the class was renamed ``McpError`` → ``MCPError``; see the import shim above). + + Both result in an exception exposing ``exc.error.code`` and ``exc.error.message``, + which is what the SDK reads when surfacing a protocol error as an + ``AgentGatewayServerError``. This helper hides the signature difference so callers + (and tests) do not need to branch on the installed mcp major. + """ + try: + # mcp 2.x: positional code + message. (ty type-checks against whichever mcp + # major is installed; the 1.x ErrorData signature makes this look wrong even + # though it is correct at runtime under v2, so suppress both shape diagnostics.) + return McpError(code, message) # ty: ignore[invalid-argument-type, too-many-positional-arguments] + except TypeError: + # mcp 1.x: a single ErrorData payload. + from mcp.types import ErrorData + + return McpError(ErrorData(code=code, message=message)) + + def mcp_server_name(init_result: Any) -> str | None: """Return ``serverInfo.name`` / ``server_info.name`` if present, else None.""" info = getattr(init_result, "server_info", None) @@ -64,3 +91,32 @@ def find_mcp_error(exc: BaseException) -> "McpError | None": if found is not None: return found return None + + +def find_malformed_result_error(exc: BaseException) -> "AttributeError | None": + """Return the first "result was None" ``AttributeError`` carried by ``exc``, else None. + + The mcp client's ``ClientSession.call_tool`` does an unguarded ``if not + result.isError:`` (mcp 1.x; renamed ``is_error`` in 2.x). When something upstream + yields ``None`` instead of a ``CallToolResult``, that line raises + ``AttributeError: 'NoneType' object has no attribute 'isError'`` *inside* + ``call_tool`` — before the SDK ever gets a value back to None-check. This matches + that AttributeError (by the ``isError`` / ``is_error`` attribute name) so the caller + can surface a clean error instead of leaking ``'NoneType' object has no attribute + 'isError'``. + + Walks ``BaseExceptionGroup`` trees (the mcp client runs inside anyio task groups), + returning the first matching ``AttributeError`` leaf. Returns None for any other + exception so the caller can defer to its generic handling. + """ + if isinstance(exc, AttributeError): + message = str(exc) + if "isError" in message or "is_error" in message: + return exc + return None + if isinstance(exc, BaseExceptionGroup): + for inner in exc.exceptions: + found = find_malformed_result_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 1f284996..7f362d68 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_malformed_result_error, find_mcp_error, mcp_input_schema, mcp_is_error, @@ -857,13 +858,49 @@ async def call_mcp_tool_customer( await session.initialize() try: result = await session.call_tool(tool.name, kwargs) - except (McpError, BaseExceptionGroup) as exc: + except (McpError, BaseExceptionGroup, AttributeError) 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 mcp_error is not None: + logger.warning( + "Tool '%s' on '%s' (server '%s') failed with MCP protocol " + "error: code=%s message=%s", + tool.name, + tool.url, + tool.server_name, + mcp_error.error.code, + mcp_error.error.message, + ) + raise AgentGatewayServerError( + mcp_error.error.message, error_code=mcp_error.error.code + ) from exc + + malformed = find_malformed_result_error(exc) + if malformed is not None: + logger.warning( + "Tool '%s' on '%s' (server '%s') returned a malformed or " + "empty result from the gateway; the MCP client received no " + "parseable CallToolResult (raw cause: %s: %s)", + tool.name, + tool.url, + tool.server_name, + type(malformed).__name__, + malformed, + ) + raise AgentGatewayServerError( + f"Tool '{tool.name}' on '{tool.url}' returned a malformed " + f"or empty result from the gateway" + ) from exc + + logger.error( + "Tool '%s' on '%s' (server '%s') raised an unhandled error " + "during invocation: %s: %s", + tool.name, + tool.url, + tool.server_name, + type(exc).__name__, + exc, + ) + raise if result is None: raise AgentGatewayServerError( diff --git a/tests/agentgateway/unit/test_agw_client.py b/tests/agentgateway/unit/test_agw_client.py index 3b2e6021..f84fdc65 100644 --- a/tests/agentgateway/unit/test_agw_client.py +++ b/tests/agentgateway/unit/test_agw_client.py @@ -980,6 +980,62 @@ async def test_customer_credentials_calls_customer_flow(self, mock_tool): mock_tool, "exchanged-token", 60.0 ) + @pytest.mark.asyncio + async def test_grouped_server_error_is_surfaced_bare(self, mock_tool): + """Regression: an AgentGatewayServerError raised by the customer flow but + re-wrapped in a BaseExceptionGroup (as anyio's task-group teardown does in + the real streamable_http_client / ClientSession path) must reach the caller + as a BARE AgentGatewayServerError — never as an ExceptionGroup, and never + leaking the opaque 'NoneType' object has no attribute 'isError' crash string. + + This pins the behavior of the outer `_find_sdk_error` unwrap in call_mcp_tool. + Without that unwrap the deployed-IBD malformed-result crash would surface as an + opaque ExceptionGroup to the agent again. + """ + from sap_cloud_sdk.agentgateway.exceptions import AgentGatewayServerError + + clean_error = AgentGatewayServerError( + "Tool 'test-tool' on 'https://example.com/mcp' returned a malformed " + "or empty result from the gateway" + ) + # mimic the guard raising the clean error, then anyio re-bundling it on the + # way out through the nested async-with task groups. + grouped = BaseExceptionGroup("unhandled errors in a TaskGroup", [clean_error]) + + with ( + patch( + "sap_cloud_sdk.agentgateway.agw_client.detect_customer_agent_credentials", + return_value="/path/to/credentials", + ), + patch( + "sap_cloud_sdk.agentgateway.agw_client.load_customer_credentials", + ) as mock_load, + patch( + "sap_cloud_sdk.agentgateway.agw_client.exchange_user_token", + return_value="exchanged-token", + ), + patch( + "sap_cloud_sdk.agentgateway.agw_client.call_mcp_tool_customer", + new_callable=AsyncMock, + side_effect=grouped, + ), + ): + mock_creds = MagicMock() + mock_creds.gateway_url = "https://agw.customer.com" + mock_load.return_value = mock_creds + + agw_client = create_client(tenant_subdomain="my-tenant") + + with pytest.raises(AgentGatewayServerError) as exc_info: + await agw_client.call_mcp_tool(tool=mock_tool, user_token="jwt-token") + + # bare, clean error — not the opaque crash, not an ExceptionGroup. + raised = exc_info.value + assert raised is clean_error + assert "malformed or empty result" in str(raised) + assert "NoneType" not in str(raised) + assert "isError" not in str(raised) + @pytest.mark.asyncio async def test_customer_flow_falls_back_to_system_token(self, mock_tool): """Customer flow falls back to system token when user_token is None.""" diff --git a/tests/agentgateway/unit/test_compat.py b/tests/agentgateway/unit/test_compat.py index f8a6f907..8689fdc2 100644 --- a/tests/agentgateway/unit/test_compat.py +++ b/tests/agentgateway/unit/test_compat.py @@ -2,11 +2,10 @@ from types import SimpleNamespace -from mcp.shared.exceptions import McpError -from mcp.types import ErrorData - from sap_cloud_sdk.agentgateway._compat import ( + find_malformed_result_error, find_mcp_error, + make_mcp_error, mcp_input_schema, mcp_is_error, mcp_server_name, @@ -98,18 +97,18 @@ class TestFindMcpError: def test_returns_bare_mcp_error(self): """A bare McpError is returned directly.""" - err = McpError(ErrorData(code=-32602, message="Unknown tool: foo")) + err = make_mcp_error(-32602, "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")) + err = make_mcp_error(-32603, "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")) + err = make_mcp_error(-32600, "bad request") group = BaseExceptionGroup("outer", [BaseExceptionGroup("inner", [err])]) assert find_mcp_error(group) is err @@ -121,3 +120,42 @@ def test_returns_none_without_mcp_error(self): 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 + + +class TestFindMalformedResultError: + """Tests for find_malformed_result_error — the isError/is_error-on-None crash.""" + + def test_returns_bare_isError_attributeerror(self): + """The mcp 1.x deref: AttributeError mentioning 'isError' is recognised.""" + err = AttributeError("'NoneType' object has no attribute 'isError'") + assert find_malformed_result_error(err) is err + + def test_returns_bare_is_error_attributeerror(self): + """The mcp 2.x deref: AttributeError mentioning 'is_error' is recognised.""" + err = AttributeError("'NoneType' object has no attribute 'is_error'") + assert find_malformed_result_error(err) is err + + def test_finds_attributeerror_in_group(self): + """Wrapped in a BaseExceptionGroup (anyio task group), it is still found.""" + err = AttributeError("'NoneType' object has no attribute 'isError'") + group = BaseExceptionGroup("eg", [err]) + assert find_malformed_result_error(group) is err + + def test_finds_attributeerror_in_nested_group(self): + """Found through nested BaseExceptionGroups.""" + err = AttributeError("'NoneType' object has no attribute 'isError'") + group = BaseExceptionGroup("outer", [BaseExceptionGroup("inner", [err])]) + assert find_malformed_result_error(group) is err + + def test_returns_none_for_unrelated_attributeerror(self): + """An AttributeError not about isError/is_error is NOT matched (not masked).""" + assert find_malformed_result_error(AttributeError("no attr 'bar'")) is None + + def test_returns_none_for_unrelated_exception(self): + """A non-AttributeError exception returns None.""" + assert find_malformed_result_error(ValueError("x")) is None + + def test_returns_none_without_match_in_group(self): + """A group with no matching AttributeError leaf returns None.""" + group = BaseExceptionGroup("eg", [ValueError("x"), RuntimeError("y")]) + assert find_malformed_result_error(group) is None diff --git a/tests/agentgateway/unit/test_customer.py b/tests/agentgateway/unit/test_customer.py index 746d2f33..36ebad16 100644 --- a/tests/agentgateway/unit/test_customer.py +++ b/tests/agentgateway/unit/test_customer.py @@ -988,8 +988,7 @@ async def test_raises_server_error_with_code_on_mcp_error( ): """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 + from sap_cloud_sdk.agentgateway._compat import make_mcp_error with ( patch("httpx.AsyncClient") as mock_client_class, @@ -1015,9 +1014,7 @@ async def test_raises_server_error_with_code_on_mcp_error( mock_session = AsyncMock() mock_session.initialize = AsyncMock() mock_session.call_tool = AsyncMock( - side_effect=McpError( - ErrorData(code=-32603, message="Internal Server Error") - ) + side_effect=make_mcp_error(-32603, "Internal Server Error") ) mock_session_ctx = AsyncMock() mock_session_ctx.__aenter__ = AsyncMock(return_value=mock_session) @@ -1035,12 +1032,11 @@ async def test_raises_server_error_on_grouped_mcp_error( ): """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 + from sap_cloud_sdk.agentgateway._compat import make_mcp_error grouped = BaseExceptionGroup( "unhandled errors in a TaskGroup", - [McpError(ErrorData(code=-32602, message="Unknown tool: foo"))], + [make_mcp_error(-32602, "Unknown tool: foo")], ) with ( @@ -1117,6 +1113,141 @@ async def test_raises_on_is_error_even_with_empty_content( with pytest.raises(AgentGatewayServerError, match="returned an error"): await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) + @pytest.mark.asyncio + async def test_maps_isError_attributeerror_to_server_error( + self, credentials, mock_tool + ): + """A bare ``AttributeError: 'NoneType' ... has no attribute 'isError'`` raised + *inside* mcp's call_tool (when it dereferences a None result) is translated into + a clean AgentGatewayServerError rather than leaking the opaque 'NoneType' message + to the agent.""" + 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=AttributeError( + "'NoneType' object has no attribute 'isError'" + ) + ) + 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) + message = str(exc_info.value) + assert "malformed or empty result" in message + assert "create_order" in message + # The raw mcp AttributeError string must NOT leak to the agent. + assert "NoneType" not in message + + @pytest.mark.asyncio + async def test_maps_grouped_isError_attributeerror_to_server_error( + self, credentials, mock_tool + ): + """The same malformed-result AttributeError, wrapped in a BaseExceptionGroup + (anyio task group — how it actually surfaces from the mcp client), is still + translated into a clean AgentGatewayServerError. Uses the ``is_error`` (mcp 2.x) + spelling to prove both spellings are recognised.""" + grouped = BaseExceptionGroup( + "unhandled errors in a TaskGroup", + [AttributeError("'NoneType' object has no attribute 'is_error'")], + ) + + 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) + message = str(exc_info.value) + assert "malformed or empty result" in message + assert "is_error" not in message + + @pytest.mark.asyncio + async def test_unrelated_attributeerror_is_reraised(self, credentials, mock_tool): + """An AttributeError unrelated to the isError/is_error dereference is NOT + swallowed — it is re-raised so genuine bugs are not masked as gateway errors.""" + 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=AttributeError("'Foo' object has no attribute 'bar'") + ) + 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(AttributeError, match="has no attribute 'bar'"): + await call_mcp_tool_customer(mock_tool, "auth-token", 60.0) + # ============================================================ diff --git a/tests/agentgateway/unit/test_lob.py b/tests/agentgateway/unit/test_lob.py index ff423193..41bff0ce 100644 --- a/tests/agentgateway/unit/test_lob.py +++ b/tests/agentgateway/unit/test_lob.py @@ -44,7 +44,6 @@ AgentGatewayServerError, MCPServerNotFoundError, ) -from sap_cloud_sdk.destination import ConsumptionLevel # Aliases for use in existing test assertions _LABEL_KEY = LABEL_KEY @@ -1235,8 +1234,7 @@ async def test_raises_when_tool_returns_is_error(self): 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 + from sap_cloud_sdk.agentgateway._compat import make_mcp_error tool = MCPTool( name="test-tool", @@ -1263,9 +1261,7 @@ async def test_raises_server_error_with_code_on_mcp_error(self): 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") - ) + side_effect=make_mcp_error(-32603, "Internal Server Error") ) mock_session.return_value.__aenter__.return_value = mock_session_instance @@ -1278,8 +1274,7 @@ async def test_raises_server_error_with_code_on_mcp_error(self): 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 + from sap_cloud_sdk.agentgateway._compat import make_mcp_error tool = MCPTool( name="test-tool", @@ -1291,7 +1286,7 @@ async def test_raises_server_error_on_grouped_mcp_error(self): ) grouped = BaseExceptionGroup( "unhandled errors in a TaskGroup", - [McpError(ErrorData(code=-32602, message="Unknown tool: foo"))], + [make_mcp_error(-32602, "Unknown tool: foo")], ) with ( diff --git a/uv.lock b/uv.lock index 6f4e24a0..f2ca3f0b 100644 --- a/uv.lock +++ b/uv.lock @@ -4451,7 +4451,7 @@ requires-dist = [ { name = "hatchling", specifier = "~=1.27.0" }, { name = "httpx", specifier = ">=0.27.0,<1" }, { name = "langgraph", marker = "extra == 'langgraph'", specifier = ">=1.0.0,<2" }, - { name = "mcp", specifier = ">=1.1.0,<2" }, + { name = "mcp", specifier = ">=1.1.0,<3" }, { name = "minio", specifier = ">=7.2.16,<7.2.21" }, { name = "opentelemetry-api", specifier = "~=1.44.0" }, { name = "opentelemetry-exporter-otlp-proto-grpc", specifier = "~=1.44.0" }, From 32fef5d58e9e39ddadf4964ccabc3fdb0ccd7f3f Mon Sep 17 00:00:00 2001 From: Prashant Date: Wed, 21 Oct 2026 10:30:45 +0200 Subject: [PATCH 2/2] fix(agentgateway): guard the malformed-result crash on the LoB flow too The AttributeError guard added for the customer flow (None-result dereference inside mcp's call_tool: 'NoneType' object has no attribute 'isError') was missing from call_mcp_tool_lob, which calls the identical mcp client the same way and carried the same latent crash. Its old except (McpError, BaseExceptionGroup) re-raised the bare group, leaking the opaque NoneType message to the agent. Mirror the customer-flow fix: widen the catch to include AttributeError, add a find_malformed_result_error branch mapping it to a clean AgentGatewayServerError, and log tool.name/url/server_name + exception type/message on every branch. Unrelated AttributeErrors are re-raised so genuine bugs are not masked. Adds three parity tests mirroring the customer-flow tests (bare + grouped isError/is_error AttributeError, and the unrelated-AttributeError re-raise). --- src/sap_cloud_sdk/agentgateway/_lob.py | 49 ++++++++-- tests/agentgateway/unit/test_lob.py | 124 +++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 6 deletions(-) diff --git a/src/sap_cloud_sdk/agentgateway/_lob.py b/src/sap_cloud_sdk/agentgateway/_lob.py index 30f880af..78e4e34c 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_malformed_result_error, find_mcp_error, mcp_input_schema, mcp_is_error, @@ -545,13 +546,49 @@ async def call_mcp_tool_lob( await session.initialize() try: result = await session.call_tool(tool.name, kwargs) - except (McpError, BaseExceptionGroup) as exc: + except (McpError, BaseExceptionGroup, AttributeError) 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 mcp_error is not None: + logger.warning( + "Tool '%s' on '%s' (server '%s') failed with MCP protocol " + "error: code=%s message=%s", + tool.name, + tool.url, + tool.server_name, + mcp_error.error.code, + mcp_error.error.message, + ) + raise AgentGatewayServerError( + mcp_error.error.message, error_code=mcp_error.error.code + ) from exc + + malformed = find_malformed_result_error(exc) + if malformed is not None: + logger.warning( + "Tool '%s' on '%s' (server '%s') returned a malformed or " + "empty result from the gateway; the MCP client received no " + "parseable CallToolResult (raw cause: %s: %s)", + tool.name, + tool.url, + tool.server_name, + type(malformed).__name__, + malformed, + ) + raise AgentGatewayServerError( + f"Tool '{tool.name}' on '{tool.url}' returned a malformed " + f"or empty result from the gateway" + ) from exc + + logger.error( + "Tool '%s' on '%s' (server '%s') raised an unhandled error " + "during invocation: %s: %s", + tool.name, + tool.url, + tool.server_name, + type(exc).__name__, + exc, + ) + raise if result is None: raise AgentGatewayServerError( f"Tool '{tool.name}' on '{tool.url}' returned None" diff --git a/tests/agentgateway/unit/test_lob.py b/tests/agentgateway/unit/test_lob.py index 41bff0ce..7e6a2345 100644 --- a/tests/agentgateway/unit/test_lob.py +++ b/tests/agentgateway/unit/test_lob.py @@ -1349,6 +1349,130 @@ async def test_raises_on_is_error_even_with_empty_content(self): with pytest.raises(AgentGatewayServerError, match="returned an error"): await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + @pytest.mark.asyncio + async def test_maps_isError_attributeerror_to_server_error(self): + """A bare ``AttributeError: 'NoneType' ... has no attribute 'isError'`` raised + *inside* mcp's call_tool (when it dereferences a None result) is translated into + a clean AgentGatewayServerError rather than leaking the opaque 'NoneType' message + to the agent. Mirrors the customer-flow guard — the LoB path shares the defect.""" + 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=AttributeError( + "'NoneType' object has no attribute 'isError'" + ) + ) + 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 = str(exc_info.value) + assert "malformed or empty result" in message + assert "test-tool" in message + # The raw mcp AttributeError string must NOT leak to the agent. + assert "NoneType" not in message + + @pytest.mark.asyncio + async def test_maps_grouped_isError_attributeerror_to_server_error(self): + """The same malformed-result AttributeError, wrapped in a BaseExceptionGroup + (anyio task group — how it actually surfaces from the mcp client), is still + translated into a clean AgentGatewayServerError. Uses the ``is_error`` (mcp 2.x) + spelling to prove both spellings are recognised.""" + 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", + [AttributeError("'NoneType' object has no attribute 'is_error'")], + ) + + 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) + message = str(exc_info.value) + assert "malformed or empty result" in message + assert "is_error" not in message + + @pytest.mark.asyncio + async def test_unrelated_attributeerror_is_reraised(self): + """An AttributeError unrelated to the isError/is_error dereference is NOT + swallowed — it is re-raised so genuine bugs are not masked as gateway errors.""" + 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=AttributeError("'Foo' object has no attribute 'bar'") + ) + mock_session.return_value.__aenter__.return_value = mock_session_instance + + with pytest.raises(AttributeError, match="has no attribute 'bar'"): + await call_mcp_tool_lob(tool, "user-auth-token", 60.0) + # ============================================================