Skip to content

Commit 35c7ee5

Browse files
committed
fix: multiple fixes for remote mcp error and timeout
1 parent b444646 commit 35c7ee5

File tree

4 files changed

+69
-19
lines changed

4 files changed

+69
-19
lines changed

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,7 @@ export class McpEventHandler {
786786
}
787787

788788
this.#currentEditingServerName = undefined
789+
this.#serverNameBeforeUpdate = undefined
789790

790791
// need to check server state now, as there is possibility of error during server initialization
791792
const serverStatusError = this.#getServerStatusError(serverName)
@@ -1238,7 +1239,7 @@ export class McpEventHandler {
12381239
const serverConfig = McpManager.instance.getAllServerConfigs().get(serverName)
12391240
if (serverConfig) {
12401241
// Emit server initialize event after permission change
1241-
const transportType = serverConfig.command ? 'stdio' : 'http'
1242+
const transportType = serverConfig.command?.trim() ? 'stdio' : 'http'
12421243
this.#telemetryController?.emitMCPServerInitializeEvent({
12431244
source: 'updatePermission',
12441245
command: transportType === 'stdio' ? serverConfig.command : undefined,
@@ -1319,7 +1320,7 @@ export class McpEventHandler {
13191320
enabled: enabled,
13201321
numTools: mcpManager.getAllToolsWithPermissions(serverName).length,
13211322
scope: config.__configPath__ === globalAgentPath ? 'global' : 'workspace',
1322-
transportType: 'stdio',
1323+
transportType: transportType,
13231324
languageServerVersion: this.#features.runtime.serverInfo.version,
13241325
})
13251326
}

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ export class McpManager {
292292
this.features.logging.debug(`MCP: initializing server [${serverName}]`)
293293

294294
const client = new Client({
295-
name: `mcp-client-${serverName}`,
295+
name: `q-chat-plugin`, // Do not use server name in the client name to avoid polluting builder-mcp metrics
296296
version: '1.0.0',
297297
})
298298

@@ -662,7 +662,7 @@ export class McpManager {
662662
disabled: cfg.disabled ?? false,
663663
}
664664
// Only add timeout to agent config if it's not 0
665-
if (cfg.timeout !== 0) {
665+
if (cfg.timeout !== undefined) {
666666
serverConfig.timeout = cfg.timeout
667667
}
668668
if (cfg.args && cfg.args.length > 0) {
@@ -1281,11 +1281,21 @@ export class McpManager {
12811281
private handleError(server: string | undefined, err: unknown) {
12821282
const msg = err instanceof Error ? err.message : String(err)
12831283

1284-
this.features.logging.error(`MCP ERROR${server ? ` [${server}]` : ''}: ${msg}`)
1284+
const isBenignSseDisconnect =
1285+
/SSE error:\s*TypeError:\s*terminated:\s*Body Timeout Error/i.test(msg) ||
1286+
/TypeError:\s*terminated:\s*Body Timeout Error/i.test(msg) ||
1287+
/TypeError:\s*terminated:\s*other side closed/i.test(msg) ||
1288+
/ECONNRESET|ENETRESET|EPIPE/i.test(msg)
12851289

1286-
if (server) {
1287-
this.setState(server, McpServerStatus.FAILED, 0, msg)
1288-
this.emitToolsChanged(server)
1290+
if (isBenignSseDisconnect) {
1291+
this.features.logging.debug(`MCP SSE idle timeout${server ? ` [${server}]` : ''}: ${msg}`)
1292+
} else {
1293+
// default path for real errors
1294+
this.features.logging.error(`MCP ERROR${server ? ` [${server}]` : ''}: ${msg}`)
1295+
if (server) {
1296+
this.setState(server, McpServerStatus.FAILED, 0, msg)
1297+
this.emitToolsChanged(server)
1298+
}
12891299
}
12901300
}
12911301

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpOauthClient.ts

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* All Rights Reserved. SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import fetch, { type RequestInit } from 'node-fetch'
6+
import type { RequestInit } from 'node-fetch'
77
import * as crypto from 'crypto'
88
import * as path from 'path'
99
import { spawn } from 'child_process'
@@ -56,11 +56,16 @@ export class OAuthClient {
5656
const port = Number(new URL(savedReg.redirect_uri).port)
5757
server = http.createServer()
5858
try {
59-
await new Promise<void>(res => server.listen(port, '127.0.0.1', res))
59+
await this.listen(server, port)
6060
redirectUri = savedReg.redirect_uri
6161
this.logger.info(`OAuth: reusing redirect URI ${redirectUri}`)
6262
} catch (e: any) {
6363
if (e.code === 'EADDRINUSE') {
64+
try {
65+
server.close()
66+
} catch {
67+
/* ignore */
68+
}
6469
this.logger.warn(`Port ${port} in use; falling back to new random port`)
6570
;({ server, redirectUri } = await this.buildCallbackServer())
6671
this.logger.info(`OAuth: new redirect URI ${redirectUri}`)
@@ -131,17 +136,16 @@ export class OAuthClient {
131136
// Spin up a one‑time HTTP listener on localhost:randomPort */
132137
private static async buildCallbackServer(): Promise<{ server: http.Server; redirectUri: string }> {
133138
const server = http.createServer()
134-
const port = await new Promise<number>(res =>
135-
server.listen(0, '127.0.0.1', () => res((server.address() as any).port))
136-
)
139+
await this.listen(server, 0)
140+
const port = (server.address() as any).port as number
137141
return { server, redirectUri: `http://localhost:${port}` }
138142
}
139143

140144
/** Discover OAuth endpoints by HEAD/WWW‑Authenticate, well‑known, or fallback */
141145
private static async discoverAS(rs: URL): Promise<Meta> {
142146
// a) HEAD → WWW‑Authenticate → resource_metadata
143147
try {
144-
const h = await fetch(rs.toString(), { method: 'HEAD' })
148+
const h = await this.fetchCompat(rs.toString(), { method: 'HEAD' })
145149
const header = h.headers.get('www-authenticate') || ''
146150
const m = /resource_metadata=(?:"([^"]+)"|([^,\s]+))/i.exec(header)
147151
if (m) {
@@ -259,7 +263,7 @@ export class OAuthClient {
259263
client_id: reg.client_id,
260264
resource: rs.toString(),
261265
})
262-
const res = await fetch(meta.token_endpoint, {
266+
const res = await this.fetchCompat(meta.token_endpoint, {
263267
method: 'POST',
264268
headers: { 'content-type': 'application/x-www-form-urlencoded' },
265269
body: form,
@@ -332,7 +336,7 @@ export class OAuthClient {
332336
redirect_uri: redirectUri,
333337
resource: rs.toString(),
334338
})
335-
const res2 = await fetch(meta.token_endpoint, {
339+
const res2 = await this.fetchCompat(meta.token_endpoint, {
336340
method: 'POST',
337341
headers: { 'content-type': 'application/x-www-form-urlencoded' },
338342
body: form2,
@@ -347,7 +351,7 @@ export class OAuthClient {
347351

348352
/** Fetch + error‑check + parse JSON */
349353
private static async json<T>(url: string, init?: RequestInit): Promise<T> {
350-
const r = await fetch(url, init)
354+
const r = await this.fetchCompat(url, init)
351355
if (!r.ok) {
352356
const txt = await r.text().catch(() => '')
353357
throw new Error(`HTTP ${r.status}@${url}${txt}`)
@@ -393,4 +397,39 @@ export class OAuthClient {
393397
'sso',
394398
'cache'
395399
)
400+
401+
/**
402+
* Await server.listen() but reject if it emits 'error' (eg EADDRINUSE),
403+
* so callers can handle it immediately instead of hanging.
404+
*/
405+
private static listen(server: http.Server, port: number, host: string = '127.0.0.1'): Promise<void> {
406+
return new Promise((resolve, reject) => {
407+
const onListening = () => {
408+
server.off('error', onError)
409+
resolve()
410+
}
411+
const onError = (err: NodeJS.ErrnoException) => {
412+
server.off('listening', onListening)
413+
reject(err)
414+
}
415+
server.once('listening', onListening)
416+
server.once('error', onError)
417+
server.listen(port, host)
418+
})
419+
}
420+
421+
/**
422+
* Fetch compatibility: use global fetch on Node >= 18, otherwise dynamically import('node-fetch').
423+
* Using Function('return import(...)') avoids downleveling to require() in CJS builds.
424+
*/
425+
private static async fetchCompat(url: string, init?: RequestInit): Promise<any> {
426+
const g = globalThis as any
427+
if (typeof g.fetch === 'function') {
428+
return g.fetch(url as any, init as any)
429+
}
430+
// Dynamic import of ESM node-fetch (only when global fetch is unavailable)
431+
const mod = await (Function('return import("node-fetch")')() as Promise<any>)
432+
const f = mod.default ?? mod
433+
return f(url as any, init as any)
434+
}
396435
}

server/aws-lsp-codewhisperer/src/shared/localProjectContextController.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export class LocalProjectContextController {
8585
try {
8686
await waitUntil(async () => this.instance, {
8787
interval: 1000,
88-
timeout: 60,
88+
timeout: 60_000,
8989
truthy: true,
9090
})
9191

@@ -359,7 +359,7 @@ export class LocalProjectContextController {
359359
}
360360
return false
361361
},
362-
{ interval: 1000, timeout: 60, truthy: true }
362+
{ interval: 1000, timeout: 60_000, truthy: true }
363363
)
364364
return true
365365
} catch (error) {

0 commit comments

Comments
 (0)