Skip to content

Commit 1bd544d

Browse files
committed
improvements from comments
1 parent f78b242 commit 1bd544d

File tree

3 files changed

+47
-20
lines changed

3 files changed

+47
-20
lines changed

docs/startup-flow.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,24 +62,25 @@ python environments extension begins activation
6262
→ falls through to `await result.manager.get(scope)`
6363

6464
**--- inner fork: fast path vs slow path (tryFastPathGet in fastPath.ts) ---**
65-
three conditions checked:
65+
Conditions checked before entering fast path:
6666
a. `_initialized` deferred is undefined (never created) OR has not yet completed
6767
b. scope is a `Uri` (not global/undefined)
68-
c. a persisted env path exists in workspace state for this scope (folder Uri)
6968

70-
FAST PATH (run if above three conditions are true):
69+
FAST PATH (background init kickoff + optional early return):
7170
**Race-condition safety (runs before any await):**
7271
1. if `_initialized` doesn't exist yet:
7372
- create deferred and **register immediately** via `setInitialized()` callback
7473
- this blocks concurrent callers from spawning duplicate background inits
75-
- kick off `startBackgroundInit()` as fire-and-forget
74+
- kick off `startBackgroundInit()` as fire-and-forget
75+
- this happens as soon as (a) and (b) are true, **even if** no persisted path exists
7676
2. get project fsPath: `getProjectFsPathForScope(api, scope)`
7777
- prefers resolved project path if available, falls back to scope.fsPath
7878
- shared across all managers to avoid lambda duplication
79-
3. read persisted path
80-
4. `resolve(persistedPath)`
81-
1. failure → see SLOW PATH
82-
2. successful → return env immediately (background init continues in parallel)
79+
3. read persisted path (only if scope is a `Uri`; may return undefined)
80+
4. if a persisted path exists:
81+
- attempt `resolve(persistedPath)`
82+
- failure (no env, mismatched manager, etc.) → fall through to SLOW PATH
83+
- success → return env immediately (background init continues in parallel)
8384
**Failure recovery (in startBackgroundInit error handler):**
8485
- if background init throws: `setInitialized(undefined)` — clear deferred so next `get()` call retries init
8586

src/managers/common/fastPath.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,21 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
6666
deferred = createDeferred<void>();
6767
opts.setInitialized(deferred);
6868
const deferredRef = deferred;
69-
Promise.resolve(opts.startBackgroundInit()).then(
70-
() => deferredRef.resolve(),
71-
(err) => {
72-
traceError(`[${opts.label}] Background initialization failed: ${err}`);
73-
// Allow subsequent get()/initialize() calls to retry after a background init failure.
74-
opts.setInitialized(undefined);
75-
deferredRef.resolve();
76-
},
77-
);
69+
try {
70+
Promise.resolve(opts.startBackgroundInit()).then(
71+
() => deferredRef.resolve(),
72+
(err) => {
73+
traceError(`[${opts.label}] Background initialization failed:`, err);
74+
// Allow subsequent get()/initialize() calls to retry after a background init failure.
75+
opts.setInitialized(undefined);
76+
deferredRef.resolve();
77+
},
78+
);
79+
} catch (syncErr) {
80+
traceError(`[${opts.label}] Background initialization threw synchronously:`, syncErr);
81+
opts.setInitialized(undefined);
82+
deferredRef.resolve();
83+
}
7884
}
7985

8086
const fsPath = opts.getProjectFsPath(opts.scope);
@@ -87,9 +93,7 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
8793
return { env: resolved };
8894
}
8995
} catch (err) {
90-
traceWarn(
91-
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init: ${err}`,
92-
);
96+
traceWarn(`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`, err);
9397
}
9498
}
9599

src/test/managers/common/fastPath.unit.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,26 @@ suite('tryFastPathGet', () => {
187187

188188
assert.ok(result, 'Should resolve successfully with Thenable init');
189189
});
190+
191+
test('synchronous background init failure resets initialized for retry', async () => {
192+
const startBackgroundInit = sinon.stub().throws(new Error('init crashed sync'));
193+
const { opts, setInitialized } = createOpts({ startBackgroundInit });
194+
const result = await tryFastPathGet(opts);
195+
196+
assert.ok(result, 'Should still return resolved env even when background init throws synchronously');
197+
assert.ok(
198+
setInitialized.called,
199+
'Should set initialized before attempting background init even when it throws synchronously',
200+
);
201+
202+
// Allow any background init error handling to run.
203+
await new Promise((resolve) => setImmediate(resolve));
204+
205+
const lastCallArg = setInitialized.lastCall.args[0] as unknown;
206+
assert.strictEqual(
207+
lastCallArg,
208+
undefined,
209+
'Should clear initialized after synchronous background init failure',
210+
);
211+
});
190212
});

0 commit comments

Comments
 (0)