-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(731): StreamableHTTPClientTransport Fails to Reconnect on Non-Resumable Streams #732
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.
Thank you for working on this fix!
While the fix correctly addresses the reconnection issue for non-resumable streams, it introduces a problem with POST-initiated SSE streams that needs to be addressed.
The current implementation treats all SSE stream reconnections the same way, but there are two distinct types:
- GET-initiated SSE streams (for server notifications)
- POST-initiated SSE streams (for streaming responses to specific requests)
When a POST-initiated SSE stream disconnects and reconnects, it incorrectly attempts to reconnect via GET, causing:
- The original tool call or request context is completely lost
- Client expects tool-specific responses but receives general server notifications
- Long-running tools never receive their completion events
We can merge this change for GET initialised streams, but we need to filter them and make sure the logic does not apply to the POST requests
Thanks @ihrpr! I've proceeded with the distinction between POST and GET initialized streams. |
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
oh, looks like there is merge conflict now, please can you resolve it and we can merge? |
Removed the check that prevented reconnection attempts if the streamable http transport was not configured with an event source to supply an event id.
Motivation and Context
Cloud hosting usually cleans up connections very frequently (30 secs on gcloud) and the MCP client needs to be durable enough to restart the transport each time.
How Has This Been Tested?
Yes, tested on my MCP client implementation and it works. Previously, when the request was terminated by google load balancer, the client would emit an error message that the connection was terminated and then notifications would stop working. Now the transport reconnects and server sent notifications keep arriving.
Breaking Changes
None
Types of changes
Checklist