Fix issue #428: ClientSession.initialize gets stuck if the MCP server process exits#434
Closed
AshwanthramKL wants to merge 1 commit intomodelcontextprotocol:mainfrom
Closed
Conversation
b3ac56d to
2a395cd
Compare
…ck if the MCP server process exits
2a395cd to
728ec85
Compare
ihrpr
requested changes
May 28, 2025
Contributor
ihrpr
left a comment
There was a problem hiding this comment.
Thank you for working on this issue! The problem you're addressing is real and needs fixing. However, I'd like to suggest an alternative approach that addresses the root cause more directly.
We need to correctly handle scenarios like missing API keys or configuration and fail with a clear message, in other words we should validate the process started successfully before yielding, instead of monitoring inside the context
Contributor
|
Just checked and #333 is fixing the same problem and is already merged. The only fix is needed for the test to work is handling ProcessLookupError error. Thank you for contributing to MCP Python SDK! |
9 tasks
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
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.
This PR fixes an issue where ClientSession.initialize gets stuck indefinitely if the MCP server process exits unexpectedly.
Motivation and Context
When using
stdio_clientto connect to an MCP server, if the server process exits unexpectedly (e.g., due to missing API keys or configuration), the client hangs indefinitely without raising an appropriate exception. This makes for a poor developer experience and can be frustrating to debug.How Has This Been Tested?
Added a test case that verifies the fix works correctly by creating a server that exits during initialization and checking that the appropriate exception is raised. The test ensures that the client properly detects when the server process exits and raises a
ProcessTerminatedEarlyErrorinstead of hanging indefinitely.Breaking Changes
None. This change only improves error handling and doesn't modify any existing APIs.
Types of changes
Checklist
Additional context
The solution adds a process monitoring task that detects when the server process exits unexpectedly and raises a
ProcessTerminatedEarlyErrorwith a descriptive message. It also ensures proper cleanup of resources to prevent leaks.Specifically:
ProcessTerminatedEarlyErrorexception classFixes #428