feat(agentgateway): support mcp 2.x and guard the malformed-result crash (#418)

Author: prashantrakheja

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",
 ]

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

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(

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"

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."""

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