-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Fix(mcp): drain pending command futures on McpSessionActor failure #7045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 @ekzhu . Could you please review this PR when you have time? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
Copilot
AI
Sep 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
Copilot uses AI. Check for mistakes.
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