Skip to content

Commit b10cb5c

Browse files
committed
Merge remote-tracking branch 'upstream/main' into gothic-vicuna
# Conflicts: # src/extension.ts # src/internal.api.ts
2 parents 0a0706f + e192ffe commit b10cb5c

File tree

9 files changed

+234
-19
lines changed

9 files changed

+234
-19
lines changed

.vscode/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"git.branchProtectionPrompt": "alwaysCommitToNewBranch",
3232
"chat.tools.terminal.autoApprove": {
3333
"npx tsc": true,
34-
"mkdir": true
34+
"mkdir": true,
35+
"npx mocha": true
3536
}
3637
}

src/common/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export const ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs';
44
export const PYTHON_EXTENSION_ID = 'ms-python.python';
55
export const JUPYTER_EXTENSION_ID = 'ms-toolsai.jupyter';
66
export const EXTENSION_ROOT_DIR = path.dirname(__dirname);
7+
export const ISSUES_URL = 'https://github.com/microsoft/vscode-python-environments/issues';
78

89
export const DEFAULT_PACKAGE_MANAGER_ID = 'ms-python.python:pip';
910
export const DEFAULT_ENV_MANAGER_ID = 'ms-python.python:venv';

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
@@ -227,4 +228,15 @@ export interface IEventNamePropertyMapping {
227228
envCount?: number;
228229
errorType?: string;
229230
};
231+
232+
/* __GDPR__
233+
"manager_ready.timeout": {
234+
"managerId": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
235+
"managerKind": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }
236+
}
237+
*/
238+
[EventNames.MANAGER_READY_TIMEOUT]: {
239+
managerId: string;
240+
managerKind: 'environment' | 'package';
241+
};
230242
}

src/common/telemetry/helpers.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { getDefaultEnvManagerSetting, getDefaultPkgManagerSetting } from '../../features/settings/settingHelpers';
22
import { EnvironmentManagers, PythonProjectManager } from '../../internal.api';
33
import { getUvEnvironments } from '../../managers/builtin/uvEnvironments';
4-
import { traceVerbose } from '../logging';
4+
import { ISSUES_URL } from '../constants';
5+
import { traceInfo, traceVerbose, traceWarn } from '../logging';
56
import { getWorkspaceFolders } from '../workspace.apis';
67
import { EventNames } from './constants';
78
import { sendTelemetryEvent } from './sender';
@@ -154,3 +155,37 @@ export async function sendEnvironmentToolUsageTelemetry(
154155
traceVerbose('Failed to send environment tool usage telemetry:', error);
155156
}
156157
}
158+
159+
/**
160+
* Logs a summary of environment discovery results after startup.
161+
* If no environments are found, logs guidance to help users troubleshoot.
162+
*/
163+
export async function logDiscoverySummary(envManagers: EnvironmentManagers): Promise<void> {
164+
const managers = envManagers.managers;
165+
let totalEnvCount = 0;
166+
const managerSummaries: string[] = [];
167+
168+
for (const manager of managers) {
169+
try {
170+
const envs = await manager.getEnvironments('all');
171+
totalEnvCount += envs.length;
172+
if (envs.length > 0) {
173+
managerSummaries.push(`${manager.displayName}: ${envs.length}`);
174+
}
175+
} catch {
176+
// Discovery errors are already logged by InternalEnvironmentManager.refresh()
177+
}
178+
}
179+
180+
if (totalEnvCount === 0) {
181+
traceWarn(
182+
`No Python environments were found. ` +
183+
`Try running "Python Environments: Run Python Environment Tool (PET) in Terminal..." from the Command Palette to diagnose. ` +
184+
`If environments should be detected, please report this: ${ISSUES_URL}/new`,
185+
);
186+
} else {
187+
traceInfo(
188+
`Environment discovery complete: ${totalEnvCount} environments found (${managerSummaries.join(', ')})`,
189+
);
190+
}
191+
}

src/common/utils/asyncUtils.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
import { traceError } from '../logging';
2+
13
export async function timeout(milliseconds: number): Promise<void> {
24
return new Promise<void>((resolve) => setTimeout(resolve, milliseconds));
35
}
6+
7+
/**
8+
* Wraps a promise so that rejection is caught and logged instead of propagated.
9+
* Use with `Promise.all` to run tasks independently — one failure won't block the others.
10+
*/
11+
export async function safeRegister(name: string, task: Promise<void>): Promise<void> {
12+
try {
13+
await task;
14+
} catch (error) {
15+
traceError(`Failed to register ${name} features:`, error);
16+
}
17+
}

src/extension.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ import { newProjectSelection } from './common/pickers/managers';
1818
import { StopWatch } from './common/stopWatch';
1919
import { EventNames } from './common/telemetry/constants';
2020
import {
21+
logDiscoverySummary,
2122
sendEnvironmentToolUsageTelemetry,
2223
sendManagerSelectionTelemetry,
2324
sendProjectStructureTelemetry,
2425
} from './common/telemetry/helpers';
2526
import { sendTelemetryEvent } from './common/telemetry/sender';
27+
import { safeRegister } from './common/utils/asyncUtils';
2628
import { createDeferred } from './common/utils/deferred';
2729

2830
import {
@@ -526,13 +528,23 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
526528
context.subscriptions.push(nativeFinder);
527529
const sysMgr = new SysPythonManager(nativeFinder, api, outputChannel);
528530
sysPythonManager.resolve(sysMgr);
531+
// Each manager registers independently — one failure must not block the others.
529532
await Promise.all([
530-
registerSystemPythonFeatures(nativeFinder, context.subscriptions, outputChannel, sysMgr),
531-
registerCondaFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
532-
registerPyenvFeatures(nativeFinder, context.subscriptions, projectManager),
533-
registerPipenvFeatures(nativeFinder, context.subscriptions, projectManager),
534-
registerPoetryFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
535-
shellStartupVarsMgr.initialize(),
533+
safeRegister(
534+
'system',
535+
registerSystemPythonFeatures(nativeFinder, context.subscriptions, outputChannel, sysMgr),
536+
),
537+
safeRegister(
538+
'conda',
539+
registerCondaFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
540+
),
541+
safeRegister('pyenv', registerPyenvFeatures(nativeFinder, context.subscriptions, projectManager)),
542+
safeRegister('pipenv', registerPipenvFeatures(nativeFinder, context.subscriptions, projectManager)),
543+
safeRegister(
544+
'poetry',
545+
registerPoetryFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
546+
),
547+
safeRegister('shellStartupVars', shellStartupVarsMgr.initialize()),
536548
]);
537549

538550
await applyInitialEnvironmentSelection(envManagers, projectManager, nativeFinder, api);
@@ -550,7 +562,10 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
550562
await terminalManager.initialize(api);
551563
sendManagerSelectionTelemetry(projectManager);
552564
await sendProjectStructureTelemetry(projectManager, envManagers);
553-
await sendEnvironmentToolUsageTelemetry(projectManager, envManagers);
565+
await sendEnvironmentToolUsageTelemetry(projectManager, envManagers);
566+
567+
// Log discovery summary to help users troubleshoot environment detection issues
568+
await logDiscoverySummary(envManagers);
554569
} catch (postInitError) {
555570
traceError('Post-initialization tasks failed:', postInitError);
556571
}

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> {

src/internal.api.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ import {
2727
ResolveEnvironmentContext,
2828
SetEnvironmentScope,
2929
} from './api';
30+
import { ISSUES_URL } from './common/constants';
3031
import { CreateEnvironmentNotSupported, RemoveEnvironmentNotSupported } from './common/errors/NotSupportedError';
32+
import { traceWarn } from './common/logging';
3133
import { StopWatch } from './common/stopWatch';
3234
import { EventNames } from './common/telemetry/constants';
3335
import { sendTelemetryEvent } from './common/telemetry/sender';
@@ -205,26 +207,47 @@ export class InternalEnvironmentManager implements EnvironmentManager {
205207

206208
async refresh(options: RefreshEnvironmentsScope): Promise<void> {
207209
const sw = new StopWatch();
210+
const SLOW_DISCOVERY_THRESHOLD_MS = 15000;
208211
try {
209212
await this.manager.refresh(options);
210213
const envs = await this.manager.getEnvironments('all').catch(() => []);
211-
sendTelemetryEvent(EventNames.ENVIRONMENT_DISCOVERY, sw.elapsedTime, {
214+
const duration = sw.elapsedTime;
215+
sendTelemetryEvent(EventNames.ENVIRONMENT_DISCOVERY, duration, {
212216
managerId: this.id,
213217
result: 'success',
214218
envCount: envs.length,
215219
});
220+
221+
// Log warning for slow discovery
222+
if (duration > SLOW_DISCOVERY_THRESHOLD_MS) {
223+
traceWarn(
224+
`[${this.displayName}] Environment discovery took ${(duration / 1000).toFixed(1)}s (found ${envs.length} environments). ` +
225+
`If this is causing problems, please report it: ${ISSUES_URL}/new`,
226+
);
227+
}
216228
} catch (ex) {
229+
const duration = sw.elapsedTime;
217230
const errorType = classifyError(ex);
218231
sendTelemetryEvent(
219232
EventNames.ENVIRONMENT_DISCOVERY,
220-
sw.elapsedTime,
233+
duration,
221234
{
222235
managerId: this.id,
223236
result: errorType === 'canceled' || errorType === 'spawn_timeout' ? 'timeout' : 'error',
224237
errorType,
225238
},
226239
ex instanceof Error ? ex : undefined,
227240
);
241+
242+
// Log verbose failure message to help users report issues
243+
const errorMessage = ex instanceof Error ? ex.message : String(ex);
244+
traceWarn(
245+
`[${this.displayName}] Environment discovery failed after ${(duration / 1000).toFixed(1)}s.\n` +
246+
` Error: ${errorType} - ${errorMessage}\n` +
247+
` If environments are not being detected correctly, please report this issue:\n` +
248+
` ${ISSUES_URL}/new`,
249+
);
250+
228251
throw ex;
229252
}
230253
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import assert from 'assert';
2+
import * as sinon from 'sinon';
3+
import * as logging from '../../common/logging';
4+
import { safeRegister } from '../../common/utils/asyncUtils';
5+
6+
suite('safeRegister', () => {
7+
let traceErrorStub: sinon.SinonStub;
8+
9+
setup(() => {
10+
traceErrorStub = sinon.stub(logging, 'traceError');
11+
});
12+
13+
teardown(() => {
14+
sinon.restore();
15+
});
16+
17+
test('resolves when the task succeeds', async () => {
18+
await safeRegister('test-manager', Promise.resolve());
19+
assert.ok(traceErrorStub.notCalled, 'traceError should not be called on success');
20+
});
21+
22+
test('resolves (not rejects) when the task fails', async () => {
23+
const failing = Promise.reject(new Error('boom'));
24+
// safeRegister must not propagate the rejection
25+
await safeRegister('failing-manager', failing);
26+
// If we got here without throwing, the test passes
27+
});
28+
29+
test('logs the manager name and error when the task fails', async () => {
30+
const error = new Error('registration exploded');
31+
await safeRegister('conda', Promise.reject(error));
32+
33+
assert.ok(traceErrorStub.calledOnce, 'traceError should be called once');
34+
const [message, loggedError] = traceErrorStub.firstCall.args;
35+
assert.ok(message.includes('conda'), 'log message should contain the manager name');
36+
assert.strictEqual(loggedError, error, 'original error should be passed through');
37+
});
38+
39+
test('independent tasks continue when one fails', async () => {
40+
const results: string[] = [];
41+
42+
await Promise.all([
43+
safeRegister('will-fail', Promise.reject(new Error('fail'))),
44+
safeRegister(
45+
'will-succeed-1',
46+
Promise.resolve().then(() => {
47+
results.push('a');
48+
}),
49+
),
50+
safeRegister(
51+
'will-succeed-2',
52+
Promise.resolve().then(() => {
53+
results.push('b');
54+
}),
55+
),
56+
]);
57+
58+
assert.deepStrictEqual(results.sort(), ['a', 'b'], 'both successful tasks should complete');
59+
assert.ok(traceErrorStub.calledOnce, 'only the failing task should log an error');
60+
});
61+
62+
test('handles non-Error rejections', async () => {
63+
await safeRegister('string-reject', Promise.reject('just a string'));
64+
65+
assert.ok(traceErrorStub.calledOnce);
66+
const [, loggedError] = traceErrorStub.firstCall.args;
67+
assert.strictEqual(loggedError, 'just a string');
68+
});
69+
});

0 commit comments

Comments
 (0)