diff --git a/packages/mcp/mcp-client/src/connection.ts b/packages/mcp/mcp-client/src/connection.ts index a4d17c6ff1..976e686a8c 100644 --- a/packages/mcp/mcp-client/src/connection.ts +++ b/packages/mcp/mcp-client/src/connection.ts @@ -19,6 +19,7 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js' import { ToolListChangedNotificationSchema } from '@modelcontextprotocol/sdk/types.js' import type { Context } from '@deepseek-ai/cordis' import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' +import { describeError } from './diagnostic.ts' import { withDeadline } from './deadline.ts' import type { McpStatusCell } from './status.ts' import { createTransport } from './transport.ts' @@ -219,8 +220,8 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe () => { if (isCurrent(generation)) armHealthPing(generation) }, (error: unknown) => { if (!isCurrent(generation)) return - pendingDownReason = String(error) - ctx.logger.warn(`${label}: health ping failed: ${String(error)} — closing the connection to reconnect`) + pendingDownReason = describeError(error) + ctx.logger.warn(`${label}: health ping failed: ${describeError(error)} — closing the connection to reconnect`) // A close that completes reaches generationDown through onclose; a // transport too broken to close still must not strand the supervisor. generation.close().catch(() => { generationDown(generation) }) @@ -368,10 +369,10 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe ) } catch (error) { if (firstAttemptError === undefined) firstAttemptError = error - lastErrorText = String(error) + lastErrorText = describeError(error) // Disposal clears current ownership before it closes the generation, so // only a live supervisor reports an attempt failure. - if (isCurrent(generation)) ctx.logger.warn(`${label}: connection attempt failed: ${String(error)}`) + if (isCurrent(generation)) ctx.logger.warn(`${label}: connection attempt failed: ${describeError(error)}`) try { await generation.close() } catch { /* transport already gone */ } const quiesced = hasClosed() || await waitForClose(closed.promise) attemptSettled = true @@ -380,7 +381,7 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe client = undefined clientClosed = undefined ctx.logger.error(`${label}: failed generation did not close within ${GENERATION_CLOSE_TIMEOUT_MS}ms — reconnect stopped to avoid overlapping server processes; reload the plugin or restart the Host to retry`) - status.update({ state: 'failed', reason: 'unclosed-generation', lastError: String(error) }) + status.update({ state: 'failed', reason: 'unclosed-generation', lastError: describeError(error) }) return } generationDown(generation) diff --git a/packages/mcp/mcp-client/src/diagnostic.ts b/packages/mcp/mcp-client/src/diagnostic.ts new file mode 100644 index 0000000000..c9e915160d --- /dev/null +++ b/packages/mcp/mcp-client/src/diagnostic.ts @@ -0,0 +1,40 @@ +/** + * One-line diagnostic for a connection-layer failure, keeping the cause chain + * that `String(error)` drops. Node's fetch wraps the actionable failure — + * `connect ECONNREFUSED 127.0.0.1:8932`, a TLS code, a DNS code — inside + * `error.cause`, so the outer `TypeError: fetch failed` alone diagnoses + * nothing. The raw identifiers are kept verbatim: they are what a person + * pastes into a search or hands to a model. + */ + +/** One readable segment of the chain; generic `Error` names are noise and dropped. */ +function segmentOf(value: unknown): string { + if (!(value instanceof Error)) return String(value) + const base = value.name === 'Error' || value.message === '' + ? (value.message === '' ? value.name : value.message) + : `${value.name}: ${value.message}` + if (base !== '' && base !== 'Error') return base + // Some aggregate connect failures carry an empty message but a syscall code. + const code = (value as { code?: unknown }).code + return typeof code === 'string' ? code : value.name +} + +/** + * Flatten an error and its `cause` chain into `outer, cause: inner, …`. + * @param error - the caught value; non-Errors stringify as-is. + * @returns the joined diagnostic line. + */ +export function describeError(error: unknown): string { + const seen = new Set() + const segments: string[] = [] + let current: unknown = error + while (current !== undefined && current !== null && !seen.has(current)) { + seen.add(current) + // The outer segment keeps its error name (`TypeError: fetch failed`). + segments.push(segments.length === 0 && current instanceof Error ? String(current) : segmentOf(current)) + current = current instanceof Error ? current.cause : undefined + } + const head = segments[0] + if (head === undefined) return String(error) + return head + segments.slice(1).map(segment => `, cause: ${segment}`).join('') +} diff --git a/packages/mcp/mcp-client/src/probe.ts b/packages/mcp/mcp-client/src/probe.ts index e16d3af8e9..d3072c2ccb 100644 --- a/packages/mcp/mcp-client/src/probe.ts +++ b/packages/mcp/mcp-client/src/probe.ts @@ -11,6 +11,7 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js' import { ListToolsResultSchema } from '@modelcontextprotocol/sdk/types.js' +import { describeError } from './diagnostic.ts' import { withDeadline } from './deadline.ts' import { createTransport } from './transport.ts' import type { Config } from './index.ts' @@ -117,7 +118,7 @@ export async function probeServer(config: Config, options: McpProbeOptions = {}) durationMs: Date.now() - startedAt, } } catch (error) { - return { ok: false, stage, message: String(error), durationMs: Date.now() - startedAt } + return { ok: false, stage, message: describeError(error), durationMs: Date.now() - startedAt } } finally { try { await client.close() diff --git a/packages/mcp/mcp-client/tests/diagnostic.spec.ts b/packages/mcp/mcp-client/tests/diagnostic.spec.ts new file mode 100644 index 0000000000..9b2adc0355 --- /dev/null +++ b/packages/mcp/mcp-client/tests/diagnostic.spec.ts @@ -0,0 +1,56 @@ +// The connection-diagnostic line: cause chains survive, raw identifiers stay +// verbatim, and hostile shapes (cycles, empty messages, bare values) cannot +// break the status text they feed. +import { describe, expect, it } from 'vitest' +import { describeError } from '@deepseek-ai/dsh-mcp-client/src/diagnostic.ts' + +describe('describeError', () => { + it('keeps a plain error as its own string', () => { + expect(describeError(new Error('boom'))).toBe('Error: boom') + expect(describeError(new TypeError('fetch failed'))).toBe('TypeError: fetch failed') + }) + + it('appends the cause chain the way fetch failures carry it', () => { + const cause = new Error('connect ECONNREFUSED 127.0.0.1:8932') + const outer = new TypeError('fetch failed', { cause }) + expect(describeError(outer)).toBe('TypeError: fetch failed, cause: connect ECONNREFUSED 127.0.0.1:8932') + }) + + it('walks nested causes and keeps a non-generic name on an inner segment', () => { + const inner = new RangeError('depth') + const middle = new Error('middle', { cause: inner }) + const outer = new Error('outer', { cause: middle }) + expect(describeError(outer)).toBe('Error: outer, cause: middle, cause: RangeError: depth') + }) + + it('falls back to the syscall code when an aggregate cause has no message', () => { + const aggregate = new Error('') + ;(aggregate as { code?: string }).code = 'ECONNREFUSED' + const outer = new TypeError('fetch failed', { cause: aggregate }) + expect(describeError(outer)).toBe('TypeError: fetch failed, cause: ECONNREFUSED') + }) + + it('names an empty codeless cause by its error name', () => { + const blank = new Error('') + const outer = new Error('outer', { cause: blank }) + expect(describeError(outer)).toBe('Error: outer, cause: Error') + }) + + it('stringifies non-error values at both positions', () => { + expect(describeError('wire literal')).toBe('wire literal') + const outer = new Error('outer', { cause: 'raw detail' }) + expect(describeError(outer)).toBe('Error: outer, cause: raw detail') + }) + + it('stringifies a nullish thrown value', () => { + expect(describeError(undefined)).toBe('undefined') + expect(describeError(null)).toBe('null') + }) + + it('stops on a circular cause chain', () => { + const a = new Error('a') + const b = new Error('b', { cause: a }) + a.cause = b + expect(describeError(a)).toBe('Error: a, cause: b') + }) +})