Retry binary downloads on transient HTTP failures

CHANGELOG.md

@@ -1,6 +1,8 @@
 # node-pre-gyp changelog
 
 ## master
+- Retry binary downloads on transient failures (HTTP 429/5xx and connection-level errors) with exponential backoff and full jitter. Configurable via `--retries` (default 2), `--retry_delay` (default 1000ms) and `--timeout` (default 30000ms).
+- HTTP download errors now carry `statusCode`, which restores the intended `Tried to download(<status>)` diagnostic in the fallback message. Previously every HTTP failure reported the generic "Pre-built binaries not installable" / "Hit error" text.
 
 ## 2.0.4-pre.0
 - Test the release workflow

README.md

@@ -86,6 +86,13 @@ Options include:
  - `--target_arch=ia32`: Pass the target arch and override the host `arch`. Any value that is [supported by Node.js](https://nodejs.org/api/os.html#osarch) is valid.
  - `--target_platform=win32`: Pass the target platform and override the host `platform`. Valid values are `linux`, `darwin`, `win32`, `sunos`, `freebsd`, `openbsd`, and `aix`.
  - `--acl=<acl>`: Set the S3 ACL when publishing binaries (e.g., `public-read`, `private`). Overrides the `binary.acl` setting in package.json.
+ - `--retries=<n>`: Number of times to retry a failed binary download, after the first attempt (default: `2`). Pass `--retries=0` to disable retrying and fall back to a source build immediately.
+ - `--retry_delay=<ms>`: Base delay in milliseconds for the exponential backoff between download retries (default: `1000`).
+ - `--timeout=<ms>`: Per-attempt timeout in milliseconds for a binary download request (default: `30000`). This applies to the response headers, not the body transfer, so a large binary downloading slowly is not interrupted. Pass `--timeout=0` to disable it.
+
+Downloads are retried on transient failures only: HTTP `429` and `5xx` responses, and connection-level errors such as `ECONNRESET` or a socket hang up. A `404` (no pre-built binary for this platform and ABI) and a `403` (private binary, handled by the authenticated download path) are never retried, so they still fall back to a source build immediately. Backoff uses full jitter, which spreads out retries when many packages are installed in parallel.
+
+These three options can also be set as npm config, in which case they take the `node_pre_gyp_` prefix — for example `npm config set node_pre_gyp_retries 5`, or `node_pre_gyp_retries=5 npm install`.
 
 Both `--build-from-source` and `--fallback-to-build` can be passed alone or they can provide values. You can pass `--fallback-to-build=false` to override the option as declared in package.json. In addition to being able to pass `--build-from-source` you can also pass `--build-from-source=myapp` where `myapp` is the name of your module.
 

lib/install.js

@@ -25,6 +25,158 @@ try {
   // do nothing
 }
 
+const DEFAULT_RETRIES = 2;
+const DEFAULT_RETRY_DELAY = 1000;
+const MAX_RETRY_DELAY = 30000;
+const DEFAULT_TIMEOUT = 30000;
+
+/**
+ * Network error codes worth retrying. node-fetch v2 wraps system errors in a FetchError that copies `code` off the
+ * underlying error. ENOTFOUND is deliberately excluded: a DNS miss is almost always a misconfigured binary.host, so
+ * retrying it only delays the fallback-to-build. EAI_AGAIN is included because it is explicitly a temporary failure.
+ */
+const RETRYABLE_NETWORK_CODES = new Set([
+  'ECONNRESET',
+  'ECONNREFUSED',
+  'ETIMEDOUT',
+  'EPIPE',
+  'EAI_AGAIN',
+  'ENETUNREACH',
+  'EHOSTUNREACH'
+]);
+
+/**
+ * Determines whether an HTTP status is worth retrying. 429 is rate limiting and 5xx is server-side, both transient.
+ *
+ * @param {number} status HTTP status code from the response.
+ * @returns {boolean} True if the request should be retried.
+ */
+function is_retryable_status(status) {
+  return status === 429 || (status >= 500 && status <= 599);
+}
+
+/**
+ * Determines whether a thrown request error is transient. Matches node-fetch's timeout types, the known transient
+ * system codes, and a bare socket hangup, which node-fetch surfaces with no usable code.
+ *
+ * @param {Error} err Error thrown by fetch, typically a node-fetch FetchError.
+ * @returns {boolean} True if the request should be retried.
+ */
+function is_retryable_error(err) {
+  if (!err) return false;
+  if (err.name === 'AbortError') return false;
+  if (err.type === 'request-timeout' || err.type === 'body-timeout') return true;
+  if (RETRYABLE_NETWORK_CODES.has(err.code)) return true;
+  if (typeof err.message === 'string' && err.message.includes('socket hang up')) return true;
+  return false;
+}
+
+/**
+ * Calculates an exponential backoff delay using full jitter, so the sleep is uniform in [0, base * 2^attempt] up to
+ * MAX_RETRY_DELAY.
+ *
+ * @param {number} attempt Zero-based index of the retry about to be made.
+ * @param {number} baseDelay Base delay in milliseconds.
+ * @returns {number} Delay in milliseconds to wait before the next attempt.
+ */
+function backoff_delay(attempt, baseDelay) {
+  const ceiling = Math.min(baseDelay * Math.pow(2, attempt), MAX_RETRY_DELAY);
+  return Math.floor(Math.random() * ceiling);
+}
+
+/**
+ * Promisified setTimeout.
+ *
+ * @param {number} ms Milliseconds to wait.
+ * @returns {Promise<void>} Resolves once the delay has elapsed.
+ */
+function sleep(ms) {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
+/**
+ * Resolves download retry settings, mirroring how proxy and cafile are sourced: an explicit option wins, then the
+ * npm-inherited config, then the default.
+ *
+ * Values arriving from npm_config_* are arbitrary strings, so each is range-checked after parsing. An unparseable or
+ * negative value falls back to its default. Note that 0 is meaningful and is honoured for all three: no retries, no
+ * backoff, and, following node-fetch's own convention for `timeout`, no timeout.
+ *
+ * @param {Object} opts Options object as built by versioning.evaluate and topped up by install().
+ * @returns {{retries: number, retryDelay: number, timeout: number}} Validated retry settings.
+ */
+function resolve_retry_opts(opts) {
+  const pick = (...vals) => vals.find((v) => v !== undefined && v !== null && v !== '');
+
+  const rawRetries = pick(opts.retries, opts.node_pre_gyp_retries, process.env.node_pre_gyp_retries);
+  const rawDelay = pick(opts.retry_delay, opts.node_pre_gyp_retry_delay, process.env.node_pre_gyp_retry_delay);
+  const rawTimeout = pick(opts.timeout, opts.node_pre_gyp_timeout, process.env.node_pre_gyp_timeout);
+
+  let retries = rawRetries === undefined ? DEFAULT_RETRIES : parseInt(rawRetries, 10);
+  let retryDelay = rawDelay === undefined ? DEFAULT_RETRY_DELAY : parseInt(rawDelay, 10);
+  let timeout = rawTimeout === undefined ? DEFAULT_TIMEOUT : parseInt(rawTimeout, 10);
+
+  if (!Number.isFinite(retries) || retries < 0) retries = DEFAULT_RETRIES;
+  if (!Number.isFinite(retryDelay) || retryDelay < 0) retryDelay = DEFAULT_RETRY_DELAY;
+  if (!Number.isFinite(timeout) || timeout < 0) timeout = DEFAULT_TIMEOUT;
+
+  return { retries, retryDelay, timeout };
+}
+
+/**
+ * Performs the binary download request with bounded retries and exponential backoff.
+ *
+ * Only the request and its status are retried, never the body stream: once the response is handed back and piped into
+ * tar, partial files exist on disk and a second attempt would extract over a half-written tree. See place_binary.
+ *
+ * @param {string} sanitized URL to download from.
+ * @param {Object} fetchOpts Options passed through to fetch, such as agent and timeout.
+ * @param {{retries: number, retryDelay: number}} retryOpts Retry settings from resolve_retry_opts.
+ * @returns {Promise<Object>} Resolves with the node-fetch Response.
+ * @throws {Error} The last error encountered once attempts are exhausted; carries statusCode for HTTP failures.
+ */
+async function fetch_with_retry(sanitized, fetchOpts, retryOpts) {
+  const { retries, retryDelay } = retryOpts;
+  let lastError;
+
+  for (let attempt = 0; attempt <= retries; attempt++) {
+    if (attempt > 0) {
+      const delay = backoff_delay(attempt - 1, retryDelay);
+      log.warn('install',
+        `retrying download (attempt ${attempt + 1}/${retries + 1}) in ${delay}ms: ${lastError}`);
+      await sleep(delay);
+    }
+
+    let res;
+    try {
+      res = await fetch(sanitized, fetchOpts);
+    } catch (e) {
+      if (attempt < retries && is_retryable_error(e)) {
+        lastError = e.message;
+        continue;
+      }
+      throw e;
+    }
+
+    if (res.ok || !is_retryable_status(res.status)) {
+      return res;
+    }
+
+    // node-fetch keeps the socket checked out until the body is consumed, so drain it before retrying.
+    if (res.body && typeof res.body.resume === 'function') res.body.resume();
+
+    lastError = `response status ${res.status} ${res.statusText}`;
+    if (attempt >= retries) {
+      const err = new Error(`response status ${res.status} ${res.statusText} on ${sanitized}`);
+      err.statusCode = res.status;
+      throw err;
+    }
+  }
+
+  // Unreachable: every path above either returns or throws. Guards against a future edit breaking that invariant.
+  throw new Error(`download failed after ${retries + 1} attempts: ${lastError}`);
+}
+
 function place_binary_authenticated(opts, targetDir, callback) {
   log.info('install', 'Attempting authenticated S3 download');
 
@@ -121,7 +273,11 @@ function place_binary(uri, targetDir, opts, callback) {
     log.log('download', `proxy agent configured using: "${proxyUrl}"`);
   }
 
-  fetch(sanitized, { agent })
+  const retryOpts = resolve_retry_opts(opts);
+
+  // The timeout covers the response headers only because the body is piped rather than buffered, so a slow but
+  // progressing tarball download is never killed mid-stream. Calling res.buffer()/text() here would change that.
+  fetch_with_retry(sanitized, { agent, timeout: retryOpts.timeout }, retryOpts)
     .then((res) => {
       if (!res.ok) {
         // If we get 403 Forbidden, the binary might be private - try authenticated download
@@ -132,8 +288,14 @@ function place_binary(uri, targetDir, opts, callback) {
           place_binary_authenticated(opts, targetDir, callback);
           return { authenticated: true };
         }
-        throw new Error(`response status ${res.status} ${res.statusText} on ${sanitized}`);
+        const err = new Error(`response status ${res.status} ${res.statusText} on ${sanitized}`);
+        // print_fallback_error branches on statusCode for its "Tried to download(N)" diagnostic.
+        err.statusCode = res.status;
+        throw err;
       }
+
+      // Committed from here: the body streams into tar, so a mid-stream failure leaves partial files in
+      // targetDir and is not safe to retry without extracting to a temp dir first.
       const dataStream = res.body;
 
       return new Promise((resolve, reject) => {
@@ -265,6 +427,14 @@ function install(gyp, argv, callback) {
 
     opts.ca = gyp.opts.ca;
     opts.cafile = gyp.opts.cafile;
+    // versioning.evaluate() builds a fresh opts object and drops anything it does not read, so these must be
+    // copied across explicitly or the flags silently become no-ops.
+    opts.retries = gyp.opts.retries;
+    opts.retry_delay = gyp.opts.retry_delay;
+    opts.timeout = gyp.opts.timeout;
+    opts.node_pre_gyp_retries = gyp.opts.node_pre_gyp_retries;
+    opts.node_pre_gyp_retry_delay = gyp.opts.node_pre_gyp_retry_delay;
+    opts.node_pre_gyp_timeout = gyp.opts.node_pre_gyp_timeout;
 
     const from = opts.hosted_tarball;
     const to = opts.module_path;
@@ -311,3 +481,18 @@ function install(gyp, argv, callback) {
 if (process.env.node_pre_gyp_mock_s3) {
   require('./mock/http')();
 }
+
+/**
+ * Pure helpers exposed for unit testing the retry decision table without standing up an HTTP round-trip for every
+ * case. Not part of the public API.
+ */
+module.exports.__test = {
+  resolve_retry_opts,
+  is_retryable_status,
+  is_retryable_error,
+  backoff_delay,
+  DEFAULT_RETRIES,
+  DEFAULT_RETRY_DELAY,
+  DEFAULT_TIMEOUT,
+  MAX_RETRY_DELAY
+};

lib/node-pre-gyp.js

@@ -99,7 +99,10 @@ proto.configDefs = {
   directory: String, // bin
   proxy: String,     // 'install'
   loglevel: String,  // everywhere
-  acl: String        // 'publish' - S3 ACL for published binaries
+  acl: String,         // 'publish' - S3 ACL for published binaries
+  retries: Number,     // 'install' - download retry attempts after the first
+  retry_delay: Number, // 'install' - base backoff delay in ms
+  timeout: Number      // 'install' - per-attempt request timeout in ms
 };
 
 /**