diff --git a/src/common/telemetry/errorClassifier.ts b/src/common/telemetry/errorClassifier.ts index 6cb20a2ab..ec4f144d9 100644 --- a/src/common/telemetry/errorClassifier.ts +++ b/src/common/telemetry/errorClassifier.ts @@ -27,6 +27,18 @@ export function isTimeoutErrorType(errorType: DiscoveryErrorType): boolean { (errorType.startsWith('rpc_') && errorType.endsWith('_timeout')); } +/** + * True when `ex` is a lost PET JSON-RPC connection — an {@link rpc.ConnectionError} or a + * {@link rpc.ResponseError} with {@link rpc.ErrorCodes.PendingResponseRejected}. Pure classifier: + * it cannot tell an intentional disposal from a crash, so callers must gate on lifecycle state. + */ +export function isPetConnectionLostError(ex: unknown): boolean { + return ( + ex instanceof rpc.ConnectionError || + (ex instanceof rpc.ResponseError && ex.code === rpc.ErrorCodes.PendingResponseRejected) + ); +} + /** * Classifies an error into a telemetry-safe category for the `errorType` property. * Does NOT include raw error messages — only the category. @@ -49,8 +61,7 @@ export function classifyError(ex: unknown): DiscoveryErrorType { } } - // JSON-RPC connection errors (e.g., PET process died mid-request, connection closed/disposed) - if (ex instanceof rpc.ConnectionError) { + if (isPetConnectionLostError(ex)) { return 'connection_error'; } diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index a09b83e9e..25be42596 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -11,7 +11,7 @@ import { getExtension } from '../../common/extension.apis'; import { traceError, traceVerbose, traceWarn } from '../../common/logging'; import { StopWatch } from '../../common/stopWatch'; import { EventNames } from '../../common/telemetry/constants'; -import { classifyError, isTimeoutErrorType } from '../../common/telemetry/errorClassifier'; +import { classifyError, isPetConnectionLostError, isTimeoutErrorType } from '../../common/telemetry/errorClassifier'; import { sendTelemetryEvent } from '../../common/telemetry/sender'; import { untildify, untildifyArray } from '../../common/utils/pathUtils'; import { isWindows } from '../../common/utils/platformUtils'; @@ -342,7 +342,10 @@ async function sendRequestWithTimeout( } } -class NativePythonFinderImpl implements NativePythonFinder { +/** + * @internal Concrete {@link NativePythonFinder}, exported only as a test seam — not public API. + */ +export class NativePythonFinderImpl implements NativePythonFinder { private connection: rpc.MessageConnection; private readonly pool: WorkerPool; private cache: Map = new Map(); @@ -357,6 +360,7 @@ class NativePythonFinderImpl implements NativePythonFinder { private startFailed: boolean = false; private restartAttempts: number = 0; private isRestarting: boolean = false; + private disposed: boolean = false; private processExitReason: string | undefined = undefined; private readonly configureRetry = new ConfigureRetryState(); /** @@ -403,15 +407,16 @@ class NativePythonFinderImpl implements NativePythonFinder { }); return environment; } catch (ex) { - // On resolve timeout or connection error (not configure — configure handles its own timeout), - // kill the hung process so next request triggers restart - if ((ex instanceof RpcTimeoutError && ex.method !== 'configure') || ex instanceof rpc.ConnectionError) { - const reason = ex instanceof rpc.ConnectionError ? 'crashed' : 'timed out'; + if ( + (ex instanceof RpcTimeoutError && ex.method !== 'configure') || + this.isRecoverableConnectionLoss(ex) + ) { + const reason = ex instanceof RpcTimeoutError ? 'timed out' : 'crashed'; this.outputChannel.warn(`[pet] Resolve request ${reason}, killing process for restart`); this.killProcess(); this.processExited = true; this.processExitReason = - ex instanceof rpc.ConnectionError ? 'rpc_connection_error' : 'rpc_resolve_timeout'; + ex instanceof RpcTimeoutError ? 'rpc_resolve_timeout' : 'rpc_connection_error'; } throw ex; } @@ -436,6 +441,10 @@ class NativePythonFinderImpl implements NativePythonFinder { } } + private isRecoverableConnectionLoss(ex: unknown): boolean { + return !this.disposed && !this.isRestarting && isPetConnectionLostError(ex); + } + /** * Ensures the PET process is running. If it has exited or failed, attempts to restart * with exponential backoff up to MAX_RESTART_ATTEMPTS times. @@ -638,6 +647,7 @@ class NativePythonFinderImpl implements NativePythonFinder { } public dispose() { + this.disposed = true; this.pool.stop(); this.startDisposables.forEach((d) => d.dispose()); this.connection.dispose(); @@ -673,24 +683,49 @@ class NativePythonFinderImpl implements NativePythonFinder { const readable = new PassThrough(); const writable = new PassThrough(); + // Owned by THIS child, so a dead child closes only its own resources after a later restart(). + const localDisposables: Disposable[] = []; + this.startDisposables = localDisposables; + + let streamsEnded = false; + let childStdout: NodeJS.ReadableStream | undefined; + const endStreams = () => { + if (streamsEnded) { + return; + } + streamsEnded = true; + // Unpipe before ending so buffered stdout can't raise a write-after-end on the ended stream. + childStdout?.unpipe(readable); + writable.unpipe(); + readable.end(); + writable.end(); + }; + try { - this.proc = spawnProcess(this.toolPath, ['server'], { env: process.env, stdio: 'pipe' }); + const proc = spawnProcess(this.toolPath, ['server'], { env: process.env, stdio: 'pipe' }); + this.proc = proc; - if (!this.proc.stdout || !this.proc.stderr || !this.proc.stdin) { + if (!proc.stdout || !proc.stderr || !proc.stdin) { throw new Error('Failed to create stdio streams for PET process'); } - this.proc.stdout.pipe(readable, { end: false }); - this.proc.stderr.on('data', (data) => this.outputChannel.error(`[pet] ${data.toString()}`)); - writable.pipe(this.proc.stdin, { end: false }); + childStdout = proc.stdout; + proc.stdout.pipe(readable, { end: false }); + proc.stderr.on('data', (data) => this.outputChannel.error(`[pet] ${data.toString()}`)); + writable.pipe(proc.stdin, { end: false }); - // Handle process exit - mark as exited so pending requests fail fast - this.proc.on('exit', (code, signal) => { - this.processExited = true; - // Preserve a more-specific reason (e.g. rpc_*) if one was already recorded before the kill. - if (this.processExitReason === undefined) { - this.processExitReason = `process_exit:${code ?? 'null'}:${signal ?? 'none'}`; + const handleChildTermination = (reason: string) => { + endStreams(); + if (this.proc === proc) { + this.processExited = true; + if (this.processExitReason === undefined) { + this.processExitReason = reason; + } } + }; + + proc.on('exit', (code, signal) => { + handleChildTermination(`process_exit:${code ?? 'null'}:${signal ?? 'none'}`); if (code !== 0) { this.outputChannel.error( `[pet] Python Environment Tools exited unexpectedly with code ${code}, signal ${signal}`, @@ -698,17 +733,12 @@ class NativePythonFinderImpl implements NativePythonFinder { } }); - // Handle process errors (e.g., ENOENT if executable not found) - this.proc.on('error', (err) => { - this.processExited = true; - if (this.processExitReason === undefined) { - this.processExitReason = 'process_error'; - } + proc.on('error', (err) => { + handleChildTermination('process_error'); this.outputChannel.error('[pet] Process error:', err); }); - const proc = this.proc; - this.startDisposables.push({ + localDisposables.push({ dispose: () => { try { if (proc.exitCode === null) { @@ -742,12 +772,9 @@ class NativePythonFinderImpl implements NativePythonFinder { new rpc.StreamMessageReader(readable), new rpc.StreamMessageWriter(writable), ); - this.startDisposables.push( + localDisposables.push( connection, - new Disposable(() => { - readable.end(); - writable.end(); - }), + new Disposable(() => endStreams()), connection.onError((ex) => { this.outputChannel.error('[pet] Connection Error:', ex); }), @@ -772,7 +799,7 @@ class NativePythonFinderImpl implements NativePythonFinder { }), connection.onNotification('telemetry', (data) => this.outputChannel.info('[pet] Telemetry: ', data)), connection.onClose(() => { - this.startDisposables.forEach((d) => d.dispose()); + localDisposables.forEach((d) => d.dispose()); }), ); @@ -852,12 +879,12 @@ class NativePythonFinderImpl implements NativePythonFinder { } catch (ex) { lastError = ex; - // Retry on timeout or connection errors (PET hung or crashed mid-request) const isRetryable = - (ex instanceof RpcTimeoutError && ex.method !== 'configure') || ex instanceof rpc.ConnectionError; + (ex instanceof RpcTimeoutError && ex.method !== 'configure') || + this.isRecoverableConnectionLoss(ex); if (isRetryable) { if (attempt < MAX_REFRESH_RETRIES) { - const reason = ex instanceof rpc.ConnectionError ? 'crashed' : 'timed out'; + const reason = ex instanceof RpcTimeoutError ? 'timed out' : 'crashed'; this.outputChannel.warn( `[pet] Refresh ${reason} (attempt ${attempt + 1}/${MAX_REFRESH_RETRIES + 1}), restarting and retrying...`, ); @@ -865,7 +892,7 @@ class NativePythonFinderImpl implements NativePythonFinder { this.killProcess(); this.processExited = true; this.processExitReason = - ex instanceof rpc.ConnectionError ? 'rpc_connection_error' : 'rpc_refresh_timeout'; + ex instanceof RpcTimeoutError ? 'rpc_refresh_timeout' : 'rpc_connection_error'; continue; } // Final attempt failed @@ -997,15 +1024,16 @@ class NativePythonFinderImpl implements NativePythonFinder { }, ex instanceof Error ? ex : undefined, ); - // On refresh timeout or connection error (not configure — configure handles its own timeout), - // kill the hung process so next request triggers restart - if ((ex instanceof RpcTimeoutError && ex.method !== 'configure') || ex instanceof rpc.ConnectionError) { - const reason = ex instanceof rpc.ConnectionError ? 'crashed' : 'timed out'; + if ( + (ex instanceof RpcTimeoutError && ex.method !== 'configure') || + this.isRecoverableConnectionLoss(ex) + ) { + const reason = ex instanceof RpcTimeoutError ? 'timed out' : 'crashed'; this.outputChannel.warn(`[pet] PET process ${reason}, killing for restart`); this.killProcess(); this.processExited = true; this.processExitReason = - ex instanceof rpc.ConnectionError ? 'rpc_connection_error' : 'rpc_refresh_timeout'; + ex instanceof RpcTimeoutError ? 'rpc_refresh_timeout' : 'rpc_connection_error'; } this.outputChannel.error('[pet] Error refreshing', ex); throw ex; diff --git a/src/test/common/telemetry/errorClassifier.unit.test.ts b/src/test/common/telemetry/errorClassifier.unit.test.ts index 9e429a85f..5ca65f605 100644 --- a/src/test/common/telemetry/errorClassifier.unit.test.ts +++ b/src/test/common/telemetry/errorClassifier.unit.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert'; import { CancellationError } from 'vscode'; import * as rpc from 'vscode-jsonrpc/node'; import { BaseError } from '../../../common/errors/types'; -import { classifyError, isTimeoutErrorType } from '../../../common/telemetry/errorClassifier'; +import { classifyError, isPetConnectionLostError, isTimeoutErrorType } from '../../../common/telemetry/errorClassifier'; import { RpcTimeoutError } from '../../../managers/common/nativePythonFinder'; suite('Error Classifier', () => { @@ -86,6 +86,13 @@ suite('Error Classifier', () => { assert.strictEqual(classifyError(new rpc.ResponseError(-32601, 'Method not found')), 'rpc_error'); }); + test('should classify PendingResponseRejected ResponseError as connection_error', () => { + assert.strictEqual( + classifyError(new rpc.ResponseError(rpc.ErrorCodes.PendingResponseRejected, 'Pending response rejected')), + 'connection_error', + ); + }); + test('should classify BaseError subclasses as already_registered', () => { // Using a concrete subclass to test (BaseError is abstract) class TestRegisteredError extends BaseError { @@ -130,6 +137,34 @@ suite('Error Classifier', () => { }); }); + suite('isPetConnectionLostError', () => { + test('recognizes JSON-RPC ConnectionError (transport failure)', () => { + assert.strictEqual( + isPetConnectionLostError(new rpc.ConnectionError(rpc.ConnectionErrors.Closed, 'closed')), + true, + ); + assert.strictEqual( + isPetConnectionLostError(new rpc.ConnectionError(rpc.ConnectionErrors.Disposed, 'disposed')), + true, + ); + }); + + test('recognizes PendingResponseRejected ResponseError (connection disposed mid-request)', () => { + assert.strictEqual( + isPetConnectionLostError(new rpc.ResponseError(rpc.ErrorCodes.PendingResponseRejected, 'rejected')), + true, + ); + }); + + test('does NOT match other ResponseError codes or unrelated errors', () => { + assert.strictEqual(isPetConnectionLostError(new rpc.ResponseError(-32600, 'Invalid request')), false); + assert.strictEqual(isPetConnectionLostError(new RpcTimeoutError('refresh', 30000)), false); + assert.strictEqual(isPetConnectionLostError(new Error('boom')), false); + assert.strictEqual(isPetConnectionLostError('nope'), false); + assert.strictEqual(isPetConnectionLostError(undefined), false); + }); + }); + suite('isTimeoutErrorType', () => { test('recognizes spawn and JSON-RPC timeout categories', () => { assert.strictEqual(isTimeoutErrorType('spawn_timeout'), true); diff --git a/src/test/managers/common/nativePythonFinder.petExit.unit.test.ts b/src/test/managers/common/nativePythonFinder.petExit.unit.test.ts new file mode 100644 index 000000000..3c6097591 --- /dev/null +++ b/src/test/managers/common/nativePythonFinder.petExit.unit.test.ts @@ -0,0 +1,425 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'node:assert'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; +import * as sinon from 'sinon'; +import * as rpc from 'vscode-jsonrpc/node'; +import { PythonProjectApi } from '../../../api'; +import * as childProcessApis from '../../../common/childProcess.apis'; +import { NativePythonFinderImpl } from '../../../managers/common/nativePythonFinder'; + +class FakeChild extends EventEmitter { + public readonly stdout = new PassThrough(); + public readonly stderr = new PassThrough(); + public readonly stdin = new PassThrough(); + public exitCode: number | null = null; + public killed = false; + + public kill(_signal?: NodeJS.Signals | number): boolean { + this.killed = true; + return true; + } + + public simulateExit(code: number | null = 0, signal: NodeJS.Signals | null = null): void { + this.markExited(code); + this.emit('exit', code, signal); + } + + public simulateError(err: Error): void { + this.markExited(); + this.emit('error', err); + } + + public markExited(code: number | null = 0): void { + if (this.exitCode === null) { + this.exitCode = code ?? 0; + } + } +} + +function flush(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + +class FakePetServer { + public readonly connection: rpc.MessageConnection; + public refreshMode: 'answer' | 'hang' = 'answer'; + + constructor(child: FakeChild) { + this.connection = rpc.createMessageConnection( + new rpc.StreamMessageReader(child.stdin), + new rpc.StreamMessageWriter(child.stdout), + ); + this.connection.onRequest('configure', () => null); + this.connection.onRequest('resolve', (p: { executable: string }) => ({ + executable: p.executable, + version: '3.11.0', + prefix: '/env', + })); + this.connection.onRequest('refresh', () => { + if (this.refreshMode === 'hang') { + return new Promise<{ duration: number }>(() => { + /* never resolves */ + }); + } + this.connection.sendNotification('manager', { tool: 'venv', executable: '/usr/bin/python3' }); + return { duration: 0 }; + }); + this.connection.listen(); + } + + public dispose(): void { + try { + this.connection.dispose(); + } catch { + /* ignore */ + } + } +} + +function isPendingResponseRejected(message: string): (err: unknown) => boolean { + return (err: unknown): boolean => { + assert.ok(err instanceof rpc.ResponseError, `${message}: expected a ResponseError`); + assert.strictEqual( + (err as rpc.ResponseError).code, + rpc.ErrorCodes.PendingResponseRejected, + `${message}: expected the connection-dispose rejection`, + ); + return true; + }; +} + +function makeOutputChannel(): unknown { + const noop = (): void => { + /* no-op */ + }; + return { + info: noop, + warn: noop, + error: noop, + debug: noop, + trace: noop, + append: noop, + appendLine: noop, + show: noop, + clear: noop, + dispose: noop, + }; +} + +suite('NativePythonFinder PET-exit RPC teardown', () => { + let children: FakeChild[] = []; + let servers: FakePetServer[] = []; + let attachServers = false; + let finder: NativePythonFinderImpl | undefined; + + setup(() => { + children = []; + servers = []; + attachServers = false; + sinon.stub(childProcessApis, 'spawnProcess').callsFake(() => { + const child = new FakeChild(); + children.push(child); + if (attachServers) { + servers.push(new FakePetServer(child)); + } + return child as unknown as ReturnType; + }); + sinon.stub(NativePythonFinderImpl.prototype as unknown as { kickoffInfoFetch: () => void }, 'kickoffInfoFetch'); + }); + + teardown(() => { + children.forEach((c) => c.markExited()); + servers.forEach((s) => s.dispose()); + try { + finder?.dispose(); + } catch { + /* ignore */ + } + finder = undefined; + sinon.restore(); + }); + + function createFinder(): NativePythonFinderImpl { + finder = new NativePythonFinderImpl( + makeOutputChannel() as never, + 'fake-pet-tool', + {} as unknown as PythonProjectApi, + undefined, + ); + return finder; + } + + function getConnection(f: NativePythonFinderImpl): rpc.MessageConnection { + return (f as unknown as { connection: rpc.MessageConnection }).connection; + } + + function getState(f: NativePythonFinderImpl): { processExited: boolean; processExitReason: string | undefined } { + return f as unknown as { processExited: boolean; processExitReason: string | undefined }; + } + + test('pending request rejects promptly when PET process exits', async () => { + const f = createFinder(); + const child = children[0]; + const connection = getConnection(f); + + const pending = connection.sendRequest('resolve', { executable: 'x' }); + await flush(); + + child.simulateExit(1, null); + + await assert.rejects(pending, isPendingResponseRejected('pending request should reject when PET exits')); + assert.strictEqual(getState(f).processExited, true, 'processExited should be set on exit'); + }); + + test('pending request rejects promptly when PET process errors', async () => { + const f = createFinder(); + const child = children[0]; + const connection = getConnection(f); + + const pending = connection.sendRequest('refresh', {}); + await flush(); + + child.simulateError(new Error('spawn ENOENT')); + + await assert.rejects(pending, isPendingResponseRejected('pending request should reject when PET errors')); + assert.strictEqual(getState(f).processExited, true, 'processExited should be set on error'); + }); + + test('duplicate error + exit is harmless (idempotent teardown)', async () => { + const f = createFinder(); + const child = children[0]; + const connection = getConnection(f); + + const pending = connection.sendRequest('resolve', { executable: 'x' }); + const rejection = assert.rejects(pending, 'request should reject exactly once'); + await flush(); + + child.simulateExit(2, null); + child.simulateError(new Error('post-exit error')); + await flush(); + + await rejection; + const state = getState(f); + assert.strictEqual(state.processExited, true); + assert.strictEqual(state.processExitReason, 'process_exit:2:none'); + }); + + test('stale old-child exit cannot close a replacement connection', async () => { + const f = createFinder(); + const oldConnection = getConnection(f); + const oldChild = children[0]; + + const newConnection = (f as unknown as { start(): rpc.MessageConnection }).start(); + (f as unknown as { connection: rpc.MessageConnection }).connection = newConnection; + const newChild = children[1]; + assert.notStrictEqual(newConnection, oldConnection, 'sanity: replacement connection is distinct'); + + const pendingNew = newConnection.sendRequest('resolve', { executable: 'x' }); + let newSettled = false; + pendingNew.then( + () => (newSettled = true), + () => (newSettled = true), + ); + await flush(); + + oldChild.simulateExit(1, null); + await flush(); + await flush(); + + assert.strictEqual(newSettled, false, 'replacement request must not be rejected by a stale child exit'); + assert.strictEqual( + getState(f).processExited, + false, + 'a stale child exit must not flip processExited on the live replacement', + ); + assert.strictEqual(getConnection(f), newConnection, 'active connection must remain the replacement'); + + newChild.simulateExit(1, null); + await assert.rejects(pendingNew, 'replacement request should reject when its own child exits'); + }); + + test('restart produces a usable connection and resets exit state', async () => { + const f = createFinder(); + const oldConnection = getConnection(f); + + const clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + try { + const restartPromise = (f as unknown as { restart(): Promise }).restart(); + await clock.tickAsync(5000); // advance past backoff (1s) and any 500ms kill timers + await restartPromise; + } finally { + clock.restore(); + } + + const newConnection = getConnection(f); + assert.notStrictEqual(newConnection, oldConnection, 'restart should create a new connection'); + const state = getState(f); + assert.strictEqual(state.processExited, false, 'processExited should be reset after restart'); + + const newChild = children[children.length - 1]; + const pending = newConnection.sendRequest('resolve', { executable: 'x' }); + let settled = false; + pending.then( + () => (settled = true), + () => (settled = true), + ); + await flush(); + assert.strictEqual(settled, false, 'request on restarted connection should stay pending until child exits'); + + newChild.simulateExit(1, null); + await assert.rejects(pending, 'request should reject once the restarted child exits'); + }); + + function stubRefreshWireDeps(f: NativePythonFinderImpl): void { + const anyF = f as unknown as { + buildConfigurationOptions: () => Promise; + configure: () => Promise; + getRefreshOptions: () => unknown; + }; + sinon + .stub(anyF, 'buildConfigurationOptions') + .resolves({ workspaceDirectories: [], environmentDirectories: [] }); + sinon.stub(anyF, 'configure').resolves(); + sinon.stub(anyF, 'getRefreshOptions').returns({}); + } + + test('a current-child crash during refresh promptly restarts and the retry succeeds', async () => { + attachServers = true; + const f = createFinder(); + stubRefreshWireDeps(f); + const oldConnection = getConnection(f); + servers[0].refreshMode = 'hang'; + + const clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + try { + const refreshPromise = (f as unknown as { doRefresh(o?: unknown): Promise }).doRefresh(undefined); + let settled = false; + let result: unknown; + let error: unknown; + refreshPromise.then( + (r) => { + settled = true; + result = r; + }, + (e) => { + settled = true; + error = e; + }, + ); + + await flush(); + children[0].simulateExit(1, null); + + for (let i = 0; i < 15 && !settled; i++) { + await flush(); + await clock.tickAsync(1000); + } + + assert.ok(settled, 'refresh should settle after the retry'); + assert.strictEqual(error, undefined, `refresh should not reject: ${error}`); + assert.ok(Array.isArray(result), 'refresh should resolve with an environment array'); + assert.strictEqual((result as unknown[]).length, 1, 'retry should return the healthy child manager info'); + assert.strictEqual(getState(f).processExited, false, 'finder should be healthy after the restart'); + assert.notStrictEqual(getConnection(f), oldConnection, 'connection should be the post-restart replacement'); + assert.strictEqual(children.length, 2, 'exactly one restart should have spawned one replacement child'); + assert.strictEqual( + (f as unknown as { restartAttempts: number }).restartAttempts, + 0, + 'a successful retry should reset restartAttempts', + ); + } finally { + clock.restore(); + } + }); + + test('a connection loss during disposal is NOT treated as a recoverable crash', async () => { + const clock = sinon.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + try { + const f = createFinder(); + sinon.stub(f as unknown as { configure: () => Promise }, 'configure').resolves(); + + const pending = f.resolve('x'); + const rejection = assert.rejects( + pending, + isPendingResponseRejected('dispose must reject the in-flight resolve with PendingResponseRejected'), + ); + await flush(); + assert.strictEqual(children.length, 1, 'sanity: exactly one child spawned'); + + children[0].markExited(); + f.dispose(); + await rejection; + + assert.strictEqual(children.length, 1, 'dispose must not spawn a replacement child'); + assert.strictEqual( + (f as unknown as { restartAttempts: number }).restartAttempts, + 0, + 'dispose must not trigger a restart', + ); + assert.strictEqual( + ( + f as unknown as { isRecoverableConnectionLoss(ex: unknown): boolean } + ).isRecoverableConnectionLoss(new rpc.ResponseError(rpc.ErrorCodes.PendingResponseRejected, 'disposed')), + false, + 'a PendingResponseRejected after dispose must be classified non-recoverable', + ); + } finally { + clock.restore(); + } + }); + + test('refresh retry limit is preserved for connection-loss errors', async () => { + const f = createFinder(); + sinon.stub(f as unknown as { killProcess: () => void }, 'killProcess'); + const attemptStub = sinon + .stub(f as unknown as { doRefreshAttempt: () => Promise }, 'doRefreshAttempt') + .rejects(new rpc.ResponseError(rpc.ErrorCodes.PendingResponseRejected, 'crash')); + + await assert.rejects( + (f as unknown as { doRefresh(o?: unknown): Promise }).doRefresh(undefined), + isPendingResponseRejected('a persistent connection loss should propagate after the retry limit'), + ); + assert.strictEqual(attemptStub.callCount, 2, 'connection-loss errors must retry exactly to the refresh limit'); + }); + + test('buffered stdout after exit does not raise a late write error or affect a replacement', async () => { + const f = createFinder(); + const child = children[0]; + const connection = getConnection(f); + + assert.strictEqual(child.stdout.listenerCount('data'), 1, 'stdout should be piped before exit'); + + const pending = connection.sendRequest('resolve', { executable: 'x' }); + const rejection = assert.rejects(pending, isPendingResponseRejected('request should reject on exit')); + await flush(); + + child.simulateExit(1, null); + await flush(); + + assert.strictEqual(child.stdout.listenerCount('data'), 0, 'stdout must be unpiped from the ended readable'); + + child.stdout.write(Buffer.from('{"jsonrpc":"2.0","method":"log"}\r\n')); + child.stdout.write(Buffer.from('more late bytes')); + await flush(); + await rejection; + + const newConnection = (f as unknown as { start(): rpc.MessageConnection }).start(); + (f as unknown as { connection: rpc.MessageConnection }).connection = newConnection; + const newChild = children[children.length - 1]; + const pendingNew = newConnection.sendRequest('resolve', { executable: 'y' }); + let newSettled = false; + pendingNew.then( + () => (newSettled = true), + () => (newSettled = true), + ); + child.stdout.write(Buffer.from('still more bytes from the dead child')); + await flush(); + assert.strictEqual(newSettled, false, 'replacement request must be unaffected by the old child output'); + + newChild.simulateExit(1, null); + await assert.rejects(pendingNew, 'replacement request should reject when its own child exits'); + }); +}); diff --git a/src/test/mocks/vsc/telemetryReporter.ts b/src/test/mocks/vsc/telemetryReporter.ts index 02360e675..01c7f2f6b 100644 --- a/src/test/mocks/vsc/telemetryReporter.ts +++ b/src/test/mocks/vsc/telemetryReporter.ts @@ -5,4 +5,13 @@ export class vscMockTelemetryReporter { public sendTelemetryEvent(): void { // Noop. } + public sendTelemetryErrorEvent(): void { + // Noop. + } + public sendDangerousTelemetryEvent(): void { + // Noop. + } + public dispose(): Promise { + return Promise.resolve(); + } }