mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(mcp): cycle-guard cause/context traversal in transport classifier
Builds on diffen77's #66547 (cherry-picked as the previous commits). Extend _is_session_expired_error's iterative traversal to follow __cause__/__context__ in addition to ExceptionGroup .exceptions — SDK wrappers often raise a generic RuntimeError *from* the message-less ClosedResourceError, leaving the transport signal reachable only via the chain. The identity-visited set guards chain cycles (handlers re-raising previously seen exceptions), and a bounded node budget (_EXC_TRAVERSAL_MAX_NODES) caps pathological acyclic graphs. Adds regression tests: cause/context chain detection, interruption precedence through chains, cyclic cause/context termination, and budget-bounded termination.
This commit is contained in:
parent
473545449b
commit
fbe086f7cc
2 changed files with 104 additions and 5 deletions
|
|
@ -178,6 +178,83 @@ def test_is_session_expired_rejects_empty_message():
|
|||
assert _is_session_expired_error(Exception()) is False
|
||||
|
||||
|
||||
def test_is_session_expired_follows_cause_chain():
|
||||
"""A transport close reachable only via ``__cause__`` must classify."""
|
||||
from anyio import ClosedResourceError
|
||||
from tools.mcp_tool import _is_session_expired_error
|
||||
|
||||
try:
|
||||
try:
|
||||
raise ClosedResourceError()
|
||||
except ClosedResourceError as inner:
|
||||
raise RuntimeError("MCP request failed") from inner
|
||||
except RuntimeError as exc:
|
||||
assert _is_session_expired_error(exc) is True
|
||||
|
||||
|
||||
def test_is_session_expired_follows_context_chain():
|
||||
"""Implicit ``__context__`` chaining must also be scanned."""
|
||||
from anyio import BrokenResourceError
|
||||
from tools.mcp_tool import _is_session_expired_error
|
||||
|
||||
try:
|
||||
try:
|
||||
raise BrokenResourceError()
|
||||
except BrokenResourceError:
|
||||
raise RuntimeError("while handling transport write")
|
||||
except RuntimeError as exc:
|
||||
assert _is_session_expired_error(exc) is True
|
||||
|
||||
|
||||
def test_is_session_expired_interruption_in_cause_chain_wins():
|
||||
"""User cancellation buried in the chain overrides transport signals."""
|
||||
from anyio import ClosedResourceError
|
||||
from tools.mcp_tool import _is_session_expired_error
|
||||
|
||||
root = InterruptedError("cancel")
|
||||
mid = ClosedResourceError()
|
||||
mid.__cause__ = root
|
||||
top = RuntimeError("transport is closed")
|
||||
top.__cause__ = mid
|
||||
assert _is_session_expired_error(top) is False
|
||||
|
||||
|
||||
def test_is_session_expired_handles_cyclic_cause_context_chain():
|
||||
"""Cycles through __cause__/__context__ must terminate (visited set)."""
|
||||
from tools.mcp_tool import _is_session_expired_error
|
||||
|
||||
a = RuntimeError("a")
|
||||
b = RuntimeError("b")
|
||||
a.__cause__ = b
|
||||
b.__context__ = a # cycle back through the other link
|
||||
assert _is_session_expired_error(a) is False
|
||||
|
||||
from anyio import ClosedResourceError
|
||||
|
||||
c = RuntimeError("c")
|
||||
d = ClosedResourceError()
|
||||
c.__cause__ = d
|
||||
d.__context__ = c # cycle, but transport error is reachable
|
||||
assert _is_session_expired_error(c) is True
|
||||
|
||||
|
||||
def test_is_session_expired_traversal_is_budget_bounded():
|
||||
"""Pathologically long chains stop at the node budget without spinning."""
|
||||
import tools.mcp_tool as mcp_mod
|
||||
from tools.mcp_tool import _is_session_expired_error
|
||||
|
||||
exc: BaseException = RuntimeError("leaf")
|
||||
for i in range(mcp_mod._EXC_TRAVERSAL_MAX_NODES * 2):
|
||||
wrapper = RuntimeError(f"layer {i}")
|
||||
wrapper.__cause__ = exc
|
||||
exc = wrapper
|
||||
|
||||
# Terminates quickly and classifies false (no transport signal within
|
||||
# budget). The exact outcome past the budget is unspecified; the
|
||||
# invariant under test is termination.
|
||||
assert _is_session_expired_error(exc) is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Handler integration — verify the recovery plumbing wires end-to-end
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -3899,6 +3899,15 @@ _SESSION_EXPIRED_MARKERS: tuple = (
|
|||
"end of file",
|
||||
)
|
||||
|
||||
# Upper bound on exception-graph nodes inspected by
|
||||
# ``_is_session_expired_error``. The visited-identity set already breaks
|
||||
# cycles across ``exceptions`` / ``__cause__`` / ``__context__``; the
|
||||
# budget additionally bounds pathological acyclic graphs (e.g. deeply
|
||||
# chained retries) so classification always terminates promptly. Kept
|
||||
# comfortably above ``sys.getrecursionlimit()`` so legitimately deep
|
||||
# wrapper stacks (task-group nesting) are still fully scanned.
|
||||
_EXC_TRAVERSAL_MAX_NODES = 10_000
|
||||
|
||||
|
||||
def _is_session_expired_error(exc: BaseException) -> bool:
|
||||
"""Return True if ``exc`` looks like an MCP transport session expiry.
|
||||
|
|
@ -3929,18 +3938,29 @@ def _is_session_expired_error(exc: BaseException) -> bool:
|
|||
transport_error_types = ()
|
||||
|
||||
# ExceptionGroup trees can be arbitrarily deep or even cyclic when custom
|
||||
# exceptions expose ``exceptions``. Traverse once, iteratively, and inspect
|
||||
# every reachable node so user interruption always overrides transport
|
||||
# markers or types found elsewhere in the graph.
|
||||
stack = [exc]
|
||||
# exceptions expose ``exceptions``, and chained exceptions
|
||||
# (``raise X from Y`` / implicit ``__context__``) can likewise form
|
||||
# cycles when handlers re-raise previously seen exceptions. Traverse
|
||||
# once, iteratively, with an identity-visited set AND a bounded node
|
||||
# budget so classification can never spin, and inspect every reachable
|
||||
# node so user interruption always overrides transport markers or types
|
||||
# found elsewhere in the graph. The chain traversal matters for real
|
||||
# failures: SDK wrappers often raise a generic RuntimeError *from* the
|
||||
# message-less ClosedResourceError, leaving the transport signal only
|
||||
# reachable via ``__cause__``.
|
||||
stack: "list[BaseException | None]" = [exc]
|
||||
seen: set[int] = set()
|
||||
transport_error_found = False
|
||||
while stack:
|
||||
budget = _EXC_TRAVERSAL_MAX_NODES
|
||||
while stack and budget > 0:
|
||||
current = stack.pop()
|
||||
if current is None:
|
||||
continue
|
||||
identity = id(current)
|
||||
if identity in seen:
|
||||
continue
|
||||
seen.add(identity)
|
||||
budget -= 1
|
||||
|
||||
if isinstance(current, InterruptedError):
|
||||
return False
|
||||
|
|
@ -3956,6 +3976,8 @@ def _is_session_expired_error(exc: BaseException) -> bool:
|
|||
transport_error_found = True
|
||||
|
||||
stack.extend(getattr(current, "exceptions", ()))
|
||||
stack.append(getattr(current, "__cause__", None))
|
||||
stack.append(getattr(current, "__context__", None))
|
||||
|
||||
return transport_error_found
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue