diff --git a/src/common/telemetry/constants.ts b/src/common/telemetry/constants.ts index 950143de..11445a55 100644 --- a/src/common/telemetry/constants.ts +++ b/src/common/telemetry/constants.ts @@ -101,6 +101,16 @@ export enum EventNames { * - hasPersistedSelection: boolean (whether a persisted env path existed in workspace state) */ ENV_SELECTION_RESULT = 'ENV_SELECTION.RESULT', + /** + * Telemetry event fired when applyInitialEnvironmentSelection returns. + * Duration measures the blocking time (excludes deferred global scope). + * Properties: + * - globalScopeDeferred: boolean (true = global scope fired in background, false = awaited) + * - workspaceFolderCount: number (total workspace folders) + * - resolvedFolderCount: number (folders that resolved with a non-undefined env) + * - settingErrorCount: number (user-configured settings that could not be applied) + */ + ENV_SELECTION_COMPLETED = 'ENV_SELECTION.COMPLETED', /** * Telemetry event fired when a lazily-registered manager completes its first initialization. * Replaces MANAGER_REGISTRATION_SKIPPED and MANAGER_REGISTRATION_FAILED for managers @@ -395,6 +405,21 @@ export interface IEventNamePropertyMapping { hasPersistedSelection: boolean; }; + /* __GDPR__ + "env_selection.completed": { + "globalScopeDeferred": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "workspaceFolderCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" }, + "resolvedFolderCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" }, + "settingErrorCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" } + } + */ + [EventNames.ENV_SELECTION_COMPLETED]: { + globalScopeDeferred: boolean; + workspaceFolderCount: number; + resolvedFolderCount: number; + settingErrorCount: number; + }; + /* __GDPR__ "manager.lazy_init": { "managerName": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts index 4d2dce67..695b0a99 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -290,7 +290,6 @@ export async function applyInitialEnvironmentSelection( `[interpreterSelection] Applying initial environment selection for ${folders.length} workspace folder(s)`, ); - // Checkpoint 1: env selection starting — managers are registered sendTelemetryEvent(EventNames.ENV_SELECTION_STARTED, activationToReadyDurationMs, { registeredManagerCount: envManagers.managers.length, registeredManagerIds: envManagers.managers.map((m) => m.id).join(','), @@ -298,6 +297,9 @@ export async function applyInitialEnvironmentSelection( }); const allErrors: SettingResolutionError[] = []; + let workspaceFolderResolved = false; + let resolvedFolderCount = 0; + const selectionStopWatch = new StopWatch(); for (const folder of folders) { try { @@ -311,23 +313,24 @@ export async function applyInitialEnvironmentSelection( ); allErrors.push(...errors); - // Checkpoint 2: priority chain resolved — which path? - const isPathA = result.environment !== undefined; - - // Get the specific environment if not already resolved const env = result.environment ?? (await result.manager.get(folder.uri)); sendTelemetryEvent(EventNames.ENV_SELECTION_RESULT, scopeStopWatch.elapsedTime, { scope: 'workspace', prioritySource: result.source, managerId: result.manager.id, - resolutionPath: isPathA ? 'envPreResolved' : 'managerDiscovery', + resolutionPath: result.environment ? 'envPreResolved' : 'managerDiscovery', hasPersistedSelection: env !== undefined, }); // Cache only — NO settings.json write (shouldPersistSettings = false) await envManagers.setEnvironment(folder.uri, env, false); + if (env) { + workspaceFolderResolved = true; + resolvedFolderCount++; + } + traceInfo( `[interpreterSelection] ${folder.name}: ${env?.displayName ?? 'none'} (source: ${result.source})`, ); @@ -336,38 +339,69 @@ export async function applyInitialEnvironmentSelection( } } - // Also apply initial selection for global scope (no workspace folder) - // This ensures defaultInterpreterPath is respected even without a workspace - try { - const globalStopWatch = new StopWatch(); - const { result, errors } = await resolvePriorityChainCore(undefined, envManagers, undefined, nativeFinder, api); - allErrors.push(...errors); + // Resolve global scope (fallback for files outside workspace folders). + // Deferred to background when a workspace folder already resolved. + const resolveGlobalScope = async (): Promise => { + try { + const globalStopWatch = new StopWatch(); + const { result, errors: globalErrors } = await resolvePriorityChainCore( + undefined, + envManagers, + undefined, + nativeFinder, + api, + ); - const isPathA = result.environment !== undefined; + const env = result.environment ?? (await result.manager.get(undefined)); - // Get the specific environment if not already resolved - const env = result.environment ?? (await result.manager.get(undefined)); + sendTelemetryEvent(EventNames.ENV_SELECTION_RESULT, globalStopWatch.elapsedTime, { + scope: 'global', + prioritySource: result.source, + managerId: result.manager.id, + resolutionPath: result.environment ? 'envPreResolved' : 'managerDiscovery', + hasPersistedSelection: env !== undefined, + }); - sendTelemetryEvent(EventNames.ENV_SELECTION_RESULT, globalStopWatch.elapsedTime, { - scope: 'global', - prioritySource: result.source, - managerId: result.manager.id, - resolutionPath: isPathA ? 'envPreResolved' : 'managerDiscovery', - hasPersistedSelection: env !== undefined, - }); + // Cache only — NO settings.json write + await envManagers.setEnvironments('global', env, false); - // Cache only — NO settings.json write (shouldPersistSettings = false) - await envManagers.setEnvironments('global', env, false); + traceInfo(`[interpreterSelection] global: ${env?.displayName ?? 'none'} (source: ${result.source})`); - traceInfo(`[interpreterSelection] global: ${env?.displayName ?? 'none'} (source: ${result.source})`); - } catch (err) { - traceError(`[interpreterSelection] Failed to set global environment: ${err}`); + return globalErrors; + } catch (err) { + traceError(`[interpreterSelection] Failed to set global environment: ${err}`); + return []; + } + }; + + if (workspaceFolderResolved) { + // Defer global scope so it doesn't block post-selection startup. + traceInfo('[interpreterSelection] Workspace env resolved, deferring global scope to background'); + resolveGlobalScope() + .then(async (globalErrors) => { + if (globalErrors.length > 0) { + await notifyUserOfSettingErrors(globalErrors); + } + }) + .catch((err) => traceError(`[interpreterSelection] Background global scope resolution failed: ${err}`)); + } else { + // No workspace folder resolved — global scope is the primary fallback, must await. + const globalErrors = await resolveGlobalScope(); + allErrors.push(...globalErrors); } - // Notify user if any settings could not be applied + // Notify user if any settings could not be applied (workspace + global when awaited) if (allErrors.length > 0) { await notifyUserOfSettingErrors(allErrors); } + + // Duration measures blocking time only (excludes deferred global scope). + sendTelemetryEvent(EventNames.ENV_SELECTION_COMPLETED, selectionStopWatch.elapsedTime, { + globalScopeDeferred: workspaceFolderResolved, + workspaceFolderCount: folders.length, + resolvedFolderCount, + settingErrorCount: allErrors.length, + }); } /** diff --git a/src/test/features/interpreterSelection.unit.test.ts b/src/test/features/interpreterSelection.unit.test.ts index 8d80d9d6..4ba21eff 100644 --- a/src/test/features/interpreterSelection.unit.test.ts +++ b/src/test/features/interpreterSelection.unit.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import * as path from 'path'; import * as sinon from 'sinon'; import { ConfigurationChangeEvent, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; -import { PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; +import { PythonEnvironment, PythonEnvironmentApi, PythonProject, SetEnvironmentScope } from '../../api'; import * as windowApis from '../../common/window.apis'; import * as workspaceApis from '../../common/workspace.apis'; import { @@ -717,6 +717,255 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => { 'showWarningMessage should not be called when ${workspaceFolder} is only unresolvable in global scope', ); }); + + test('should catch and continue when workspace folder resolution throws', async () => { + // When manager.get() throws for a folder, the error should be caught + // and the function should still complete (falling through to awaited global scope). + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // Make venvManager.get() throw for the workspace folder + mockVenvManager.get.rejects(new Error('Simulated venv discovery failure')); + // systemManager.get() throws for workspace scope but succeeds for global scope + mockSystemManager.get.callsFake((scope: Uri | undefined) => { + if (scope) { + return Promise.reject(new Error('Simulated system discovery failure')); + } + return Promise.resolve(undefined); + }); + + // Should NOT throw — the per-folder catch block handles it + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // setEnvironment should NOT have been called for the failed folder + assert.ok( + mockEnvManagers.setEnvironment.notCalled, + 'setEnvironment should not be called when folder resolution throws', + ); + + // Global scope should still be resolved (awaited, since no workspace folder succeeded) + assert.ok( + mockEnvManagers.setEnvironments.called, + 'setEnvironments should still be called for global scope after folder failure', + ); + }); + + test('should continue processing remaining folders when one folder throws', async () => { + // In multi-root: if folder 1's resolution throws (e.g., setEnvironment fails), + // the error is caught and folder 2 should still be processed. + const uri1 = Uri.file('/workspace1'); + const uri2 = Uri.file('/workspace2'); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([ + { uri: uri1, name: 'workspace1', index: 0 }, + { uri: uri2, name: 'workspace2', index: 1 }, + ]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // setEnvironment throws for folder 1 only, succeeds for folder 2 + mockEnvManagers.setEnvironment.callsFake((scope: SetEnvironmentScope) => { + if (scope && !Array.isArray(scope) && scope.fsPath === uri1.fsPath) { + return Promise.reject(new Error('Folder 1 setEnvironment failure')); + } + return Promise.resolve(); + }); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // setEnvironment should be called for both folders (folder 1 threw, folder 2 succeeded) + assert.strictEqual( + mockEnvManagers.setEnvironment.callCount, + 2, + 'setEnvironment should be attempted for both folders', + ); + // Folder 2 succeeded — verify it was called with the correct URI + const secondCallUri = mockEnvManagers.setEnvironment.secondCall.args[0] as Uri; + assert.strictEqual(secondCallUri.fsPath, uri2.fsPath, 'Second call should be for folder 2'); + }); + + test('should show warning when pythonProjects references unregistered manager', async () => { + // Priority 1 error path: pythonProjects[] names a manager that isn't registered. + // Should fall through to auto-discovery and show a warning. + const workspaceFolder = { uri: testUri, name: 'test', index: 0 } as WorkspaceFolder; + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); + sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(workspaceFolder); + sandbox + .stub(workspaceApis, 'getConfiguration') + .returns( + createMockConfig([{ path: '.', envManager: 'ms-python.python:nonexistent' }]) as WorkspaceConfiguration, + ); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + mockProjectManager.get.returns({ uri: testUri, name: 'test' } as PythonProject); + + const showWarnStub = sandbox.stub(windowApis, 'showWarningMessage').resolves(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should still set the environment (falls through to auto-discovery) + assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called via fallback'); + + // Should show a warning about the unregistered manager + assert.ok(showWarnStub.called, 'showWarningMessage should be called for unregistered pythonProjects manager'); + }); + + test('should show warning when defaultEnvManager references unregistered manager', async () => { + // Priority 2 error path: defaultEnvManager names a manager that isn't registered. + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python-envs' && key === 'defaultEnvManager') { + return 'ms-python.python:nonexistent'; + } + return undefined; + }); + + const showWarnStub = sandbox.stub(windowApis, 'showWarningMessage').resolves(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should still set the environment (falls through to auto-discovery) + assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called via fallback'); + + // Should show a warning about the unregistered manager + assert.ok(showWarnStub.called, 'showWarningMessage should be called for unregistered defaultEnvManager'); + }); + + test('should handle global scope errors when deferred to background', async () => { + // When workspace folder resolves but global scope has setting errors, + // notifyUserOfSettingErrors should still be called from the background task. + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + + // Global scope gets a defaultEnvManager that doesn't exist → produces a SettingResolutionError + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string, scope?: Uri) => { + if (!scope && section === 'python-envs' && key === 'defaultEnvManager') { + return 'ms-python.python:nonexistent-global'; + } + return undefined; + }); + + const showWarnStub = sandbox.stub(windowApis, 'showWarningMessage').resolves(undefined); + + // Use a deferred promise to deterministically wait for the background global scope + let resolveGlobalDone!: () => void; + const globalDone = new Promise((resolve) => { + resolveGlobalDone = resolve; + }); + mockEnvManagers.setEnvironments.callsFake(async () => { + resolveGlobalDone(); + }); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Workspace folder should resolve (venv found) + assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called for workspace folder'); + + // Wait for the background global scope to call setEnvironments + await globalDone; + // Flush microtasks so the .then() handler for notifyUserOfSettingErrors runs + await new Promise((resolve) => process.nextTick(resolve)); + + // Global scope should still resolve (falls to auto-discovery) and show warning + assert.ok(mockEnvManagers.setEnvironments.called, 'setEnvironments should be called for global scope'); + assert.ok( + showWarnStub.called, + 'showWarningMessage should be called for global scope setting error even when deferred', + ); + }); + + test('should handle global scope crash when deferred to background', async () => { + // When workspace folder resolves but global scope crashes (resolveGlobalScope catch block), + // the error should be logged and not propagate. + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').returns(undefined); + + // Use a deferred promise to deterministically wait for the background global scope + let resolveGlobalDone!: () => void; + const globalDone = new Promise((resolve) => { + resolveGlobalDone = resolve; + }); + mockEnvManagers.setEnvironments.callsFake(async () => { + resolveGlobalDone(); + throw new Error('Simulated global scope crash'); + }); + + // Should NOT throw — errors are caught inside resolveGlobalScope + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Wait for the background global scope to call setEnvironments + await globalDone; + // Flush microtasks so the catch handler runs + await new Promise((resolve) => process.nextTick(resolve)); + + // Workspace folder should still have resolved + assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called for workspace folder'); + + // setEnvironments was called (and threw), proving the global scope was attempted + assert.ok( + mockEnvManagers.setEnvironments.called, + 'setEnvironments should have been attempted for global scope', + ); + }); + + test('notifyUserOfSettingErrors shows warning with Open Settings for defaultInterpreterPath', async () => { + // Trigger the defaultInterpreterPath error branch of notifyUserOfSettingErrors. + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([{ uri: testUri, name: 'test', index: 0 }]); + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '/nonexistent/python'; + } + return undefined; + }); + // nativeFinder.resolve fails — path can't be resolved + mockNativeFinder.resolve.rejects(new Error('Not found')); + + const showWarnStub = sandbox.stub(windowApis, 'showWarningMessage').resolves(undefined); + + await applyInitialEnvironmentSelection( + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + assert.ok(showWarnStub.called, 'showWarningMessage should be called for unresolvable defaultInterpreterPath'); + const warningMessage = showWarnStub.firstCall.args[0] as string; + assert.ok(warningMessage.includes('/nonexistent/python'), 'Warning message should include the configured path'); + }); }); suite('Interpreter Selection - resolveGlobalEnvironmentByPriority', () => {