-
Notifications
You must be signed in to change notification settings - Fork 41
Don't block on global scope during ext startup #1456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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}`); | ||
| } | ||
| }; | ||
|
Comment on lines
+357
to
+389
|
||
|
|
||
| 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, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please verify the GDPR annotation key string ("env_selection.completed") matches the repository’s established telemetry/GDPR naming convention for the corresponding EventNames value ('ENV_SELECTION.COMPLETED'). If the extractor expects an exact match (or a specific normalization), a mismatch can lead to missing/invalid GDPR metadata for this event.