fix(mcp-client): keep the cause chain in connection diagnostics
String(error) flattened a fetch failure to 'TypeError: fetch failed', dropping the actionable cause (connect ECONNREFUSED host:port, a DNS or TLS code). describeError walks the cause chain into 'outer, cause: inner, …' — raw identifiers verbatim, since they are what gets searched or handed to a model — and feeds the attempt, ping, probe, and unclosed-generation diagnostics.
This commit is contained in:
1 parent
2425fa4f84
commit
032fd5f6d5
4 files changed
+104
-6
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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<unknown>()
|
||||
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('')
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
Reference in new issue
Block a user