Merge pull request #234 from bizob2828/fix-download

lib/common.js

@@ -7,8 +7,8 @@
 const path = require('path')
 const common = module.exports
 
-common.PACKAGE_ROOT = path.resolve(__dirname, '..')
-common.BUILD_PATH = path.resolve(common.PACKAGE_ROOT, './build/Release')
+const pkgRoot = path.resolve(__dirname, '..')
+common.BUILD_PATH = path.resolve(pkgRoot, './build/Release')
 common.REMOTE_PATH = process.env.NR_NATIVE_METRICS_REMOTE_PATH || 'nodejs_agent/builds/'
 common.IS_WIN = process.platform === 'win32'
 

lib/pre-build.js

@@ -28,7 +28,6 @@ const {
   parseArgs,
   logStart,
   logFinish,
-  PACKAGE_ROOT,
   BUILD_PATH,
   REMOTE_PATH,
   IS_WIN
@@ -46,21 +45,18 @@ preBuild.load = function load(target) {
   return require(path.join(BUILD_PATH, getBinFileName(target)))
 }
 
-preBuild.makePath = async function makePath(pathToMake) {
+preBuild.makePath = async function makePath() {
   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)
-
   try {
-    await fs.access(pathToMake, accessRights)
+    await fs.access(BUILD_PATH, accessRights)
   } catch (err) {
     if (err?.code !== 'ENOENT') {
       // It exists but we don't have read+write access! This is a problem.
-      throw new Error(`Do not have access to '${pathToMake}': ${err}`)
+      throw new Error(`Do not have access to '${BUILD_PATH}': ${err}`)
     }
 
-    await fs.mkdir(pathToMake, { recursive: true })
+    await fs.mkdir(BUILD_PATH, { recursive: true })
   }
 }
 
@@ -85,26 +81,20 @@ preBuild.moveBuild = async function moveBuild(target) {
   await fs.rename(filePath, destination)
 }
 
-function setupRequest(url, fileName) {
+function setupRequest() {
   let client = null
-  let options = {}
+  const options = {}
   const proxyHost = process.env.NR_NATIVE_METRICS_PROXY_HOST
 
   if (proxyHost) {
-    const parsedUrl = new URL(DOWNLOAD_HOST)
-    options = parsedUrl
-    options.path = REMOTE_PATH + fileName
     options.agent = new ProxyAgent(proxyHost)
     client = /^https:/.test(proxyHost) ? https : http
+  } else if (DOWNLOAD_HOST.startsWith('https:')) {
+    client = https
   } else {
-    options = url
-    if (DOWNLOAD_HOST.startsWith('https:')) {
-      client = https
-    } else {
-      // eslint-disable-next-line no-console
-      console.log(`Falling back to http, please consider enabling SSL on ${DOWNLOAD_HOST}`)
-      client = http
-    }
+    // eslint-disable-next-line no-console
+    console.log(`Falling back to http, please consider enabling SSL on ${DOWNLOAD_HOST}`)
+    client = http
   }
 
   return { client, options }
@@ -113,10 +103,10 @@ function setupRequest(url, fileName) {
 preBuild.download = async function download(target) {
   const fileName = getPackageFileName(target)
   const url = DOWNLOAD_HOST + REMOTE_PATH + fileName
-  const { client, options } = setupRequest(url, fileName)
+  const { client, options } = setupRequest()
 
   return new Promise((resolve, reject) => {
-    client.get(options, function handleResponse(res) {
+    client.get(url, options, function handleResponse(res) {
       if (res.statusCode === 404) {
         reject(new Error('No pre-built artifacts for your OS/architecture.'))
       } else if (res.statusCode !== 200) {
@@ -150,7 +140,7 @@ preBuild.download = async function download(target) {
 }
 
 preBuild.saveDownload = async function saveDownload(target, data) {
-  await preBuild.makePath(BUILD_PATH)
+  await preBuild.makePath()
 
   const filePath = path.join(BUILD_PATH, getBinFileName(target))
   await fs.writeFile(filePath, data)

tests/integration/download-server.js

@@ -25,6 +25,8 @@ execSync(`node ./lib/pre-build rebuild native_metrics`)
 execSync(`mv ./build/Release/*.node ${BINARY_TMP}`)
 const [file] = findBinary()
 const binaryPath = path.join(BINARY_TMP, file)
+// remove build folder as it is recreated during install/download
+execSync(`rm -rf ./build`)
 
 const server = http.createServer(function (req, res) {
   const raw = fs.createReadStream(binaryPath)

tests/integration/pre-build.tap.js

@@ -19,9 +19,15 @@ const { IS_WIN } = require('../../lib/common')
  * Locates the pre-built native metrics binary in `./build/Release` folder.
  */
 function findBinary() {
-  return fs.readdirSync('./build/Release').filter((file) => {
-    return file.endsWith('.node')
-  })
+  try {
+    return fs.readdirSync('./build/Release').filter((file) => {
+      return file.endsWith('.node')
+    })
+  } catch (err) {
+    if (err?.code === 'ENOENT') {
+      return []
+    }
+  }
 }
 
 /**
@@ -45,7 +51,7 @@ function waitFor(child, msg) {
 
 tap.test('pre-build commands', function (t) {
   t.beforeEach(() => {
-    execSync('rm -rf ./build/Release/*')
+    execSync('rm -rf ./build')
   })
 
   CMDS.forEach((cmd) => {

tests/unit/pre-build.tap.js

@@ -10,16 +10,14 @@ const sinon = require('sinon')
 const proxyquire = require('proxyquire')
 const nock = require('nock')
 const zlib = require('zlib')
-const { IS_WIN } = require('../../lib/common')
+const { IS_WIN, BUILD_PATH } = require('../../lib/common')
 
 tap.test('pre-build tests', (t) => {
   t.autoend()
 
   t.test('makePath', (t) => {
     t.autoend()
 
-    const fakePath = 'tests/unit/fake-path'
-
     let mockFsPromiseApi
     let preBuild
 
@@ -39,17 +37,16 @@ tap.test('pre-build tests', (t) => {
 
     t.test('should make a nested folder path accordingly if it does not exist', async (t) => {
       mockFsPromiseApi.access.rejects({ code: 'ENOENT' })
-      await preBuild.makePath(fakePath)
+      await preBuild.makePath()
       t.equal(mockFsPromiseApi.mkdir.callCount, 1, 'should have called mkdir')
     })
 
     t.test('should throw if permissions to path are incorrect', { skip: IS_WIN }, async (t) => {
-      const fullPath = `${process.cwd()}/${fakePath}`
       mockFsPromiseApi.access.rejects({ code: 'EACCESS' })
       t.equal(mockFsPromiseApi.mkdir.callCount, 0, 'should not have called mkdir')
       t.rejects(
-        preBuild.makePath(fakePath),
-        new Error(`Do not have access to '${fullPath}'`),
+        preBuild.makePath(),
+        new Error(`Do not have access to '${BUILD_PATH}'`),
         'should error with EACCESS'
       )
     })
@@ -59,15 +56,11 @@ tap.test('pre-build tests', (t) => {
       const expectedError = new Error('whoops')
       mockFsPromiseApi.mkdir.rejects(expectedError)
 
-      t.rejects(
-        preBuild.makePath(fakePath),
-        expectedError,
-        'should have rejected with expectedError'
-      )
+      t.rejects(preBuild.makePath(), expectedError, 'should have rejected with expectedError')
     })
 
     t.test('should not create the nested folder path if it exists and is accessible', async (t) => {
-      await preBuild.makePath(fakePath)
+      await preBuild.makePath()
       t.equal(mockFsPromiseApi.mkdir.callCount, 0, 'should not have called mkdir')
     })
   })