fix(mcp): don't close the MCP session on a missed heartbeat ping#41373
Open
RaccoonsNMangos wants to merge 1 commit into
Open
fix(mcp): don't close the MCP session on a missed heartbeat ping#41373RaccoonsNMangos wants to merge 1 commit into
RaccoonsNMangos wants to merge 1 commit into
Conversation
### Problem In HTTP/streamable transport, `startHeartbeat` pings the client and calls `server.close()` if the ping isn't answered within 5s, reaping the session and its browser context. Any client that doesn't promptly answer server-initiated pings — long-running tool calls, or aggregating proxies/gateways — gets `session terminated (404)`. Fixes microsoft/playwright-mcp#1293 (related: microsoft#982, microsoft#1140, microsoft#1307). Raising the 5s timeout doesn't help: `server.ping()` still rejects on the SDK's ~60s request timeout, which hits the same `.catch(() => server.close())`. The reap is caused by closing on a missed ping, not by the race-timeout value. ### Change Keep the heartbeat as a keepalive but no longer close the session on a missed ping. Dead transports are still cleaned up by `transport.onclose`, so a missed application-level ping no longer reaps a live session. The ping continues on the same 3s cadence in all cases. ### Alternatives considered - Making the timeout configurable (microsoft#982) — doesn't help clients that *never* answer pings. - Closing only after N consecutive misses — same limitation. Happy to adjust to whatever matches the heartbeat's intended design. Signed-off-by: RaccoonsNMangos <nathanzen@proton.me>
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
1 participant
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.
Problem
In HTTP/streamable transport,
startHeartbeatpings the client and callsserver.close()if the ping isn't answered within 5s, reaping the session and its browser context. Any client that doesn't promptly answer server-initiated pings — long-running tool calls, or aggregating proxies/gateways — getssession terminated (404). Fixes microsoft/playwright-mcp#1293 (related: #982, #1140, #1307).Raising the 5s timeout doesn't help:
server.ping()still rejects on the SDK's ~60s request timeout, which hits the same.catch(() => server.close()). The reap is caused by closing on a missed ping, not by the race-timeout value.Change
Keep the heartbeat as a keepalive but no longer close the session on a missed ping. Dead transports are still cleaned up by
transport.onclose, so a missed application-level ping no longer reaps a live session. The ping continues on the same 3s cadence in all cases.Alternatives considered