Skip to content

Conversation

@raben
Copy link

@raben raben commented Dec 5, 2025

Link to Issue or Description of Change

1. Link to an existing issue:

2. Description:

Problem:
When an MCP server returns an HTTP error (e.g., 401, 403), the MCP SDK uses anyio cancel scopes internally, which raise CancelledError. Since CancelledError is a BaseException (not Exception) in Python 3.8+, the existing except Exception block in MCPSessionManager.create_session() does not catch it.

This causes the error to propagate uncaught, leading to:

  • Application hangs
  • "ASGI callable returned without completing response" errors in web frameworks
  • Inability to handle MCP connection failures gracefully

Solution:
This fix explicitly catches asyncio.CancelledError before the general Exception handler and converts it to a ConnectionError with a descriptive message. This allows:

  • Proper error handling and cleanup
  • Exit stack cleanup to prevent resource leaks
  • Graceful error propagation that applications can handle

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
tests/unittests/tools/mcp_tool/test_mcp_session_manager.py::TestMCPSessionManager::test_create_session_cancelled_error PASSED

Manual End-to-End (E2E) Tests:

Tested with an MCP server returning HTTP 403 Forbidden:

  1. Before fix: Application hangs with "ASGI callable returned without completing response"
  2. After fix: ConnectionError is raised with descriptive message, allowing proper error handling

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

This issue affects multiple users as described in #3708. The root cause is that asyncio.CancelledError inherits from BaseException (not Exception) in Python 3.8+, so it escapes the existing exception handler.

@google-cla
Copy link

google-cla bot commented Dec 5, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @raben, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical issue in the MCPSessionManager where asyncio.CancelledError was not being properly handled during session creation, particularly when MCP servers returned HTTP errors. By explicitly catching this BaseException and converting it to a ConnectionError, the change ensures that applications can gracefully manage connection failures, prevent hangs, and perform necessary resource cleanup, significantly improving the robustness of MCP session management.

Highlights

  • Problem Addressed: When an MCP server returns an HTTP error (e.g., 401, 403), the MCP SDK's internal anyio cancel scopes raise CancelledError. Since CancelledError is a BaseException (not Exception) in Python 3.8+, the existing except Exception block in MCPSessionManager.create_session() does not catch it, leading to application hangs and ungraceful connection failure handling.
  • Solution Implemented: The fix explicitly catches asyncio.CancelledError before the general Exception handler. It converts the CancelledError into a ConnectionError with a descriptive message, ensuring proper error handling, resource cleanup via exit_stack, and graceful error propagation that applications can manage.
  • New Unit Test: A new unit test, test_create_session_cancelled_error, has been added to test_mcp_session_manager.py to verify that CancelledError is correctly caught and converted to ConnectionError with proper cleanup during session creation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@adk-bot adk-bot added the mcp [Component] Issues about MCP support label Dec 5, 2025
@adk-bot
Copy link
Collaborator

adk-bot commented Dec 5, 2025

Response from ADK Triaging Agent

Hello @raben, thank you for your contribution!

Before we can merge your pull request, you'll need to sign our Contributor License Agreement (CLA). You can find more information and sign the CLA at https://cla.developers.google.com/.

Thanks!

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is a well-executed fix for an important issue. The pull request correctly identifies that asyncio.CancelledError was not being handled because it inherits from BaseException, and implements a clear solution by adding a specific except block. The inclusion of a new unit test to validate the fix is excellent. My review includes a few minor suggestions to improve code clarity and maintainability by removing a redundant conditional check and some unused code in the new test.

Comment on lines 374 to 380
if exit_stack:
try:
await exit_stack.aclose()
except Exception as exit_stack_error:
logger.warning(
'Error during cancelled session cleanup: %s', exit_stack_error
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The if exit_stack: check appears to be redundant. The exit_stack variable is initialized unconditionally on line 331 with exit_stack = AsyncExitStack(), so it will never be falsy within this try...except block. You can simplify the code by removing this conditional check. This also applies to the similar block in the except Exception handler starting on line 388.

          try:
            await exit_stack.aclose()
          except Exception as exit_stack_error:
            logger.warning(
                'Error during cancelled session cleanup: %s', exit_stack_error
            )

Comment on lines 296 to 299
@patch("google.adk.tools.mcp_tool.mcp_session_manager.ClientSession")
async def test_create_session_cancelled_error(
self, mock_session_class, mock_exit_stack_class, mock_stdio
):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The patch for ClientSession and its corresponding argument mock_session_class are not used in this test. The CancelledError is raised before ClientSession is ever instantiated. To keep the test clean and focused on its specific purpose, you can remove this unnecessary patch and argument.

  async def test_create_session_cancelled_error(
      self, mock_exit_stack_class, mock_stdio
  ):

"""
manager = MCPSessionManager(self.mock_stdio_connection_params)

mock_session = MockClientSession()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The mock_session variable is initialized but never used in this test. It can be safely removed to improve code clarity.

When an MCP server returns an HTTP error (e.g., 401, 403), the MCP SDK
uses anyio cancel scopes internally, which raise CancelledError. Since
CancelledError is a BaseException (not Exception) in Python 3.8+, the
existing `except Exception` block does not catch it.

This causes the error to propagate uncaught, leading to:
- Application hangs
- ASGI callable returned without completing response errors
- Inability to handle MCP connection failures gracefully

This fix explicitly catches asyncio.CancelledError and converts it to
a ConnectionError with a descriptive message, allowing proper error
handling and cleanup.

Fixes google#3708
@raben raben force-pushed the fix/mcp-session-cancelled-error-handling branch from 7d5be08 to 5787554 Compare December 5, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mcp [Component] Issues about MCP support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug Report: MCP Module Raises CancelledError and Prevents Custom Error Handling

2 participants