Skip to content

Commit e192ffe

Browse files
authored
fix: create independence between manager registration (#1296)
ensure that one manager failing registration does not stop the ability for the rest of the process of setup to succeed
1 parent bac8824 commit e192ffe

File tree

4 files changed

+102
-7
lines changed

4 files changed

+102
-7
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/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: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
sendProjectStructureTelemetry,
2525
} from './common/telemetry/helpers';
2626
import { sendTelemetryEvent } from './common/telemetry/sender';
27+
import { safeRegister } from './common/utils/asyncUtils';
2728
import { createDeferred } from './common/utils/deferred';
2829

2930
import {
@@ -527,13 +528,23 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
527528
context.subscriptions.push(nativeFinder);
528529
const sysMgr = new SysPythonManager(nativeFinder, api, outputChannel);
529530
sysPythonManager.resolve(sysMgr);
531+
// Each manager registers independently — one failure must not block the others.
530532
await Promise.all([
531-
registerSystemPythonFeatures(nativeFinder, context.subscriptions, outputChannel, sysMgr),
532-
registerCondaFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
533-
registerPyenvFeatures(nativeFinder, context.subscriptions, projectManager),
534-
registerPipenvFeatures(nativeFinder, context.subscriptions, projectManager),
535-
registerPoetryFeatures(nativeFinder, context.subscriptions, outputChannel, projectManager),
536-
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()),
537548
]);
538549

539550
await applyInitialEnvironmentSelection(envManagers, projectManager, nativeFinder, api);
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)