-
Notifications
You must be signed in to change notification settings - Fork 1
fix: add retry with backoff for setup-api-client npm install #1656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -241,27 +241,57 @@ runs: | |||||||||||||
| # lru-cache is an explicit transitive dep of @octokit/auth-app required for | ||||||||||||||
| # GitHub App token minting; pin it here so npm always hoists a specific version | ||||||||||||||
| # even if a prior cached node_modules state is missing it. | ||||||||||||||
| # Capture stderr for debugging if the command fails | ||||||||||||||
| npm_output=$(mktemp) | ||||||||||||||
| npm_cmd=(npm install --no-save --location=project \ | ||||||||||||||
| @octokit/rest@20.0.2 \ | ||||||||||||||
| @octokit/plugin-retry@6.0.1 \ | ||||||||||||||
| @octokit/plugin-paginate-rest@9.1.5 \ | ||||||||||||||
| @octokit/auth-app@6.0.3 \ | ||||||||||||||
| lru-cache@10.4.3) | ||||||||||||||
| if "${npm_cmd[@]}" 2>"$npm_output"; then | ||||||||||||||
| rm -f "$npm_output" | ||||||||||||||
| else | ||||||||||||||
| echo "::warning::npm install failed with: $(cat "$npm_output")" | ||||||||||||||
| echo "::warning::Retrying with --legacy-peer-deps" | ||||||||||||||
| # | ||||||||||||||
| # Retry with exponential backoff to survive transient npm registry errors | ||||||||||||||
| # (e.g. 403 Forbidden from CDN/rate-limit on safe-buffer, undici, etc.). | ||||||||||||||
| NPM_PACKAGES=( | ||||||||||||||
| @octokit/rest@20.0.2 | ||||||||||||||
| @octokit/plugin-retry@6.0.1 | ||||||||||||||
| @octokit/plugin-paginate-rest@9.1.5 | ||||||||||||||
| @octokit/auth-app@6.0.3 | ||||||||||||||
| lru-cache@10.4.3 | ||||||||||||||
| ) | ||||||||||||||
| NPM_MAX_RETRIES=3 | ||||||||||||||
| NPM_BACKOFF=5 # seconds; doubles each retry (5, 10) | ||||||||||||||
| npm_installed=false | ||||||||||||||
|
|
||||||||||||||
| for (( attempt=1; attempt<=NPM_MAX_RETRIES; attempt++ )); do | ||||||||||||||
| npm_output=$(mktemp) | ||||||||||||||
|
|
||||||||||||||
| if npm install --no-save --location=project "${NPM_PACKAGES[@]}" 2>"$npm_output"; then | ||||||||||||||
| rm -f "$npm_output" | ||||||||||||||
| npm_installed=true | ||||||||||||||
| break | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| npm_err=$(cat "$npm_output") | ||||||||||||||
| rm -f "$npm_output" | ||||||||||||||
| npm_cmd=(npm install --no-save --legacy-peer-deps --location=project \ | ||||||||||||||
| @octokit/rest@20.0.2 \ | ||||||||||||||
| @octokit/plugin-retry@6.0.1 \ | ||||||||||||||
| @octokit/plugin-paginate-rest@9.1.5 \ | ||||||||||||||
| @octokit/auth-app@6.0.3 \ | ||||||||||||||
| lru-cache@10.4.3) | ||||||||||||||
| "${npm_cmd[@]}" | ||||||||||||||
| echo "::warning::npm install attempt $attempt/$NPM_MAX_RETRIES failed: $npm_err" | ||||||||||||||
|
|
||||||||||||||
| # On first failure, also try --legacy-peer-deps in case it's a peer dep conflict | ||||||||||||||
| if [ "$attempt" -eq 1 ]; then | ||||||||||||||
| echo "::warning::Retrying with --legacy-peer-deps" | ||||||||||||||
| npm_output=$(mktemp) | ||||||||||||||
| if npm install --no-save --legacy-peer-deps --location=project "${NPM_PACKAGES[@]}" 2>"$npm_output"; then | ||||||||||||||
| rm -f "$npm_output" | ||||||||||||||
| npm_installed=true | ||||||||||||||
| break | ||||||||||||||
| fi | ||||||||||||||
| npm_err_legacy=$(cat "$npm_output") | ||||||||||||||
| rm -f "$npm_output" | ||||||||||||||
|
Comment on lines
+279
to
+281
|
||||||||||||||
| fi | |
| rm -f "$npm_output" | |
| fi | |
| npm_err_legacy=$(cat "$npm_output") | |
| rm -f "$npm_output" | |
| echo "::warning::npm install with --legacy-peer-deps failed: $npm_err_legacy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backoff comment/behavior doesn’t match the stated plan: with
NPM_MAX_RETRIES=3the loop only sleeps twice (5s then 10s) and never reaches 20s, even though the comment says "(5, 10, 20)" and the PR description mentions 5/10/20. Either increase the retry count (e.g. 4 attempts / 3 retries) or adjust the comment/plan so it accurately reflects the actual delays.