fix: retry failed manager initialization - #24
Open
StellaHuang95 wants to merge 1 commit into
Open
Conversation
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>
Owner
Author
|
🔒 Automated review in progress — @StellaHuang95 is auto-reviewing this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
_initializeddeferred and, infinally, resolves it and leaves it set even when discovery threw. The top-of-method guardif (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 callinitialize()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 eachinitialize():const initialized = createDeferred<void>(); this._initialized = initialized;.if (this._initialized === initialized) { this._initialized = undefined; }.finally(initialized.resolve()) so concurrent waiters unblock.State transitions:
undefined -> deferred -> resolved(stays set; shared, one-time).undefined -> deferred -> resolvedwith the guard cleared (retryable).tool_not_found/ manager-absent: treated as completed init (no wasteful rediscovery).Preserved behavior:
venv,system) still rethrow; swallow-style (conda,pipenv,poetry,pyenv) still log, emit telemetry, and never throw to callers.=== initialized) stops a late failing run from clearing a guard that a concurrentclearCache()+ reinit installed.Tests
New focused
initialize()suites for venv, system, pyenv, pipenv, and poetry, plus an extended conda suite. They cover:tool_not_foundis treated as completed and is not retried;clearCache()+ reinit.