Skip to content

Commit 56fd76c

Browse files
committed
fixes
1 parent 40fd6c1 commit 56fd76c

2 files changed

Lines changed: 44 additions & 17 deletions

File tree

src/features/interpreterSelection.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ export async function applyInitialEnvironmentSelection(
354354
// Errors inside resolveGlobalScope are handled internally:
355355
// - Setting resolution errors (bad paths, unknown managers) → notifyUserOfSettingErrors
356356
// - Unexpected crashes → logged via traceError, never silently swallowed
357-
const resolveGlobalScope = async () => {
357+
const resolveGlobalScope = async (): Promise<SettingResolutionError[]> => {
358358
try {
359359
const globalStopWatch = new StopWatch();
360360
const { result, errors: globalErrors } = await resolvePriorityChainCore(
@@ -380,11 +380,10 @@ export async function applyInitialEnvironmentSelection(
380380

381381
traceInfo(`[interpreterSelection] global: ${env?.displayName ?? 'none'} (source: ${result.source})`);
382382

383-
if (globalErrors.length > 0) {
384-
await notifyUserOfSettingErrors(globalErrors);
385-
}
383+
return globalErrors;
386384
} catch (err) {
387385
traceError(`[interpreterSelection] Failed to set global environment: ${err}`);
386+
return [];
388387
}
389388
};
390389

@@ -395,21 +394,26 @@ export async function applyInitialEnvironmentSelection(
395394
// multi-root workspace is not affected by deferring the global scope.
396395
// Defer global scope to a background task so we don't block post-selection
397396
// startup work in extension.ts (clearHangWatchdog, terminal init, telemetry).
398-
// The outer .catch is a safety net — resolveGlobalScope has its own try/catch,
399-
// so this only fires if the inner handler itself throws unexpectedly.
397+
// Global errors are notified separately since we can't aggregate with workspace errors.
400398
traceInfo('[interpreterSelection] Workspace env resolved, deferring global scope to background');
401-
resolveGlobalScope().catch((err) =>
402-
traceError(`[interpreterSelection] Background global scope resolution failed: ${err}`),
403-
);
399+
resolveGlobalScope()
400+
.then(async (globalErrors) => {
401+
if (globalErrors.length > 0) {
402+
await notifyUserOfSettingErrors(globalErrors);
403+
}
404+
})
405+
.catch((err) => traceError(`[interpreterSelection] Background global scope resolution failed: ${err}`));
404406
} else {
405407
// Either: (a) no workspace folders are open, (b) every folder resolved with
406408
// env=undefined (no Python found), or (c) every folder threw an error.
407409
// In all cases the global environment is the user's primary fallback,
408-
// so we must await it before returning.
409-
await resolveGlobalScope();
410+
// so we must await it before returning. Errors are aggregated with workspace
411+
// errors so notifyUserOfSettingErrors can dedupe across both scopes.
412+
const globalErrors = await resolveGlobalScope();
413+
allErrors.push(...globalErrors);
410414
}
411415

412-
// Notify user if any workspace-scoped settings could not be applied
416+
// Notify user if any settings could not be applied (workspace + global when awaited)
413417
if (allErrors.length > 0) {
414418
await notifyUserOfSettingErrors(allErrors);
415419
}

src/test/features/interpreterSelection.unit.test.ts

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,16 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => {
868868

869869
const showWarnStub = sandbox.stub(windowApis, 'showWarningMessage').resolves(undefined);
870870

871+
// Use a deferred promise to deterministically wait for the background global scope
872+
let resolveGlobalDone!: () => void;
873+
const globalDone = new Promise<void>((resolve) => {
874+
resolveGlobalDone = resolve;
875+
});
876+
const origSetEnvironments = mockEnvManagers.setEnvironments;
877+
origSetEnvironments.callsFake(async (...args: unknown[]) => {
878+
resolveGlobalDone();
879+
});
880+
871881
await applyInitialEnvironmentSelection(
872882
mockEnvManagers as unknown as EnvironmentManagers,
873883
mockProjectManager as unknown as PythonProjectManager,
@@ -878,8 +888,10 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => {
878888
// Workspace folder should resolve (venv found)
879889
assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called for workspace folder');
880890

881-
// Wait a tick for the background global scope to complete
882-
await new Promise((resolve) => setTimeout(resolve, 50));
891+
// Wait for the background global scope to call setEnvironments
892+
await globalDone;
893+
// Flush microtasks so the .then() handler for notifyUserOfSettingErrors runs
894+
await new Promise<void>((resolve) => process.nextTick(resolve));
883895

884896
// Global scope should still resolve (falls to auto-discovery) and show warning
885897
assert.ok(mockEnvManagers.setEnvironments.called, 'setEnvironments should be called for global scope');
@@ -896,8 +908,17 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => {
896908
sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration);
897909
sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined);
898910

911+
// Use a deferred promise to deterministically wait for the background global scope
912+
let resolveGlobalDone!: () => void;
913+
const globalDone = new Promise<void>((resolve) => {
914+
resolveGlobalDone = resolve;
915+
});
916+
899917
// Make setEnvironments throw — simulating a crash in global scope
900-
mockEnvManagers.setEnvironments.rejects(new Error('Simulated global scope crash'));
918+
mockEnvManagers.setEnvironments.callsFake(async () => {
919+
resolveGlobalDone();
920+
throw new Error('Simulated global scope crash');
921+
});
901922

902923
// Should NOT throw — errors are caught inside resolveGlobalScope
903924
await applyInitialEnvironmentSelection(
@@ -907,8 +928,10 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => {
907928
mockApi as unknown as PythonEnvironmentApi,
908929
);
909930

910-
// Wait a tick for the background global scope to complete
911-
await new Promise((resolve) => setTimeout(resolve, 50));
931+
// Wait for the background global scope to call setEnvironments
932+
await globalDone;
933+
// Flush microtasks so the catch handler runs
934+
await new Promise<void>((resolve) => process.nextTick(resolve));
912935

913936
// Workspace folder should still have resolved
914937
assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called for workspace folder');

0 commit comments

Comments
 (0)