Skip to content

Commit 93a0f09

Browse files
committed
Fix(mcp): drain pending command futures on actor failure; add Streamable HTTP test
- In `McpSessionActor._run_actor`, when an outer exception occurs (e.g., session startup failure), drain `_command_queue` and set exceptions on any pending command futures. This prevents callers from hanging indefinitely when the actor fails before processing queued commands. Draining is best‑effort and guarded. - Propagate the same exception to `_shutdown_future` when present. - Add unit test `test_run_actor_drains_queue_on_session_exception` using `StreamableHttpServerParams` with an invalid URL to trigger a startup failure and assert pending futures are failed. This hardens failure handling and makes client behavior predictable under connection errors.
1 parent f76f92d commit 93a0f09

2 files changed

Lines changed: 55 additions & 1 deletion

File tree

python/packages/autogen-ext/src/autogen_ext/tools/mcp/_actor.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,19 @@ async def _run_actor(self) -> None:
262262
except Exception as e:
263263
cmd["future"].set_exception(e)
264264
except Exception as e:
265+
try:
266+
while True:
267+
try:
268+
pending_cmd = self._command_queue.get_nowait()
269+
except asyncio.QueueEmpty:
270+
break
271+
fut = pending_cmd.get("future")
272+
if fut is not None and not fut.done():
273+
fut.set_exception(e)
274+
except Exception:
275+
# Best-effort draining only
276+
pass
277+
265278
if self._shutdown_future and not self._shutdown_future.done():
266279
self._shutdown_future.set_exception(e)
267280
else:

python/packages/autogen-ext/tests/tools/test_mcp_actor.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
RequestUsage,
1717
UserMessage,
1818
)
19-
from autogen_ext.tools.mcp import StdioServerParams
19+
from autogen_ext.tools.mcp import StdioServerParams, StreamableHttpServerParams
2020
from autogen_ext.tools.mcp._actor import (
2121
McpSessionActor,
2222
_parse_sampling_content, # pyright: ignore[reportPrivateUsage]
@@ -553,6 +553,47 @@ async def test_run_actor_session_exception() -> None:
553553
assert actor._actor_task is None # type: ignore[reportPrivateUsage]
554554

555555

556+
@pytest.mark.asyncio
557+
async def test_run_actor_drains_queue_on_session_exception() -> None:
558+
"""Ensure pending command futures are failed when session creation raises.
559+
560+
Uses StreamableHttpServerParams with an invalid URL to trigger failure,
561+
covering the queue-draining logic added in the referenced commit.
562+
"""
563+
# Use an invalid local URL/port to force immediate connection failure
564+
server_params = StreamableHttpServerParams(
565+
url="http://127.0.0.1:1/invalid", # very likely closed port
566+
timeout=0.1,
567+
sse_read_timeout=0.1,
568+
)
569+
actor = McpSessionActor(server_params)
570+
571+
# Prepare pending commands before the actor starts, so the outer except drains them
572+
fut1: asyncio.Future[Any] = asyncio.Future()
573+
fut2: asyncio.Future[Any] = asyncio.Future()
574+
await actor._command_queue.put({"type": "list_tools", "future": fut1}) # type: ignore[reportPrivateUsage]
575+
await actor._command_queue.put({"type": "call_tool", "name": "t", "args": {}, "future": fut2}) # type: ignore[reportPrivateUsage]
576+
577+
actor._active = True # type: ignore[reportPrivateUsage]
578+
task = asyncio.create_task(actor._run_actor()) # type: ignore[reportPrivateUsage]
579+
580+
# Wait for task to complete; it should handle the exception and drain the queue
581+
try:
582+
await asyncio.wait_for(task, timeout=2.0)
583+
except asyncio.TimeoutError:
584+
# If something goes wrong, ensure task cleanup for test stability
585+
task.cancel()
586+
with pytest.raises(asyncio.CancelledError):
587+
await task
588+
589+
# Verify futures were failed by the draining logic
590+
assert fut1.done()
591+
assert fut1.exception() is not None
592+
593+
assert fut2.done()
594+
assert fut2.exception() is not None
595+
596+
556597
@pytest.mark.asyncio
557598
async def test_run_actor_shutdown_future_exception() -> None:
558599
"""Test _run_actor sets exception on shutdown future when session fails."""

0 commit comments

Comments
 (0)