Delegate retry logic to the external library - @gar/promise-retry

Author: manzoorwanijk

DEPENDENCIES.md

@@ -532,6 +532,7 @@ graph LR;
   npmcli-arborist-->bin-links;
   npmcli-arborist-->cacache;
   npmcli-arborist-->common-ancestor-path;
+  npmcli-arborist-->gar-promise-retry["@gar/promise-retry"];
   npmcli-arborist-->hosted-git-info;
   npmcli-arborist-->isaacs-string-locale-compare["@isaacs/string-locale-compare"];
   npmcli-arborist-->json-stringify-nice;

package-lock.json

@@ -14440,6 +14440,7 @@
       "version": "9.4.0",
       "license": "ISC",
       "dependencies": {
+        "@gar/promise-retry": "^1.0.0",
         "@isaacs/string-locale-compare": "^1.1.0",
         "@npmcli/fs": "^5.0.0",
         "@npmcli/installed-package-contents": "^4.0.0",

workspaces/arborist/lib/arborist/rebuild.js

@@ -9,36 +9,13 @@ const runScript = require('@npmcli/run-script')
 const { callLimit: promiseCallLimit } = require('promise-call-limit')
 const { depth: dfwalk } = require('treeverse')
 const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp')
+const { promiseRetry } = require('@gar/promise-retry')
 const { log, time } = require('proc-log')
 const { resolve } = require('node:path')
 
 const boolEnv = b => b ? '1' : ''
 const sortNodes = (a, b) => (a.depth - b.depth) || localeCompare(a.path, b.path)
 
-// On Windows, antivirus/indexer can transiently lock files, causing
-// EPERM/EACCES/EBUSY on the rename inside write-file-atomic (used by
-// bin-links/fix-bin.js).  Retry with backoff for up to ~7.5 seconds.
-/* istanbul ignore next */
-const retryBinLinks = async (binLinks, node, opts, retries = 4) => {
-  const delay = (5 - retries) * 500
-  await new Promise(r => setTimeout(r, delay))
-  try {
-    return await binLinks({
-      pkg: node.package,
-      path: node.path,
-      top: !!(node.isTop || node.globalTop),
-      force: opts.force,
-      global: !!node.globalTop,
-    })
-  } catch (err) {
-    if (retries > 0 &&
-        (err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'EBUSY')) {
-      return retryBinLinks(binLinks, node, opts, retries - 1)
-    }
-    throw err
-  }
-}
-
 const _checkBins = Symbol.for('checkBins')
 
 // defined by reify mixin
@@ -405,19 +382,20 @@ module.exports = cls => class Builder extends cls {
 
     const timeEnd = time.start(`build:link:${node.location}`)
 
-    const p = binLinks({
+    // On Windows, antivirus/indexer can transiently lock files, causing EPERM/EACCES/EBUSY on the rename inside write-file-atomic (used by bin-links/fix-bin.js), so, retry with backoff.
+    const p = promiseRetry((retry) => binLinks({
       pkg: node.package,
       path: node.path,
       top: !!(node.isTop || node.globalTop),
       force: this.options.force,
       global: !!node.globalTop,
-    }).catch(/* istanbul ignore next - Windows retry for antivirus locks */ err => {
+    }).catch(/* istanbul ignore next - Windows-only transient antivirus locks */ err => {
       if (process.platform === 'win32' &&
           (err.code === 'EPERM' || err.code === 'EACCES' || err.code === 'EBUSY')) {
-        return retryBinLinks(binLinks, node, this.options)
+        return retry(err)
       }
       throw err
-    })
+    }), { retries: 5, minTimeout: 500 })
 
     await (this.#doHandleOptionalFailure
       ? this[_handleOptionalFailure](node, p)

workspaces/arborist/package.json

@@ -3,6 +3,7 @@
   "version": "9.4.0",
   "description": "Manage node_modules trees",
   "dependencies": {
+    "@gar/promise-retry": "^1.0.0",
     "@isaacs/string-locale-compare": "^1.1.0",
     "@npmcli/fs": "^5.0.0",
     "@npmcli/installed-package-contents": "^4.0.0",