refactor: update prebuild to download first, fallback to build

lib/gyp-utils.js

@@ -81,21 +81,17 @@ utils.gypVersion = function gypVersion() {
   return match && match[1]
 }
 
-utils.execGyp = function execGyp(args, opts, cb) {
+utils.execGyp = function execGyp(args, opts) {
   const cmd = utils.extractGypCmd(args)
   const spawnOpts = {}
   if (!opts.quiet) {
     spawnOpts.stdio = [0, 1, 2]
   }
   console.log('> ' + cmd + ' ' + args.join(' ')) // eslint-disable-line no-console
 
-  const child = cp.spawn(cmd, args, spawnOpts)
-  child.on('error', cb)
-  child.on('close', function onGypClose(code) {
-    if (code !== 0) {
-      cb(new Error('Command exited with non-zero code: ' + code))
-    } else {
-      cb(null)
-    }
-  })
+  const child = cp.spawnSync(cmd, args, spawnOpts)
+
+  if (child.status !== 0) {
+    throw new Error('Command exited with non-zero code: ' + child.status)
+  }
 }

lib/pre-build.js

@@ -13,7 +13,7 @@
 // XXX This file must not have any deps. This file will run during the install
 // XXX step of the module and we are _not_ guaranteed that the dependencies have
 // XXX already installed. Core modules are okay.
-const fs = require('fs')
+const fs = require('fs/promises')
 const http = require('http')
 const https = require('https')
 const os = require('os')
@@ -46,31 +46,25 @@
   return require(path.join(BUILD_PATH, getBinFileName(target)))
 }
 
-preBuild.makePath = function makePath(pathToMake, cb) {
+preBuild.makePath = async function makePath(pathToMake) {
   const accessRights = fs.constants.R_OK | fs.constants.W_OK
 
   // We only want to make the parts after the package directory.
   pathToMake = path.join(PACKAGE_ROOT, pathToMake)
-  fs.access(pathToMake, accessRights, function fsAccessCB(err) {
-    if (!err) {
-      return cb()
-    } else if (err?.code !== 'ENOENT') {
+
+  try {
+    await fs.access(pathToMake, accessRights)
+  } catch (err) {
+    if (err?.code !== 'ENOENT') {
       // It exists but we don't have read+write access! This is a problem.
-      return cb(new Error(`Do not have access to '${pathToMake}': ${err}`))
+      throw new Error(`Do not have access to '${pathToMake}': ${err}`)
     }
 
-    // It probably does not exist, so try to make it.
-    fs.mkdir(pathToMake, { recursive: true }, function fsMkDirDb(mkdirErr) {
-      if (mkdirErr) {
-        return cb(mkdirErr)
-      }
-
-      cb()
-    })
-  })
+    await fs.mkdir(pathToMake, { recursive: true })
+  }
 }
 
-preBuild.build = function build(target, rebuild, cb) {
+preBuild.build = function build(target, rebuild) {
   const HAS_OLD_NODE_GYP_ARGS_FOR_WINDOWS = semver.lt(gypVersion() || '0.0.0', '3.7.0')
 
   if (IS_WIN && HAS_OLD_NODE_GYP_ARGS_FOR_WINDOWS) {
@@ -79,70 +73,16 @@
 
   const cmds = rebuild ? ['clean', 'configure'] : ['configure']
 
-  execGyp(cmds, opts, function cleanCb(err) {
-    if (err) {
-      return cb(err)
-    }
+  execGyp(cmds, opts)
 
-    const jobs = Math.round(CPU_COUNT / 2)
-    execGyp(['build', '-j', jobs, target], opts, cb)
-  })
+  const jobs = Math.round(CPU_COUNT / 2)
+  execGyp(['build', '-j', jobs, target], opts)
 }
 
-preBuild.moveBuild = function moveBuild(target, cb) {
+preBuild.moveBuild = async function moveBuild(target) {
   const filePath = path.join(BUILD_PATH, target + '.node')
   const destination = path.join(BUILD_PATH, getBinFileName(target))
-  fs.rename(filePath, destination, cb)
-}
-
-/**
- * Pipes the response and gunzip and unzips the data
- *
- * @param {Object} params
- * @param {http.ServerResponse} params.res response from download site
- * @param {string} url download url
- * @param {Function} cb callback when download is done
- */
-function unzipFile(url, cb, res) {
-  if (res.statusCode === 404) {
-    return cb(new Error('No pre-built artifacts for your OS/architecture.'))
-  } else if (res.statusCode !== 200) {
-    return cb(new Error('Failed to download ' + url + ': code ' + res.statusCode))
-  }
-
-  let hasCalledBack = false
-  const unzip = zlib.createGunzip()
-  const buffers = []
-  let size = 0
-
-  res.pipe(unzip).on('data', function onResData(data) {
-    buffers.push(data)
-    size += data.length
-  })
-
-  res.on('error', function onResError(err) {
-    if (!hasCalledBack) {
-      hasCalledBack = true
-      cb(new Error('Failed to download ' + url + ': ' + err.message))
-    }
-  })
-
-  unzip.on('error', function onResError(err) {
-    if (!hasCalledBack) {
-      hasCalledBack = true
-      cb(new Error('Failed to unzip ' + url + ': ' + err.message))
-    }
-  })
-
-  unzip.on('end', function onResEnd() {
-    if (hasCalledBack) {
-      return
-    }
-    hasCalledBack = true
-    cb(null, Buffer.concat(buffers, size))
-  })
-
-  res.resume()
+  await fs.rename(filePath, destination)
 }
 
 function setupRequest(url, fileName) {
@@ -170,91 +110,96 @@
   return { client, options }
 }
 
-preBuild.download = function download(target, cb) {
+preBuild.download = async function download(target) {
   const fileName = getPackageFileName(target)
   const url = DOWNLOAD_HOST + REMOTE_PATH + fileName
   const { client, options } = setupRequest(url, fileName)
 
-  client.get(options, unzipFile.bind(null, url, cb))
-}
-
-preBuild.saveDownload = function saveDownload(target, data, cb) {
-  preBuild.makePath(BUILD_PATH, function makePathCB(err) {
-    if (err) {
-      return cb(err)
-    }
+  return new Promise((resolve, reject) => {
+    client.get(options, function handleResponse(res) {
+      if (res.statusCode === 404) {
+        reject(new Error('No pre-built artifacts for your OS/architecture.'))
+      } else if (res.statusCode !== 200) {
+        reject(new Error('Failed to download ' + url + ': code ' + res.statusCode))
+      }
 
-    const filePath = path.join(BUILD_PATH, getBinFileName(target))
-    fs.writeFile(filePath, data, cb)
-  })
-}
+      const unzip = zlib.createGunzip()
+      const buffers = []
+      let size = 0
 
-preBuild.install = function install(target, cb) {
-  const errors = []
+      res.on('error', function httpError(err) {
+        reject(new Error('Failed to download ' + url + ': ' + err.message))
+      })
 
-  const noBuild = opts['no-build'] || process.env.NR_NATIVE_METRICS_NO_BUILD
-  const noDownload = opts['no-download'] || process.env.NR_NATIVE_METRICS_NO_DOWNLOAD
+      unzip.on('error', function unzipError(err) {
+        reject(new Error('Failed to unzip ' + url + ': ' + err.message))
+      })
 
-  // If NR_NATIVE_METRICS_NO_BUILD env var is specified, jump straight to downloading
-  if (noBuild) {
-    return doDownload()
-  }
+      res.pipe(unzip).on('data', function onResData(data) {
+        buffers.push(data)
+        size += data.length
+      })
 
-  // Otherwise, first attempt to build the package using the source. If that fails, try
-  // downloading the package. If that also fails, whoops!
-  preBuild.build(target, true, function buildCB(buildErr) {
-    if (!buildErr) {
-      return preBuild.moveBuild(target, function moveBuildCB(moveErr) {
-        if (moveErr) {
-          errors.push(moveErr)
-          doDownload()
-        } else {
-          doCallback()
-        }
+      unzip.on('end', function onResEnd() {
+        resolve(Buffer.concat(buffers, size))
       })
-    }
-    errors.push(buildErr)
 
-    // Building failed, try downloading.
-    doDownload()
+      res.resume()
+    })
   })
+}
 
-  function doDownload() {
-    if (noDownload && !noBuild) {
-      return doCallback(new Error('Downloading is disabled.'))
-    }
+preBuild.saveDownload = async function saveDownload(target, data) {
+  await preBuild.makePath(BUILD_PATH)
 
-    preBuild.download(target, function downloadCB(err, data) {
-      if (err) {
-        return doCallback(err)
-      }
+  const filePath = path.join(BUILD_PATH, getBinFileName(target))
+  await fs.writeFile(filePath, data)
+}
 
-      preBuild.saveDownload(target, data, doCallback)
-    })
+preBuild.install = async function install(target) {
+  const noBuild = opts['no-build'] || process.env.NR_NATIVE_METRICS_NO_BUILD
+  const noDownload = opts['no-download'] || process.env.NR_NATIVE_METRICS_NO_DOWNLOAD
+
+  if (noDownload && !noBuild) {
+    // If NR_NATIVE_METRICS_NO_DOWNLOAD env var is specified, jump straight to building
+    preBuild.build(target, true)
+    return await preBuild.moveBuild(target)
   }
 
-  function doCallback(err) {
-    if (err) {
-      errors.push(err)
-      cb(err)
-    } else {
-      cb()
+  // Try the download path first, if that fails try building if config allows
+  try {
+    const data = await preBuild.download(target)
+    await preBuild.saveDownload(target, data)
+  } catch (err) {
+    // eslint-disable-next-line no-console
+    console.log(`Download error: ${err.message}, falling back to build`)
+
+    if (noBuild) {
+      throw new Error('Building is disabled by configuration')
     }
+
+    preBuild.build(target, true)
+    await preBuild.moveBuild(target)
   }
 }
 
-preBuild.executeCli = function executeCli(cmd, target) {
+preBuild.executeCli = async function executeCli(cmd, target) {
   logStart(cmd)
   if (cmd === 'build' || cmd === 'rebuild') {
-    preBuild.build(target, cmd === 'rebuild', function buildCb(err) {
-      if (err) {
-        logFinish(cmd, target, err)
-      } else {
-        preBuild.moveBuild(target, logFinish.bind(this, cmd, target))
-      }
-    })
+    try {
+      preBuild.build(target, cmd === 'rebuild')
+      await preBuild.moveBuild(target)
+      logFinish(cmd, target)
+    } catch (err) {
+      logFinish(cmd, target, err)
+    }
   } else if (cmd === 'install') {
-    preBuild.install(target, logFinish.bind(this, cmd, target))
+    try {
+      await preBuild.install(target)
+      logFinish(cmd, target)
+    } catch (err) {
+      logFinish(cmd, target, err)
+    }
   }
 }
 

tests/integration/download-server.js

@@ -19,7 +19,7 @@ function findBinary() {
 
 // building module to serve in the server instead of grabbing from
 // download.newrelic.com
-execSync(`node ./lib/pre-build install native_metrics`)
+execSync(`node ./lib/pre-build rebuild native_metrics`)
 // moving module to avoid a passing test on download
 // even though the file existing in the build/Release folder
 execSync(`mv ./build/Release/*.node ${BINARY_TMP}`)