fix: handle ExceptionGroup and CancelledError in MCP error flow #87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When an MCP connection fails, two issues were masking the real error: 1. Cleanup raises ExceptionGroup containing the HTTP error (Python 3.11+) 2. Session initialization raises CancelledError instead of the real error This fix addresses both: **ExceptionGroup handling:** - Catch BaseException in cleanup to handle ExceptionGroup - Extract and log individual exceptions from the group - Gracefully handle Python 3.10 (no ExceptionGroup) and 3.11+ - Log HTTPStatusError at debug level (expected during cleanup) **CancelledError handling:** - Catch BaseException in connection loop (not just Exception) - Properly convert CancelledError to ADCPConnectionError - Preserve KeyboardInterrupt and SystemExit behavior Now users see the actual connection error (e.g., "405 Method Not Allowed") instead of confusing cleanup error messages. Added test to verify ExceptionGroup handling works correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Addresses code review feedback to make error handling more robust: 1. Fix ExceptionGroup import - use built-in type directly, not from builtins 2. Add KeyboardInterrupt/SystemExit re-raise in cleanup path 3. Replace string-based HTTPStatusError check with isinstance 4. Update all references from BaseExceptionGroup to ExceptionGroup These changes improve type safety, handle critical interrupts correctly, and make the code more maintainable. All tests pass and mypy type checking succeeds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Ruff doesn't recognize sys.version_info guards for undefined names. Changed to try/except pattern which both ruff and mypy accept. Also removed unused sys import since we no longer need version checking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The test_cleanup_handles_exception_group test uses ExceptionGroup which doesn't exist in Python 3.10. Added skipif marker to skip this test on Python < 3.11. The functionality still works correctly in 3.10 (it gracefully handles the absence of ExceptionGroup), we just can't test it directly since we can't create an ExceptionGroup in the test. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixed error handling in MCP adapter to properly preserve original connection errors instead of masking them with cleanup exceptions. Added support for Python 3.11+ ExceptionGroup handling and fixed CancelledError leaking through from failed initialization.
Changes
Test Plan
🤖 Generated with Claude Code