Skip to content

mcp: cleanup transport after keepalive ping fails #360

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

Merged
merged 2 commits into from
Aug 22, 2025

Conversation

h9jiang
Copy link
Collaborator

@h9jiang h9jiang commented Aug 22, 2025

An onClose function is passed to the ServerSession and ClientSession to help cleanup resources from the caller.

The onClose function will be executed as part of the ServerSession and ClientSession closure.

Fixes #258

@@ -40,6 +40,8 @@ type StreamableHTTPHandler struct {
getServer func(*http.Request) *Server
opts StreamableHTTPOptions

onTransportDeletion func(sessionID string) // for testing only
Copy link
Collaborator Author

@h9jiang h9jiang Aug 22, 2025

Choose a reason for hiding this comment

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

I would prefer not to add this field only for testing purpose. But the test need some signal from the production indicating transportations have changed.

Let me know if you have any better solution to this.

An onClose function is passed to the ServerSession and ClientSession
to help cleanup resources from the caller.

The onClose function will be executed as part of the ServerSession
and ClientSession closure.

Fixes modelcontextprotocol#258
Copy link
Contributor

@findleyr findleyr left a comment

Choose a reason for hiding this comment

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

Thanks! This essentially LGTM, but a couple superficial comments.

- remove unnecessary lock for sending channel
- add todo to refactor the bind method
@h9jiang h9jiang requested a review from findleyr August 22, 2025 19:24
@findleyr findleyr merged commit fb69971 into modelcontextprotocol:main Aug 22, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

streamable session persists past client disconnect
2 participants