Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/managers/builtin/sysPythonManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ export class SysPythonManager implements EnvironmentManager {
return this._initialized.promise;
}

this._initialized = createDeferred();
const initialized = createDeferred<void>();
this._initialized = initialized;

try {
await this.internalRefresh(false, SysManagerStrings.sysManagerDiscovering);
Expand All @@ -96,8 +97,13 @@ export class SysPythonManager implements EnvironmentManager {
}
}
}
} catch (ex) {
if (this._initialized === initialized) {
this._initialized = undefined;
}
throw ex;
} finally {
this._initialized.resolve();
initialized.resolve();
}
}

Expand Down
10 changes: 8 additions & 2 deletions src/managers/builtin/venvManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,18 @@ export class VenvManager implements EnvironmentManager {
return this._initialized.promise;
}

this._initialized = createDeferred();
const initialized = createDeferred<void>();
this._initialized = initialized;

try {
await this.internalRefresh(undefined, false, VenvManagerStrings.venvInitialize);
} catch (ex) {
if (this._initialized === initialized) {
this._initialized = undefined;
}
throw ex;
} finally {
this._initialized.resolve();
initialized.resolve();
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/managers/conda/condaEnvManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ export class CondaEnvManager implements EnvironmentManager, Disposable {
return this._initialized.promise;
}

this._initialized = createDeferred();
const initialized = createDeferred<void>();
this._initialized = initialized;
const stopWatch = new StopWatch();
let result: 'success' | 'tool_not_found' | 'error' = 'success';
let envCount = 0;
Expand Down Expand Up @@ -165,6 +166,9 @@ export class CondaEnvManager implements EnvironmentManager, Disposable {
result = 'error';
errorType = classifyError(ex);
traceError('Conda lazy initialization failed', ex);
if (this._initialized === initialized) {
this._initialized = undefined;
}
} finally {
sendTelemetryEvent(EventNames.MANAGER_LAZY_INIT, stopWatch.elapsedTime, {
managerName: 'conda',
Expand All @@ -173,7 +177,7 @@ export class CondaEnvManager implements EnvironmentManager, Disposable {
toolSource,
errorType,
});
this._initialized.resolve();
initialized.resolve();
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/managers/pipenv/pipenvManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ export class PipenvManager implements EnvironmentManager, Disposable {
if (this._initialized) {
return this._initialized.promise;
}
this._initialized = createDeferred();
const initialized = createDeferred<void>();
this._initialized = initialized;
const stopWatch = new StopWatch();
let result: 'success' | 'tool_not_found' | 'error' = 'success';
let envCount = 0;
Expand Down Expand Up @@ -129,6 +130,9 @@ export class PipenvManager implements EnvironmentManager, Disposable {
result = 'error';
errorType = classifyError(ex);
traceError('Pipenv lazy initialization failed', ex);
if (this._initialized === initialized) {
this._initialized = undefined;
}
} finally {
sendTelemetryEvent(EventNames.MANAGER_LAZY_INIT, stopWatch.elapsedTime, {
managerName: 'pipenv',
Expand All @@ -137,7 +141,7 @@ export class PipenvManager implements EnvironmentManager, Disposable {
toolSource,
errorType,
});
this._initialized.resolve();
initialized.resolve();
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/managers/poetry/poetryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ export class PoetryManager implements EnvironmentManager, Disposable {
if (this._initialized) {
return this._initialized.promise;
}
this._initialized = createDeferred();
const initialized = createDeferred<void>();
this._initialized = initialized;
const stopWatch = new StopWatch();
let result: 'success' | 'tool_not_found' | 'error' = 'success';
let envCount = 0;
Expand Down Expand Up @@ -127,6 +128,9 @@ export class PoetryManager implements EnvironmentManager, Disposable {
result = 'error';
errorType = classifyError(ex);
traceError('Poetry lazy initialization failed', ex);
if (this._initialized === initialized) {
this._initialized = undefined;
}
} finally {
sendTelemetryEvent(EventNames.MANAGER_LAZY_INIT, stopWatch.elapsedTime, {
managerName: 'poetry',
Expand All @@ -135,7 +139,7 @@ export class PoetryManager implements EnvironmentManager, Disposable {
toolSource,
errorType,
});
this._initialized.resolve();
initialized.resolve();
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/managers/pyenv/pyenvManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ export class PyEnvManager implements EnvironmentManager, Disposable {
if (this._initialized) {
return this._initialized.promise;
}
this._initialized = createDeferred();
const initialized = createDeferred<void>();
this._initialized = initialized;
const stopWatch = new StopWatch();
let result: 'success' | 'tool_not_found' | 'error' = 'success';
let envCount = 0;
Expand Down Expand Up @@ -128,6 +129,9 @@ export class PyEnvManager implements EnvironmentManager, Disposable {
result = 'error';
errorType = classifyError(ex);
traceError('Pyenv lazy initialization failed', ex);
if (this._initialized === initialized) {
this._initialized = undefined;
}
} finally {
sendTelemetryEvent(EventNames.MANAGER_LAZY_INIT, stopWatch.elapsedTime, {
managerName: 'pyenv',
Expand All @@ -136,7 +140,7 @@ export class PyEnvManager implements EnvironmentManager, Disposable {
toolSource,
errorType,
});
this._initialized.resolve();
initialized.resolve();
}
}

Expand Down
73 changes: 73 additions & 0 deletions src/test/managers/builtin/sysPythonManager.initialize.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import assert from 'assert';
import * as sinon from 'sinon';
import { anything, reset, when } from 'ts-mockito';
import { PythonEnvironmentApi } from '../../../api';
import * as logging from '../../../common/logging';
import * as cache from '../../../managers/builtin/cache';
import { SysPythonManager } from '../../../managers/builtin/sysPythonManager';
import * as utils from '../../../managers/builtin/utils';
import * as uvInstaller from '../../../managers/builtin/uvPythonInstaller';
import { NativePythonFinder } from '../../../managers/common/nativePythonFinder';
import { mockedVSCodeNamespaces } from '../../unittests';

suite('SysPythonManager.initialize - retry after failure (throw style)', () => {
let refreshPythonsStub: sinon.SinonStub;

setup(() => {
when(mockedVSCodeNamespaces.window!.withProgress(anything(), anything())).thenCall(
(_options: any, task: any) => task({ report: sinon.stub() }, { isCancellationRequested: false }),
);
refreshPythonsStub = sinon.stub(utils, 'refreshPythons');
sinon.stub(uvInstaller, 'promptInstallPythonViaUv').resolves(undefined);
sinon.stub(cache, 'getSystemEnvForGlobal').resolves(undefined);
sinon.stub(logging, 'traceError');
sinon.stub(logging, 'traceWarn');
});

teardown(() => {
sinon.restore();
reset(mockedVSCodeNamespaces.window!);
});

function createManager(): SysPythonManager {
const api = {
getPythonProjects: sinon.stub().returns([]),
getPythonProject: sinon.stub().returns(undefined),
} as any as PythonEnvironmentApi;
return new SysPythonManager({} as NativePythonFinder, api, {
info: sinon.stub(),
error: sinon.stub(),
warn: sinon.stub(),
} as any);
}

test('rethrows on failure but clears state so a later call retries', async () => {
refreshPythonsStub.onFirstCall().rejects(new Error('discovery boom'));
refreshPythonsStub.onSecondCall().resolves([]);

const mgr = createManager();

await assert.rejects(mgr.initialize(), /discovery boom/);
assert.strictEqual(refreshPythonsStub.callCount, 1);

await assert.doesNotReject(mgr.initialize());
assert.strictEqual(refreshPythonsStub.callCount, 2, 'a later call must retry after a failure');

await mgr.initialize();
assert.strictEqual(refreshPythonsStub.callCount, 2, 'no re-discovery after a successful init');
});

test('settles concurrent waiters during a failing run (leader rejects, waiter resolves)', async () => {
refreshPythonsStub.rejects(new Error('discovery boom'));

const mgr = createManager();

const leader = mgr.initialize();
const waiter = mgr.initialize();

await assert.rejects(leader, /discovery boom/);
await assert.doesNotReject(waiter);
assert.strictEqual(refreshPythonsStub.callCount, 1, 'concurrent callers share one discovery run');
});
});
88 changes: 88 additions & 0 deletions src/test/managers/builtin/venvManager.initialize.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import assert from 'assert';
import * as sinon from 'sinon';
import { EnvironmentManager, PythonEnvironmentApi } from '../../../api';
import * as logging from '../../../common/logging';
import * as windowApis from '../../../common/window.apis';
import { VenvManager } from '../../../managers/builtin/venvManager';
import * as venvUtils from '../../../managers/builtin/venvUtils';
import { NativePythonFinder } from '../../../managers/common/nativePythonFinder';

suite('VenvManager.initialize - retry after failure (throw style)', () => {
let findVirtualEnvironmentsStub: sinon.SinonStub;

setup(() => {
findVirtualEnvironmentsStub = sinon.stub(venvUtils, 'findVirtualEnvironments');
sinon.stub(venvUtils, 'getVenvForGlobal').resolves(undefined);
sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => {
return await (task as any)({ report: sinon.stub() }, { isCancellationRequested: false } as any);
});
sinon.stub(logging, 'traceError');
sinon.stub(logging, 'traceWarn');
});

teardown(() => {
sinon.restore();
});

function createManager(): VenvManager {
const api = {
getEnvironments: sinon.stub().resolves([]),
getPythonProject: sinon.stub().returns(undefined),
getPythonProjects: sinon.stub().returns([]),
refreshEnvironments: sinon.stub().resolves(undefined),
} as any as PythonEnvironmentApi;
const baseManager = {
getEnvironments: sinon.stub().resolves([]),
} as any as EnvironmentManager;
return new VenvManager({} as NativePythonFinder, api, baseManager, {
info: sinon.stub(),
error: sinon.stub(),
warn: sinon.stub(),
} as any);
}

test('rethrows on failure but clears state so a later call retries and succeeds', async () => {
findVirtualEnvironmentsStub.onFirstCall().rejects(new Error('discovery boom'));
findVirtualEnvironmentsStub.onSecondCall().resolves([]);

const mgr = createManager();

await assert.rejects(mgr.initialize(), /discovery boom/);
assert.strictEqual(findVirtualEnvironmentsStub.callCount, 1);

await assert.doesNotReject(mgr.initialize());
assert.strictEqual(findVirtualEnvironmentsStub.callCount, 2, 'a later call must retry after a failure');

await mgr.initialize();
assert.strictEqual(findVirtualEnvironmentsStub.callCount, 2, 'no re-discovery after a successful init');
});

test('settles concurrent waiters during a failing run (leader rejects, waiter resolves)', async () => {
findVirtualEnvironmentsStub.rejects(new Error('discovery boom'));

const mgr = createManager();

const leader = mgr.initialize();
const waiter = mgr.initialize();

await assert.rejects(leader, /discovery boom/);
await assert.doesNotReject(waiter);
assert.strictEqual(findVirtualEnvironmentsStub.callCount, 1, 'concurrent callers share one discovery run');

findVirtualEnvironmentsStub.resetBehavior();
findVirtualEnvironmentsStub.resolves([]);
await assert.doesNotReject(mgr.initialize());
assert.strictEqual(findVirtualEnvironmentsStub.callCount, 2, 'a fresh call retries after failure');
});

test('does not re-run discovery after a successful initialize()', async () => {
findVirtualEnvironmentsStub.resolves([]);
const mgr = createManager();

await mgr.initialize();
await mgr.initialize();

assert.strictEqual(findVirtualEnvironmentsStub.callCount, 1);
});
});
57 changes: 57 additions & 0 deletions src/test/managers/conda/condaEnvManager.initialize.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,63 @@ suite('CondaEnvManager.initialize - lazy registration flow', () => {
assert.strictEqual(refreshCondaEnvsStub.callCount, 1);
});

test('error path is retryable: a failed run clears state so a later call retries and succeeds', async () => {
getCondaStub.resolves('/usr/bin/conda');
constructSourcingStub.resolves({ toString: () => '' } as any);
refreshCondaEnvsStub.onFirstCall().rejects(new Error('boom'));
refreshCondaEnvsStub
.onSecondCall()
.resolves([makeEnv('base', Uri.file('/opt/miniconda3').fsPath, '3.11.0')]);

const mgr = createManager();

await assert.doesNotReject(mgr.initialize(), 'initialize() must never throw to its caller');
assert.strictEqual(refreshCondaEnvsStub.callCount, 1);

await mgr.initialize();
assert.strictEqual(refreshCondaEnvsStub.callCount, 2, 'a later call must retry after a failed run');

const lazyInitCalls = sendTelemetryStub.getCalls().filter((c) => c.args[0] === EventNames.MANAGER_LAZY_INIT);
assert.strictEqual(lazyInitCalls.length, 2);
assert.strictEqual(lazyInitCalls[0].args[2].result, 'error');
assert.strictEqual(lazyInitCalls[1].args[2].result, 'success');

await mgr.initialize();
assert.strictEqual(refreshCondaEnvsStub.callCount, 2, 'no re-discovery after a successful init');
});

test('error path settles concurrent waiters without rejecting, then permits a retry', async () => {
getCondaStub.resolves('/usr/bin/conda');
constructSourcingStub.resolves({ toString: () => '' } as any);
refreshCondaEnvsStub.onFirstCall().rejects(new Error('boom'));
refreshCondaEnvsStub.onSecondCall().resolves([]);

const mgr = createManager();

const results = await Promise.allSettled([mgr.initialize(), mgr.initialize(), mgr.initialize()]);
assert.ok(
results.every((r) => r.status === 'fulfilled'),
'all concurrent waiters must settle without rejecting',
);
assert.strictEqual(refreshCondaEnvsStub.callCount, 1, 'concurrent callers share one discovery run');

await mgr.initialize();
assert.strictEqual(refreshCondaEnvsStub.callCount, 2, 'a fresh call retries after failure');
});

test('tool_not_found is treated as completed init and is not retried', async () => {
getCondaStub.rejects(new Error('Conda not found'));

const mgr = createManager();
await mgr.initialize();
await mgr.initialize();

assert.strictEqual(refreshCondaEnvsStub.callCount, 1, 'tool_not_found must not cause repeated discovery');
const lazyInitCalls = sendTelemetryStub.getCalls().filter((c) => c.args[0] === EventNames.MANAGER_LAZY_INIT);
assert.strictEqual(lazyInitCalls.length, 1);
assert.strictEqual(lazyInitCalls[0].args[2].result, 'tool_not_found');
});

test('no PET refresh is triggered before initialize(): construction alone does no work', () => {
// Simply constructing the manager must not call into discovery.
createManager();
Expand Down
Loading
Loading