Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
Expand Down
56 changes: 56 additions & 0 deletions src/sap_cloud_sdk/agentgateway/_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
49 changes: 43 additions & 6 deletions src/sap_cloud_sdk/agentgateway/_customer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Comment thread
NicoleMGomes marked this conversation as resolved.
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(
Expand Down
49 changes: 43 additions & 6 deletions src/sap_cloud_sdk/agentgateway/_lob.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand Down
56 changes: 56 additions & 0 deletions tests/agentgateway/unit/test_agw_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
50 changes: 44 additions & 6 deletions tests/agentgateway/unit/test_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand All @@ -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
Loading
Loading