Skip to content

Commit bac8824

Browse files
authored
feat: implement timeout handling for environment and package manager registration (#1295)
`waitForEnvManager` created a deferred that had no timeout, no cancellation, and no fallback- this add the timeout. This meant that if any project's configured default manager never registered (e.g., settings point to an uninstalled extension or a manager whose binary isn't found), the deferred promise would hang forever. ie **one bad project poisoned global calls** now: - A single misconfigured project can no longer hang the entire extension indefinitely - Consumers still get results from all managers that registered normally - The timeout 30s accommodate slow 3rd-party extension activation - The warning log provides diagnostics for investigating missing managers
1 parent 1558336 commit bac8824

File tree

2 files changed

+65
-8
lines changed

2 files changed

+65
-8
lines changed

src/common/telemetry/constants.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ export enum EventNames {
4949
* - errorType: string (error class name, on failure only)
5050
*/
5151
ENVIRONMENT_DISCOVERY = 'ENVIRONMENT_DISCOVERY',
52+
MANAGER_READY_TIMEOUT = 'MANAGER_READY.TIMEOUT',
5253
}
5354

5455
// Map all events to their properties
@@ -226,4 +227,15 @@ export interface IEventNamePropertyMapping {
226227
envCount?: number;
227228
errorType?: string;
228229
};
230+
231+
/* __GDPR__
232+
"manager_ready.timeout": {
233+
"managerId": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
234+
"managerKind": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }
235+
}
236+
*/
237+
[EventNames.MANAGER_READY_TIMEOUT]: {
238+
managerId: string;
239+
managerKind: 'environment' | 'package';
240+
};
229241
}

src/features/common/managerReady.ts

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
import { Disposable, l10n, Uri } from 'vscode';
2-
import { EnvironmentManagers, PythonProjectManager } from '../../internal.api';
3-
import { createDeferred, Deferred } from '../../common/utils/deferred';
42
import { allExtensions, getExtension } from '../../common/extension.apis';
5-
import { traceError, traceInfo } from '../../common/logging';
6-
import { showErrorMessage } from '../../common/window.apis';
7-
import { getDefaultEnvManagerSetting, getDefaultPkgManagerSetting } from '../settings/settingHelpers';
83
import { WorkbenchStrings } from '../../common/localize';
4+
import { traceError, traceInfo, traceWarn } from '../../common/logging';
5+
import { EventNames } from '../../common/telemetry/constants';
6+
import { sendTelemetryEvent } from '../../common/telemetry/sender';
7+
import { createDeferred, Deferred } from '../../common/utils/deferred';
8+
import { showErrorMessage } from '../../common/window.apis';
99
import { installExtension } from '../../common/workbenchCommands';
10+
import { EnvironmentManagers, PythonProjectManager } from '../../internal.api';
11+
import { getDefaultEnvManagerSetting, getDefaultPkgManagerSetting } from '../settings/settingHelpers';
12+
13+
const MANAGER_READY_TIMEOUT_MS = 30_000;
1014

1115
interface ManagerReady extends Disposable {
1216
waitForEnvManager(uris?: Uri[]): Promise<void>;
@@ -29,7 +33,10 @@ class ManagerReadyImpl implements ManagerReady {
2933
private readonly checked: Set<string> = new Set();
3034
private readonly disposables: Disposable[] = [];
3135

32-
constructor(em: EnvironmentManagers, private readonly pm: PythonProjectManager) {
36+
constructor(
37+
em: EnvironmentManagers,
38+
private readonly pm: PythonProjectManager,
39+
) {
3340
this.disposables.push(
3441
em.onDidChangeEnvironmentManager((e) => {
3542
if (this.envManagers.has(e.manager.id)) {
@@ -104,6 +111,44 @@ class ManagerReadyImpl implements ManagerReady {
104111
}
105112
}
106113

114+
/**
115+
* Wraps a deferred with a timeout so a missing/dead manager cannot block the API forever.
116+
* On timeout the deferred is resolved (not rejected) so callers proceed with degraded results
117+
* instead of hanging.
118+
*/
119+
private _withTimeout(deferred: Deferred<void>, managerId: string, kind: string): Promise<void> {
120+
if (deferred.completed) {
121+
return deferred.promise;
122+
}
123+
return new Promise<void>((resolve) => {
124+
const timer = setTimeout(() => {
125+
if (!deferred.completed) {
126+
traceWarn(
127+
`Timed out after ${MANAGER_READY_TIMEOUT_MS / 1000}s waiting for ${kind} manager "${managerId}" to register. ` +
128+
`The manager may not be installed or its extension failed to activate. Proceeding without it. ` +
129+
`To prevent this, check your "python-envs.defaultEnvManager" and "python-envs.pythonProjects" settings.`,
130+
);
131+
sendTelemetryEvent(EventNames.MANAGER_READY_TIMEOUT, undefined, {
132+
managerId,
133+
managerKind: kind as 'environment' | 'package',
134+
});
135+
deferred.resolve();
136+
}
137+
}, MANAGER_READY_TIMEOUT_MS);
138+
139+
deferred.promise.then(
140+
() => {
141+
clearTimeout(timer);
142+
resolve();
143+
},
144+
() => {
145+
clearTimeout(timer);
146+
resolve();
147+
},
148+
);
149+
});
150+
}
151+
107152
public dispose(): void {
108153
this.disposables.forEach((d) => d.dispose());
109154
this.envManagers.clear();
@@ -116,7 +161,7 @@ class ManagerReadyImpl implements ManagerReady {
116161
}
117162
const deferred = createDeferred<void>();
118163
this.envManagers.set(managerId, deferred);
119-
return deferred.promise;
164+
return this._withTimeout(deferred, managerId, 'environment');
120165
}
121166

122167
public async waitForEnvManager(uris?: Uri[]): Promise<void> {
@@ -165,7 +210,7 @@ class ManagerReadyImpl implements ManagerReady {
165210
}
166211
const deferred = createDeferred<void>();
167212
this.pkgManagers.set(managerId, deferred);
168-
return deferred.promise;
213+
return this._withTimeout(deferred, managerId, 'package');
169214
}
170215

171216
public async waitForPkgManager(uris?: Uri[]): Promise<void> {

0 commit comments

Comments
 (0)