Skip to content

Commit

Permalink
fix: Fixed issue when only downloading prebuilts. Also fixed an issue
Browse files Browse the repository at this point in the history
with Node 20 and using a proxy server.
  • Loading branch information
bizob2828 committed Sep 8, 2023
1 parent a783ef8 commit f450f53
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 43 deletions.
4 changes: 2 additions & 2 deletions lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down
39 changes: 15 additions & 24 deletions lib/pre-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const {
parseArgs,
logStart,
logFinish,
PACKAGE_ROOT,
BUILD_PATH,
REMOTE_PATH,
IS_WIN
Expand All @@ -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 })
}
}

Expand All @@ -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 }
Expand All @@ -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) {
Expand Down Expand Up @@ -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)

Check warning on line 146 in lib/pre-build.js

View workflow job for this annotation

GitHub Actions / lint (lts/*)

Unexpected console statement
await fs.writeFile(filePath, data)
}

Expand Down
2 changes: 2 additions & 0 deletions tests/integration/download-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 10 additions & 4 deletions tests/integration/pre-build.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
}
}
}

/**
Expand All @@ -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) => {
Expand Down
19 changes: 6 additions & 13 deletions tests/unit/pre-build.tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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'
)
})
Expand All @@ -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')
})
})
Expand Down

0 comments on commit f450f53

Please sign in to comment.