Skip to content

fix: retry failed manager initialization - #24

Open
StellaHuang95 wants to merge 1 commit into
mainfrom
stellahuang-microsoft-refactored-enigma
Open

fix: retry failed manager initialization#24
StellaHuang95 wants to merge 1 commit into
mainfrom
stellahuang-microsoft-refactored-enigma

Conversation

@StellaHuang95

Copy link
Copy Markdown
Owner

Summary

Make all six built-in Python environment manager initialize() methods retryable after a failed initialization attempt. A successful initialization stays shared and runs once, and each manager keeps its existing throw-vs-swallow behavior.

Problem

Every manager memoizes initialization in a _initialized deferred and, in finally, resolves it and leaves it set even when discovery threw. The top-of-method guard if (this._initialized) return this._initialized.promise; then returns that settled promise forever, so a single transient failure (e.g. a tool momentarily unavailable during the first call) permanently poisons discovery for the rest of the session with no way to retry.

Affected managers: venv, system, conda, pipenv, poetry, pyenv.

Reproduction: call initialize() while discovery fails (venv/system reject; conda/pipenv/poetry/pyenv swallow and log), then fix the environment and call initialize() again. No rediscovery happens and the manager stays empty for the session.

Root cause

finally { this._initialized.resolve(); } runs on both success and failure and never clears the guard, so the guard short-circuits every later call.

Fix

Mirror the existing reset-on-failure pattern in fastPath.ts. In each initialize():

  • Capture the deferred locally: const initialized = createDeferred<void>(); this._initialized = initialized;.
  • On an actual thrown exception, clear the guard so a later call retries, but only if this run still owns it: if (this._initialized === initialized) { this._initialized = undefined; }.
  • Always settle the captured deferred in finally (initialized.resolve()) so concurrent waiters unblock.

State transitions:

  • success: undefined -> deferred -> resolved (stays set; shared, one-time).
  • failure: undefined -> deferred -> resolved with the guard cleared (retryable).
  • non-throwing tool_not_found / manager-absent: treated as completed init (no wasteful rediscovery).

Preserved behavior:

  • throw-style (venv, system) still rethrow; swallow-style (conda, pipenv, poetry, pyenv) still log, emit telemetry, and never throw to callers.
  • the ownership check (=== initialized) stops a late failing run from clearing a guard that a concurrent clearCache() + reinit installed.

Tests

New focused initialize() suites for venv, system, pyenv, pipenv, and poetry, plus an extended conda suite. They cover:

  • a failed run clears state so a later call retries and succeeds;
  • a successful init is not re-run;
  • concurrent callers share one run and all settle (throw-style leader rejects while waiters resolve; swallow-style all resolve);
  • non-throwing tool_not_found is treated as completed and is not retried;
  • a late failing run does not clobber a newer deferred installed by clearCache() + reinit.

All six built-in manager initialize() methods memoized initialization in a
_initialized deferred and resolved it in finally even on failure, leaving the
guard set. The top guard then returned that settled promise forever, so a single
transient discovery failure permanently poisoned discovery for the rest of the
session with no way to retry.

Capture the deferred locally, clear the guard on a thrown exception only if this
run still owns it so a later call retries, and always settle the captured deferred
in finally so concurrent waiters unblock. Each manager's throw-vs-swallow behavior
is preserved, and a non-throwing tool_not_found stays a completed init.

Affected: venv, system, conda, pipenv, poetry, pyenv.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@StellaHuang95 StellaHuang95 added the bug Something isn't working label Aug 23, 2026
@StellaHuang95

StellaHuang95 commented Aug 23, 2026

Copy link
Copy Markdown
Owner Author

🔒 Automated review in progress — @StellaHuang95 is auto-reviewing this PR.

@StellaHuang95 StellaHuang95 added the review-auto:approved Automated review: no blocking findings (approval posted). label Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant