fix(mcp): probe the list signature instead of masking its TypeError (#104150)
_paginate_full_list wrapped the paginated list call in try/except TypeError to detect the mcp 1.x calling convention. The same except also caught TypeErrors raised INSIDE the modern list call — e.g. a server response decode failure — and retried with the legacy cursor= keyword, replacing the real error with a misleading 'unexpected keyword argument cursor' and making genuine MCP pagination failures undiagnosable. Probe list_method's signature instead (_list_method_accepts_params): the legacy cursor= fallback fires only when the method genuinely doesn't accept the mcp 2.0 params= keyword (or takes **kwargs), so a TypeError from inside the list call propagates to the caller. Regression tests: the decode TypeError surfaces and the legacy retry doesn't run; a genuinely 1.x-shaped method keeps using the cursor fallback.
This commit is contained in:
@@ -7,6 +7,8 @@ past page 1. Port of the invariant behind anomalyco/opencode#35439/#35500.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
@@ -68,3 +70,38 @@ class TestDiscoveryUsesPagination:
|
||||
|
||||
asyncio.run(server._discover_tools())
|
||||
assert [t.name for t in server._tools] == ["first", "second"]
|
||||
|
||||
|
||||
class TestTypeErrorPropagation:
|
||||
"""#104150: a TypeError raised INSIDE the list call (e.g. a server
|
||||
response decode failure) must propagate — it must not be caught by the
|
||||
mcp-2.0/1.x calling-convention fallback and replaced by a misleading
|
||||
'unexpected keyword argument cursor' error."""
|
||||
|
||||
def test_type_error_from_modern_list_call_propagates(self):
|
||||
calls = {"n": 0}
|
||||
|
||||
async def broken_list(**kwargs):
|
||||
calls["n"] += 1
|
||||
if calls["n"] == 1:
|
||||
return SimpleNamespace(tools=[_tool("first")], nextCursor="page-2")
|
||||
raise TypeError("server response decode failed")
|
||||
|
||||
with pytest.raises(TypeError, match="server response decode failed"):
|
||||
asyncio.run(_paginate_full_list(broken_list, "tools", "srv"))
|
||||
|
||||
assert calls["n"] == 2, "the legacy-cursor retry must not run"
|
||||
|
||||
def test_legacy_signature_still_uses_cursor_fallback(self):
|
||||
"""A genuinely 1.x-shaped method (no params kwarg) keeps working."""
|
||||
calls = {"n": 0}
|
||||
|
||||
async def legacy_list(cursor=None):
|
||||
calls["n"] += 1
|
||||
if calls["n"] == 1:
|
||||
return SimpleNamespace(tools=[_tool("first")], nextCursor="p2")
|
||||
return SimpleNamespace(tools=[_tool("second")], nextCursor=None)
|
||||
|
||||
items = asyncio.run(_paginate_full_list(legacy_list, "tools", "srv"))
|
||||
assert [t.name for t in items] == ["first", "second"]
|
||||
assert calls["n"] == 2
|
||||
|
||||
+29
-4
@@ -252,6 +252,26 @@ _JSONRPC_METHOD_NOT_FOUND = -32601
|
||||
_MCP_LIST_MAX_PAGES = 50
|
||||
|
||||
|
||||
def _list_method_accepts_params(list_method) -> bool:
|
||||
"""True when *list_method* accepts the mcp 2.0 ``params=`` keyword.
|
||||
|
||||
Probing the signature instead of catching TypeError around the call keeps
|
||||
a TypeError raised INSIDE the list call — e.g. a server response decode
|
||||
failure — propagating, rather than being replaced by a misleading
|
||||
legacy-cursor retry error (#104150).
|
||||
"""
|
||||
import inspect
|
||||
|
||||
try:
|
||||
sig = inspect.signature(list_method)
|
||||
except (TypeError, ValueError):
|
||||
return True # can't introspect — assume the modern convention
|
||||
return any(
|
||||
param.name == "params" or param.kind == inspect.Parameter.VAR_KEYWORD
|
||||
for param in sig.parameters.values()
|
||||
)
|
||||
|
||||
|
||||
async def _paginate_full_list(list_method, items_attr: str, server_name: str,
|
||||
cache_meta_out: Optional[dict] = None):
|
||||
"""Drain a paginated ``list_*`` call by following ``nextCursor``; ``cache_meta_out`` gets the
|
||||
@@ -263,12 +283,17 @@ async def _paginate_full_list(list_method, items_attr: str, server_name: str,
|
||||
result = await list_method()
|
||||
else:
|
||||
# mcp 2.0 takes params=PaginatedRequestParams, 1.x takes cursor=.
|
||||
try:
|
||||
# Signature-probed (see _list_method_accepts_params) instead of
|
||||
# try/except TypeError: a TypeError from inside the list call
|
||||
# must propagate, not trigger the legacy fallback (#104150).
|
||||
if _list_method_accepts_params(list_method):
|
||||
import mcp.types as _types # late: keeps the SDK import lazy
|
||||
_params_cls = getattr(_types, "PaginatedRequestParams", None)
|
||||
result = await (list_method(params=_params_cls(cursor=cursor)) if _params_cls is not None
|
||||
else list_method(cursor=cursor))
|
||||
except TypeError:
|
||||
if _params_cls is not None:
|
||||
result = await list_method(params=_params_cls(cursor=cursor))
|
||||
else:
|
||||
result = await list_method(cursor=cursor)
|
||||
else:
|
||||
result = await list_method(cursor=cursor)
|
||||
if cache_meta_out is not None and not items:
|
||||
for key, snake, camel in (("ttl_ms", "ttl_ms", "ttlMs"), ("cache_scope", "cache_scope", "cacheScope")):
|
||||
|
||||
Reference in New Issue
Block a user