Fix(mcp): drain pending command futures on McpSessionActor failure - #7045
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7045 +/- ##
==========================================
+ Coverage 80.99% 81.05% +0.05%
==========================================
Files 238 238
Lines 18291 18302 +11
==========================================
+ Hits 14815 14834 +19
+ Misses 3476 3468 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- 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. This hardens failure handling and makes client behavior predictable under connection errors.
93a0f09 to
6e8385b
Compare
|
Hi Eric Zhu (@ekzhu) . Could you please review this PR when you have time? |
There was a problem hiding this comment.
Pull Request Overview
This PR improves error handling in the MCP (Model Context Protocol) session actor by ensuring pending command futures are properly failed when the actor crashes during startup. The change prevents clients from hanging indefinitely when connection failures occur.
Key changes:
- Added queue draining logic to fail pending command futures when
McpSessionActor._run_actorencounters exceptions - Implemented best-effort error handling that swallows internal draining errors to maintain robustness
- Added comprehensive test coverage for the new failure scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
python/packages/autogen-ext/src/autogen_ext/tools/mcp/_actor.py |
Added queue draining logic in the outer exception handler to fail pending command futures |
python/packages/autogen-ext/tests/tools/test_mcp_actor.py |
Added two test cases covering queue draining behavior and error swallowing during failure scenarios |
|
|
||
| @pytest.mark.asyncio | ||
| async def test_run_actor_draining_swallows_internal_errors() -> None: | ||
| """draining errors during exception handling are swallowed. |
There was a problem hiding this comment.
The docstring should start with a capital letter: 'Draining errors during exception handling are swallowed.'
| """draining errors during exception handling are swallowed. | |
| """Draining errors during exception handling are swallowed. |
Why are these changes needed?
McpSessionActor._run_actor, when an outer exception occurs (e.g., session startup failure), drain_command_queueand 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.This hardens failure handling and makes client behavior predictable under connection errors.
Related issue number
None
Checks