Retry binary downloads on transient HTTP failures

[X-Guardian]

Fixes #694.

Summary

place_binary made exactly one download attempt, so a transient 5xx, a 429, or a dropped connection forced a source compile even when the request may have succeeded moments later. This PR adds bounded retries with exponential backoff around the download request.

It also sets statusCode on HTTP download errors. print_fallback_error already branches on that property to produce its Tried to download(<status>) diagnostic, but the thrown error never carried it, so every HTTP failure took the generic "not installable" branch. That fix is included here rather than split out because the feature depends on it: once a download can fail after several attempts, the fallback message needs to distinguish an exhausted 5xx from an instant 404.

What changed

• fetch_with_retry() wraps the download request, retrying transient failures with exponential backoff and full jitter. It returns the Response unchanged for any non-retryable outcome, so the 403-authenticated path and the extraction logic below it are untouched.
• resolve_retry_opts() resolves the new settings from an explicit option, then npm config, then the default, mirroring how proxy and cafile are sourced.
• Errors thrown for a non-2xx response now carry statusCode.
• Three new options, wired through configDefs and copied onto opts alongside the existing ca/cafile copy.

What is retried

Outcome	Retried	Reason
429, 5xx	Yes	Rate limiting and server-side errors are transient
ECONNRESET, ECONNREFUSED, ETIMEDOUT, EPIPE, ENETUNREACH, EHOSTUNREACH	Yes	Transient transport failures
EAI_AGAIN	Yes	Explicitly a temporary DNS failure
socket hang up, node-fetch request-timeout / body-timeout	Yes	Server closed, never responded, or stalled
404	No	No binary for this platform/ABI; must fall through to build immediately
403	No	Handled by the existing authenticated download path
Other 4xx	No	The request itself is wrong; retrying cannot fix it
ENOTFOUND	No	Almost always a misconfigured binary.host; treated as permanent
AbortError	No	Deliberate cancellation
Any failure after a 200, mid-stream	No	See below

The 404 and 403 exclusions are deliberate and load-bearing for install performance, not just correctness. A module with no pre-built binary for the running ABI hits the 404 path on every install; adding retry delay there would slow down every such install everywhere. Both are covered by tests.

Retrying the request, never the stream

The retry loop wraps the request and its status check only. Once the response body is piped into tar.extract, the download is committed.

A mid-stream failure has already written partial files into opts.module_path. Retrying without cleanup would extract over a half-written tree, and any file the first attempt created but the second did not reach would be left truncated. A truncated .node would then satisfy the existsAsync(binary_module) check on a subsequent install and fail later at require time, which is a considerably worse failure than the one being fixed.

Handling that safely means extracting to a temp directory and renaming on success. That is a larger change and is deliberately out of scope here; the constraint is commented in the code so it is not "fixed" later without that context.

Options

Option	npm config	Default
--retries=<n>	node_pre_gyp_retries	2 (3 attempts total)
--retry_delay=<ms>	node_pre_gyp_retry_delay	1000
--timeout=<ms>	node_pre_gyp_timeout	30000

--retries=0 disables retrying entirely, for CI that would rather fail fast and compile.

Defaults are conservative and match npm's own fetch-retries default of 2. Backoff uses full jitter rather than the delay/2 + random variant, because npm installs many packages in parallel and full jitter is what actually spreads out a fleet of machines all pulling from one struggling host.

The per-attempt timeout bounds the worst case against a host that is slow to fail, such as the reported 504 that took around 11 seconds, and it makes a fully hung connection retryable rather than an indefinite stall.

It applies to the response headers only, not the body transfer, so a slow but progressing download of a large binary is never killed part-way. That follows from the response body being piped straight into tar rather than buffered: node-fetch only arms its body timeout inside consumeBody(), which this path never calls. Worth knowing if anyone later changes the download to buffer the response, since that would start enforcing the timeout across the whole transfer. body-timeout is treated as retryable so the behaviour would stay correct if that happened.

User-visible output

Before, for the transient 504 in the linked issue:

[error] install response status 504 Gateway Time-out on https://github.com/.../node-v137-linux-x64-glibc.tar.gz
[warn]  Pre-built binaries not installable for muhammara@6.0.5 and node@24.19.0 (node-v137 ABI, glibc) (falling back to source compile with node-gyp)
[warn]  Hit error response status 504 Gateway Time-out on https://github.com/.../node-v137-linux-x64-glibc.tar.gz

After, in the common case the retry succeeds and the install completes with a single warning:

[warn]  install retrying download (attempt 2/3) in 743ms: response status 504 Gateway Time-out

If every attempt fails, the fallback message is now the status-aware one that print_fallback_error was always intended to produce:

[warn]  install retrying download (attempt 2/3) in 743ms: response status 504 Gateway Time-out
[warn]  install retrying download (attempt 3/3) in 1620ms: response status 504 Gateway Time-out
[error] install response status 504 Gateway Time-out on https://github.com/.../node-v137-linux-x64-glibc.tar.gz
[warn]  Tried to download(504): https://github.com/.../node-v137-linux-x64-glibc.tar.gz
[warn]  Pre-built binaries not found for muhammara@6.0.5 and node@24.19.0 (node-v137 ABI, glibc) (falling back to source compile with node-gyp)

Retry warnings are logged at warn so they are visible at the default loglevel: a silent multi-second stall during npm install is confusing, and a silent successful retry would hide a degrading binary host.

No new dependencies

Nothing retry-related was added. make-fetch-happen would replace node-fetch and pull in roughly ten transitive packages, which is a transport rewrite rather than an addition. p-retry and promise-retry would each add a dependency (plus retry) to save about 25 lines, and neither handles the part that actually matters here: deciding retryability from a node-fetch Response versus a FetchError, and leaving 403 alone so the authenticated path still fires immediately.

Given node-pre-gyp is an install-time dependency of a great many native addons, the bar for adding to that tree is high. The logic is small enough to keep local, and the repo already hand-rolls its proxy agent and S3 setup.

Tests

New test/retry.test.js, following the existing nock and tape patterns in test/private-binary.test.js. 87 assertions covering:

• 500, 503, 504 and 429 retried to eventual success
• retries exhausted, asserting the attempt count and statusCode
• 404, 403 and ENOTFOUND not retried, verified by leaving a follow-up interceptor unconsumed
• network errors retried, both with a transient code and as a bare socket hang up
• per-attempt timeout retried via delayConnection
• retries: 0 disabling retries
• config resolution: defaults, explicit options, npm-style string config, and malformed or negative values falling back to defaults rather than producing NaN
• the retryability decision table and backoff bounds directly

The integration tests pass their retry settings through the same gyp.opts path the feature uses, so a broken config copy surfaces as a slow or failing test rather than a silent pass. versioning.evaluate() builds a fresh opts object and discards anything it does not read, which is why the new settings are copied explicitly alongside ca/cafile. The existing opts.proxy has that latent bug today and is never populated.

[ox-security]

OX Security reviewed this pull request — nothing to fix.

✔ No issues found

Branch feat/download-retry → master

View scan in OX Security →