Skip to content

Commit 794ddfa

Browse files
committed
fix: add connection readiness check and retry mechanism for MCP servers
- Add connection readiness check before marking server as connected - Implement retry logic with exponential backoff for initial tool list fetch - Add retry mechanism for tool calls to handle connection initialization race - Wait for server to be fully ready before showing 'Running' status This fixes the issue where the first MCP tool call fails with 'Connection closed' error after marketplace installation, ensuring servers are fully initialized before accepting tool calls. Fixes #8468
1 parent 8622d93 commit 794ddfa

File tree

1 file changed

+154
-20
lines changed

1 file changed

+154
-20
lines changed

src/services/mcp/McpHub.ts

Lines changed: 154 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -831,14 +831,33 @@ export class McpHub {
831831

832832
// Connect (this will automatically start the transport)
833833
await client.connect(transport)
834-
connection.server.status = "connected"
835-
connection.server.error = ""
836-
connection.server.instructions = client.getInstructions()
837834

838-
// Initial fetch of tools and resources
839-
connection.server.tools = await this.fetchToolsList(name, source)
840-
connection.server.resources = await this.fetchResourcesList(name, source)
841-
connection.server.resourceTemplates = await this.fetchResourceTemplatesList(name, source)
835+
// Wait for the connection to be fully established before marking as connected
836+
// This helps prevent "Connection closed" errors on first tool call after installation
837+
try {
838+
// Attempt to fetch tools list as a connection readiness check
839+
// If this fails, the connection isn't ready yet
840+
const tools = await this.fetchToolsListWithRetry(name, source, client)
841+
842+
// Connection is ready, mark as connected
843+
connection.server.status = "connected"
844+
connection.server.error = ""
845+
connection.server.instructions = client.getInstructions()
846+
connection.server.tools = tools
847+
848+
// Fetch remaining resources
849+
connection.server.resources = await this.fetchResourcesList(name, source)
850+
connection.server.resourceTemplates = await this.fetchResourceTemplatesList(name, source)
851+
} catch (readinessError) {
852+
// If we can't fetch tools, the connection isn't ready
853+
console.error(`Server ${name} failed readiness check:`, readinessError)
854+
connection.server.status = "disconnected"
855+
this.appendErrorMessage(
856+
connection,
857+
`Connection not ready: ${readinessError instanceof Error ? readinessError.message : String(readinessError)}`,
858+
)
859+
throw new Error(`MCP server ${name} failed to initialize properly. Please try restarting the server.`)
860+
}
842861
} catch (error) {
843862
// Update status with error
844863
const connection = this.findConnection(name, source)
@@ -902,6 +921,90 @@ export class McpHub {
902921
)
903922
}
904923

924+
/**
925+
* Fetches the tools list with retry logic to handle connection initialization race conditions.
926+
* This is particularly important for newly installed MCP servers from the marketplace.
927+
* @param serverName The name of the server
928+
* @param source The source of the server (global or project)
929+
* @param client Optional client to use directly (for initial connection check)
930+
* @returns Promise<McpTool[]> The list of tools
931+
*/
932+
private async fetchToolsListWithRetry(
933+
serverName: string,
934+
source?: "global" | "project",
935+
client?: Client,
936+
): Promise<McpTool[]> {
937+
const maxRetries = 3
938+
const baseDelay = 500 // Start with 500ms delay
939+
940+
for (let attempt = 0; attempt < maxRetries; attempt++) {
941+
try {
942+
// If client is provided, use it directly for the request
943+
if (client) {
944+
const response = await client.request({ method: "tools/list" }, ListToolsResultSchema)
945+
946+
// If successful, fetch the full tool configuration
947+
const connection = this.findConnection(serverName, source)
948+
if (connection) {
949+
const actualSource = connection.server.source || "global"
950+
let alwaysAllowConfig: string[] = []
951+
let disabledToolsList: string[] = []
952+
953+
try {
954+
let serverConfigData: Record<string, any> = {}
955+
if (actualSource === "project") {
956+
const projectMcpPath = await this.getProjectMcpPath()
957+
if (projectMcpPath) {
958+
const content = await fs.readFile(projectMcpPath, "utf-8")
959+
serverConfigData = JSON.parse(content)
960+
}
961+
} else {
962+
const configPath = await this.getMcpSettingsFilePath()
963+
const content = await fs.readFile(configPath, "utf-8")
964+
serverConfigData = JSON.parse(content)
965+
}
966+
if (serverConfigData) {
967+
alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || []
968+
disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || []
969+
}
970+
} catch (error) {
971+
console.error(`Failed to read tool configuration for ${serverName}:`, error)
972+
}
973+
974+
const tools = (response?.tools || []).map((tool) => ({
975+
...tool,
976+
alwaysAllow: alwaysAllowConfig.includes(tool.name),
977+
enabledForPrompt: !disabledToolsList.includes(tool.name),
978+
}))
979+
980+
return tools
981+
}
982+
return response?.tools || []
983+
} else {
984+
// Use the existing fetchToolsList method
985+
return await this.fetchToolsList(serverName, source)
986+
}
987+
} catch (error) {
988+
const isLastAttempt = attempt === maxRetries - 1
989+
990+
if (isLastAttempt) {
991+
console.error(`Failed to fetch tools for ${serverName} after ${maxRetries} attempts:`, error)
992+
throw error
993+
}
994+
995+
// Calculate exponential backoff delay
996+
const delay = baseDelay * Math.pow(2, attempt)
997+
console.log(
998+
`Retrying tools fetch for ${serverName} after ${delay}ms (attempt ${attempt + 1}/${maxRetries})`,
999+
)
1000+
await new Promise((resolve) => setTimeout(resolve, delay))
1001+
}
1002+
}
1003+
1004+
// This should never be reached due to the throw in the last attempt
1005+
return []
1006+
}
1007+
9051008
private async fetchToolsList(serverName: string, source?: "global" | "project"): Promise<McpTool[]> {
9061009
try {
9071010
// Use the helper method to find the connection
@@ -1601,19 +1704,50 @@ export class McpHub {
16011704
timeout = 60 * 1000
16021705
}
16031706

1604-
return await connection.client.request(
1605-
{
1606-
method: "tools/call",
1607-
params: {
1608-
name: toolName,
1609-
arguments: toolArguments,
1610-
},
1611-
},
1612-
CallToolResultSchema,
1613-
{
1614-
timeout,
1615-
},
1616-
)
1707+
// Add retry logic for tool calls to handle connection initialization race conditions
1708+
// This is particularly important for the first tool call after marketplace installation
1709+
const maxRetries = 2
1710+
const retryDelay = 1000 // 1 second delay between retries
1711+
1712+
for (let attempt = 0; attempt < maxRetries; attempt++) {
1713+
try {
1714+
return await connection.client.request(
1715+
{
1716+
method: "tools/call",
1717+
params: {
1718+
name: toolName,
1719+
arguments: toolArguments,
1720+
},
1721+
},
1722+
CallToolResultSchema,
1723+
{
1724+
timeout,
1725+
},
1726+
)
1727+
} catch (error: any) {
1728+
const isLastAttempt = attempt === maxRetries - 1
1729+
const isConnectionError = error?.code === -32000 && error?.message?.includes("Connection closed")
1730+
1731+
// Only retry on connection errors, not on other types of errors
1732+
if (!isConnectionError || isLastAttempt) {
1733+
throw error
1734+
}
1735+
1736+
console.log(
1737+
`Tool call failed with connection error, retrying in ${retryDelay}ms (attempt ${attempt + 1}/${maxRetries})`,
1738+
)
1739+
await new Promise((resolve) => setTimeout(resolve, retryDelay))
1740+
1741+
// Check if connection is still valid before retrying
1742+
const retryConnection = this.findConnection(serverName, source)
1743+
if (!retryConnection || retryConnection.type !== "connected") {
1744+
throw new Error(`Connection lost for server: ${serverName}`)
1745+
}
1746+
}
1747+
}
1748+
1749+
// This should never be reached due to the throw in the last attempt
1750+
throw new Error(`Failed to call tool ${toolName} on server ${serverName} after ${maxRetries} attempts`)
16171751
}
16181752

16191753
/**

0 commit comments

Comments
 (0)