fix: handle stale MCP client reconnection to prevent persistent EOF errors#312
Open
w495604217 wants to merge 1 commit intohangwin:masterfrom
Open
fix: handle stale MCP client reconnection to prevent persistent EOF errors#312w495604217 wants to merge 1 commit intohangwin:masterfrom
w495604217 wants to merge 1 commit intohangwin:masterfrom
Conversation
…rrors When the HTTP transport to the Chrome extension drops (e.g., browser restart, extension reload, network interruption), the singleton mcpClient becomes permanently unusable. The ping() call throws but its exception is caught by the outer try-catch, which closes the client and sets it to null. However, subsequent calls may still fail because: 1. ping() exceptions were not caught independently, causing the entire ensureMcpClient() to fail silently (returning undefined) 2. handleToolCall() had no retry logic, so a single transient connection error would immediately return an error to the caller This fix: - Catches ping() failures independently and falls through to rebuild - Always explicitly closes the old client before creating a new one - Adds a forceNew parameter to ensureMcpClient() for explicit rebuilds - Adds one automatic retry in handleToolCall() for connection-related errors (EOF, closed, ECONNREFUSED, fetch failed, client is closing) - Re-throws connection errors instead of silently swallowing them Fixes persistent 'connection closed: EOF' errors reported by MCP clients (e.g., Antigravity, Claude Desktop) after browser/extension restarts.
|
nice |
|
Amazing! Athough: PR #301 (PyEL666) — Cleaner variant: just removes the caching from getMcpServer() itself, making it a factory. Only changes mcp-server.ts, no changes needed to server/index.ts. Open since Feb 22. |
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.
Problem
When the HTTP transport to the Chrome extension drops (e.g., browser restart, extension reload, network interruption), the singleton
mcpClientinmcp-server-stdio.tsbecomes permanently unusable.Root cause:
ensureMcpClient()catchesping()failures in the outer try-catch, which closes the client and sets it to null — but subsequent calls may still fail because the error is silently swallowed (the function returnsundefinedinstead of throwing). Additionally,handleToolCall()has no retry logic, so a single transient connection error immediately returns an error to the caller.This causes persistent
connection closed: EOF/client is closing: EOFerrors that can only be resolved by restarting the entire MCP server process.Fix
ping()failures are now caught separately and fall through to client rebuild, instead of being caught by the outer try-catchclose()the old client before creating a new oneforceNewparameter — allows callers to explicitly request a fresh clienthandleToolCall()— connection-related errors (EOF, closed, ECONNREFUSED, fetch failed, client is closing) trigger one automatic retry with a freshly built clientTesting
Verified with multiple MCP clients (Antigravity, custom Node.js clients) that the patched server correctly recovers from: