diff --git a/src/features/common/managerReady.ts b/src/features/common/managerReady.ts index c85b3134..a0b71c26 100644 --- a/src/features/common/managerReady.ts +++ b/src/features/common/managerReady.ts @@ -10,7 +10,7 @@ import { installExtension } from '../../common/workbenchCommands'; import { EnvironmentManagers, PythonProjectManager } from '../../internal.api'; import { getDefaultEnvManagerSetting, getDefaultPkgManagerSetting } from '../settings/settingHelpers'; -const MANAGER_READY_TIMEOUT_MS = 30_000; +export const MANAGER_READY_TIMEOUT_MS = 30_000; interface ManagerReady extends Disposable { waitForEnvManager(uris?: Uri[]): Promise; @@ -27,6 +27,48 @@ function getExtensionId(managerId: string): string | undefined { return parts ? parts[1] : undefined; } +/** + * Wraps a deferred with a timeout so a missing/dead manager cannot block the API forever. + * On timeout the deferred is resolved (not rejected) so callers proceed with degraded results + * instead of hanging. + */ +export function withManagerTimeout( + deferred: Deferred, + managerId: string, + kind: 'environment' | 'package', +): Promise { + if (deferred.completed) { + return deferred.promise; + } + return new Promise((resolve) => { + const timer = setTimeout(() => { + if (!deferred.completed) { + traceWarn( + `Timed out after ${MANAGER_READY_TIMEOUT_MS / 1000}s waiting for ${kind} manager "${managerId}" to register. ` + + `The manager may not be installed or its extension failed to activate. Proceeding without it. ` + + `To prevent this, check your "python-envs.defaultEnvManager" and "python-envs.pythonProjects" settings.`, + ); + sendTelemetryEvent(EventNames.MANAGER_READY_TIMEOUT, undefined, { + managerId, + managerKind: kind, + }); + deferred.resolve(); + } + }, MANAGER_READY_TIMEOUT_MS); + + deferred.promise.then( + () => { + clearTimeout(timer); + resolve(); + }, + () => { + clearTimeout(timer); + resolve(); + }, + ); + }); +} + class ManagerReadyImpl implements ManagerReady { private readonly envManagers: Map> = new Map(); private readonly pkgManagers: Map> = new Map(); @@ -111,44 +153,6 @@ class ManagerReadyImpl implements ManagerReady { } } - /** - * Wraps a deferred with a timeout so a missing/dead manager cannot block the API forever. - * On timeout the deferred is resolved (not rejected) so callers proceed with degraded results - * instead of hanging. - */ - private _withTimeout(deferred: Deferred, managerId: string, kind: string): Promise { - if (deferred.completed) { - return deferred.promise; - } - return new Promise((resolve) => { - const timer = setTimeout(() => { - if (!deferred.completed) { - traceWarn( - `Timed out after ${MANAGER_READY_TIMEOUT_MS / 1000}s waiting for ${kind} manager "${managerId}" to register. ` + - `The manager may not be installed or its extension failed to activate. Proceeding without it. ` + - `To prevent this, check your "python-envs.defaultEnvManager" and "python-envs.pythonProjects" settings.`, - ); - sendTelemetryEvent(EventNames.MANAGER_READY_TIMEOUT, undefined, { - managerId, - managerKind: kind as 'environment' | 'package', - }); - deferred.resolve(); - } - }, MANAGER_READY_TIMEOUT_MS); - - deferred.promise.then( - () => { - clearTimeout(timer); - resolve(); - }, - () => { - clearTimeout(timer); - resolve(); - }, - ); - }); - } - public dispose(): void { this.disposables.forEach((d) => d.dispose()); this.envManagers.clear(); @@ -161,7 +165,7 @@ class ManagerReadyImpl implements ManagerReady { } const deferred = createDeferred(); this.envManagers.set(managerId, deferred); - return this._withTimeout(deferred, managerId, 'environment'); + return withManagerTimeout(deferred, managerId, 'environment'); } public async waitForEnvManager(uris?: Uri[]): Promise { @@ -210,7 +214,7 @@ class ManagerReadyImpl implements ManagerReady { } const deferred = createDeferred(); this.pkgManagers.set(managerId, deferred); - return this._withTimeout(deferred, managerId, 'package'); + return withManagerTimeout(deferred, managerId, 'package'); } public async waitForPkgManager(uris?: Uri[]): Promise { diff --git a/src/test/features/common/managerReady.unit.test.ts b/src/test/features/common/managerReady.unit.test.ts new file mode 100644 index 00000000..7cef4b54 --- /dev/null +++ b/src/test/features/common/managerReady.unit.test.ts @@ -0,0 +1,106 @@ +import assert from 'assert'; +import * as sinon from 'sinon'; +import * as logging from '../../../common/logging'; +import { EventNames } from '../../../common/telemetry/constants'; +import * as telemetrySender from '../../../common/telemetry/sender'; +import { createDeferred } from '../../../common/utils/deferred'; +import { MANAGER_READY_TIMEOUT_MS, withManagerTimeout } from '../../../features/common/managerReady'; + +suite('withManagerTimeout', () => { + let clock: sinon.SinonFakeTimers; + let traceWarnStub: sinon.SinonStub; + let sendTelemetryStub: sinon.SinonStub; + + setup(() => { + clock = sinon.useFakeTimers(); + traceWarnStub = sinon.stub(logging, 'traceWarn'); + sendTelemetryStub = sinon.stub(telemetrySender, 'sendTelemetryEvent'); + }); + + teardown(() => { + clock.restore(); + sinon.restore(); + }); + + test('deferred never resolves → timeout fires, logs warning, sends telemetry', async () => { + const deferred = createDeferred(); + const promise = withManagerTimeout(deferred, 'test-ext:venv', 'environment'); + + // Advance past the timeout + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); // flush microtasks + + await promise; + + // Warning was logged with manager ID + assert.ok(traceWarnStub.calledOnce, 'traceWarn should be called once'); + assert.ok(traceWarnStub.firstCall.args[0].includes('test-ext:venv'), 'warning should contain the manager ID'); + + // Telemetry was sent + assert.ok(sendTelemetryStub.calledOnce, 'sendTelemetryEvent should be called once'); + const [eventName, , properties] = sendTelemetryStub.firstCall.args; + assert.strictEqual(eventName, EventNames.MANAGER_READY_TIMEOUT); + assert.strictEqual(properties.managerId, 'test-ext:venv'); + assert.strictEqual(properties.managerKind, 'environment'); + }); + + test('deferred resolves before timeout → no warning, no telemetry', async () => { + const deferred = createDeferred(); + const promise = withManagerTimeout(deferred, 'test-ext:conda', 'environment'); + + // Resolve before timeout + deferred.resolve(); + await clock.tickAsync(0); + + await promise; + + // Advance past the timeout to confirm it was cleared + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + + assert.ok(traceWarnStub.notCalled, 'traceWarn should not be called'); + assert.ok(sendTelemetryStub.notCalled, 'sendTelemetryEvent should not be called'); + }); + + test('timeout resolves (not rejects) the deferred', async () => { + const deferred = createDeferred(); + const promise = withManagerTimeout(deferred, 'test-ext:missing', 'environment'); + + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + + // This must resolve — if it rejects, the test fails + await promise; + + assert.ok(deferred.resolved, 'deferred should be resolved, not rejected'); + assert.ok(!deferred.rejected, 'deferred should not be rejected'); + }); + + test('already-completed deferred returns immediately without timeout', async () => { + const deferred = createDeferred(); + deferred.resolve(); + + const promise = withManagerTimeout(deferred, 'test-ext:venv', 'environment'); + await promise; + + // No timer was set, so nothing should fire + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + + assert.ok(traceWarnStub.notCalled, 'traceWarn should not be called for completed deferred'); + assert.ok(sendTelemetryStub.notCalled, 'sendTelemetryEvent should not be called for completed deferred'); + }); + + test('package manager kind is passed through to telemetry', async () => { + const deferred = createDeferred(); + const promise = withManagerTimeout(deferred, 'test-ext:pip', 'package'); + + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + await promise; + + const [, , properties] = sendTelemetryStub.firstCall.args; + assert.strictEqual(properties.managerId, 'test-ext:pip'); + assert.strictEqual(properties.managerKind, 'package'); + }); +});