-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: implemented exception handling for client indefinite blocks #500
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
- propagated the error once value error is raised instead of send it to stream - added exception groups to handle value error as well as connection error etc with exception groups - added errors list to add the exception groups and convert it to normal exception and throw to client
* added exceptiongroup dependency in toml for handling exception group issue from python 3.10 to python 3.13 * added handle_exception function to handle the exceptions * updated the uv.lock
- changed the handle_exception functions implementation - replaced the Exception parameter with BaseExceptionGroup[Exception]
@jerome3o-anthropic @jspahrsummers @dsp-ant Adding here for PR review |
…er-has-incorrect-base-url
@Kludex Synced the PR. It’s up to date with main |
…er-has-incorrect-base-url
@Kludex @jerome3o-anthropic Synced the PR. It's up to date with main |
…er-has-incorrect-base-url
@Kludex Synced the PR. |
…er-has-incorrect-base-url
@dsp-ant @jerome3o-anthropic @Kludex Synced the PR with main |
…er-has-incorrect-base-url
@dsp-ant @jerome3o-anthropic @Kludex @ihrpr Synced the PR with main. Guys I've been following this PR and syncing for past 3 weeks I'm not getting any feedback and suggestions on this. I'd be happy to know the status of this PR |
…er-has-incorrect-base-url
@dsp-ant @jerome3o-anthropic @Kludex @ihrpr Synced the PR with main. |
…er-has-incorrect-base-url
@dsp-ant @jerome3o-anthropic @Kludex @ihrpr Synced the PR with main. |
I'm not sure why test cases are failing. @ihrpr |
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.
Thank you for your contribution!
The PR addresses a real issue that should be fixed, but the current implementation is overly complex and introduces unnecessary dependencies. Here's my recommendation:
- Simplify exception handling:
- Use specific exception types instead of catching all exceptions
- Preserve original exception types and tracebacks
- Consider using regular try/except instead of ExceptionGroups - Maintain MCP client configuration:
- Keep using create_mcp_http_client instead of raw httpx.AsyncClient
- If there's a reason for the change, document it - Improve code clarity:
- Reduce nesting levels
- Add comments explaining the exception flow
- Consider extracting some logic into helper functions - Add tests:
- The PR needs unit tests specifically for the error scenarios
- Test cases for URL mismatch, connection errors, etc. - Fix the handle_exception function:
- Either make it return a string or update the type signature
- Preserve the original exception context
This PR fixes #447 #642
Mismatch URL
Connection Error
Motivation and Context
This PR solves the indefinite blocking issue of the client by handling the exception thrown by url mismatch and also all the exceptions happens during connection:
How Has This Been Tested?
uv pip install /dist/mcp-1.6.1.dev12+70115b9-py3-none-any.whl
in the local servers.Breaking Changes
There are no breaking changes.
Types of changes
Checklist