-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: Enhance error handling for closed and broken resources in retry … #2929
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
base: main
Are you sure you want to change the base?
Conversation
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.
Summary of Changes
Hello @utkarshkingh, 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 significantly improves the robustness of the MCPSessionManager by enhancing its error handling capabilities. It ensures that the system can more effectively recover from various network-related disruptions, specifically by treating broken resources similarly to closed ones, leading to more reliable session management and clearer error diagnostics.
Highlights
- Enhanced Retry Mechanism: The retry_on_closed_resource decorator now handles anyio.BrokenResourceError in addition to anyio.ClosedResourceError, ensuring operations are retried for a wider range of connection issues.
- Improved Session Creation Error Handling: The create_session method's cleanup logic has been updated to gracefully handle anyio.BrokenResourceError and anyio.ClosedResourceError during exit_stack closure, preventing secondary errors from obscuring the primary connection failure.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
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 issue 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
-
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. ↩
Response from ADK Triaging Agent Hello @utkarshkingh, thank you for your contribution! It's great to see that you've included a "How to Test" section. To help reviewers verify the fix, could you please also provide the logs or screenshots from your manual tests, as suggested in our contribution guidelines? This will help speed up the review process. Thanks! |
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.
Code Review
This pull request enhances error handling in the MCPSessionManager
by also catching anyio.BrokenResourceError
in the retry decorator and during session creation cleanup. The changes are generally good and improve the resilience of the session manager. However, I've identified a significant issue with the simple retry logic in the retry_on_closed_resource
decorator. When applied to non-idempotent operations, such as the generic call_tool
method, it can lead to unintended side effects. I've left a detailed comment explaining the risk and suggesting ways to mitigate it.
Description #2769 resolved
This PR enhances the error handling in the
MCPSessionManager
to make it more resilient to connection failures. The changes focus on handlinganyio.BrokenResourceError
in addition toanyio.ClosedResourceError
, ensuring the session manager can gracefully recover from a wider range of network-related issues.Changelog
Improved
retry_on_closed_resource
decorator:anyio.BrokenResourceError
alongsideanyio.ClosedResourceError
.Enhanced exception handling in
create_session
:create_session
method has been updated to specifically catchanyio.BrokenResourceError
andanyio.ClosedResourceError
if session creation fails.How to Test
anyio.BrokenResourceError
during an MCP session operation.retry_on_closed_resource
decorator catches the exception and retries the operation.create_session
and confirm that the original exception is raised without being obscured by a cleanup error.