diff --git a/src/common/telemetry/constants.ts b/src/common/telemetry/constants.ts index 29d7aa8a..ae4ca4e2 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 @@ -386,6 +396,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..f714297d 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -298,6 +298,9 @@ export async function applyInitialEnvironmentSelection( }); const allErrors: SettingResolutionError[] = []; + let workspaceFolderResolved = false; + let resolvedFolderCount = 0; + const selectionStopWatch = new StopWatch(); for (const folder of folders) { try { @@ -328,6 +331,11 @@ export async function applyInitialEnvironmentSelection( // 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 +344,85 @@ 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); + // Global scope: resolve a fallback Python environment for files opened OUTSIDE all + // workspace folders (e.g., /tmp/script.py). This is NOT a workspace folder — every + // workspace folder was already fully resolved and cached in the for-loop above, + // so switching between workspace folders is unaffected by whether this runs now or later. + // + // When at least one workspace folder resolved, we defer global scope to the background + // so it doesn't block post-selection startup (clearHangWatchdog, terminal init, telemetry). + // Errors inside resolveGlobalScope are handled internally: + // - Setting resolution errors (bad paths, unknown managers) → notifyUserOfSettingErrors + // - Unexpected crashes → logged via traceError, never silently swallowed + const resolveGlobalScope = async () => { + try { + const globalStopWatch = new StopWatch(); + const { result, errors: globalErrors } = await resolvePriorityChainCore( + undefined, + envManagers, + undefined, + nativeFinder, + api, + ); - const isPathA = result.environment !== undefined; + 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: isPathA ? '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, - }); + 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}`); + if (globalErrors.length > 0) { + await notifyUserOfSettingErrors(globalErrors); + } + } catch (err) { + traceError(`[interpreterSelection] Failed to set global environment: ${err}`); + } + }; + + if (workspaceFolderResolved) { + // At least one workspace folder got a non-undefined environment (in multi-root, + // ANY folder succeeding is sufficient). ALL workspace folder envs are already + // active via setEnvironment calls in the loop — switching between folders in a + // multi-root workspace is not affected by deferring the global scope. + // Defer global scope to a background task so we don't block post-selection + // startup work in extension.ts (clearHangWatchdog, terminal init, telemetry). + // The outer .catch is a safety net — resolveGlobalScope has its own try/catch, + // so this only fires if the inner handler itself throws unexpectedly. + traceInfo('[interpreterSelection] Workspace env resolved, deferring global scope to background'); + resolveGlobalScope().catch((err) => + traceError(`[interpreterSelection] Background global scope resolution failed: ${err}`), + ); + } else { + // Either: (a) no workspace folders are open, (b) every folder resolved with + // env=undefined (no Python found), or (c) every folder threw an error. + // In all cases the global environment is the user's primary fallback, + // so we must await it before returning. + await resolveGlobalScope(); } - // Notify user if any settings could not be applied + // Notify user if any workspace-scoped settings could not be applied if (allErrors.length > 0) { await notifyUserOfSettingErrors(allErrors); } + + // Checkpoint 3: env selection function returning — duration measures blocking time only. + // If globalScopeDeferred=true, the global scope is still running in the background + // and its duration is NOT included in this measurement. + 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..927dc97a 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,235 @@ 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); + + 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 a tick for the background global scope to complete + await new Promise((resolve) => setTimeout(resolve, 50)); + + // 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); + + // Make setEnvironments throw — simulating a crash in global scope + mockEnvManagers.setEnvironments.rejects(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 a tick for the background global scope to complete + await new Promise((resolve) => setTimeout(resolve, 50)); + + // 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', () => {