From f450f539957c7026394a0999b3c8e2170eece469 Mon Sep 17 00:00:00 2001 From: Bob Evans Date: Fri, 8 Sep 2023 12:36:54 -0400 Subject: [PATCH] fix: Fixed issue when only downloading prebuilts. Also fixed an issue with Node 20 and using a proxy server. --- lib/common.js | 4 +-- lib/pre-build.js | 39 +++++++++++----------------- tests/integration/download-server.js | 2 ++ tests/integration/pre-build.tap.js | 14 +++++++--- tests/unit/pre-build.tap.js | 19 +++++--------- 5 files changed, 35 insertions(+), 43 deletions(-) diff --git a/lib/common.js b/lib/common.js index 5a9e775..d9ebd28 100644 --- a/lib/common.js +++ b/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' diff --git a/lib/pre-build.js b/lib/pre-build.js index 0b9c62e..7ff495e 100644 --- a/lib/pre-build.js +++ b/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,9 +140,10 @@ 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)) + console.log('saving path', filePath) await fs.writeFile(filePath, data) } diff --git a/tests/integration/download-server.js b/tests/integration/download-server.js index cb95f51..c004961 100644 --- a/tests/integration/download-server.js +++ b/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) diff --git a/tests/integration/pre-build.tap.js b/tests/integration/pre-build.tap.js index adcc989..95748e5 100644 --- a/tests/integration/pre-build.tap.js +++ b/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) => { diff --git a/tests/unit/pre-build.tap.js b/tests/unit/pre-build.tap.js index bf76c21..cc14c95 100644 --- a/tests/unit/pre-build.tap.js +++ b/tests/unit/pre-build.tap.js @@ -10,7 +10,7 @@ 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() @@ -18,8 +18,6 @@ tap.test('pre-build tests', (t) => { 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') }) })