From 6030b2670d2f0b0f106d9d8c59a5d2b1675749f3 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 20 Jan 2022 17:00:05 +0000 Subject: [PATCH] fix: test for multiple caller entries (#1304) * fix: test for multiple caller entries * fix: Add test for thrown error * fix: stick to original api --- lib/caller.js | 20 ++++++++----------- lib/transport.js | 39 ++++++++++++++++++++++++------------- test/transport/core.test.js | 12 ++++++++++++ 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/lib/caller.js b/lib/caller.js index b62900089..f39e08781 100644 --- a/lib/caller.js +++ b/lib/caller.js @@ -1,16 +1,10 @@ 'use strict' -const fs = require('fs') - function noOpPrepareStackTrace (_, stack) { return stack } -function isOutsideNodeModules (input) { - return typeof input === 'string' && fs.existsSync(input) && input.indexOf('node_modules') === -1 -} - -module.exports = function getCaller () { +module.exports = function getCallers () { const originalPrepare = Error.prepareStackTrace Error.prepareStackTrace = noOpPrepareStackTrace const stack = new Error().stack @@ -22,13 +16,15 @@ module.exports = function getCaller () { const entries = stack.slice(2) - for (const entry of entries) { - const file = entry ? entry.getFileName() : undefined + const fileNames = [] - if (isOutsideNodeModules(file)) { - return file + for (const entry of entries) { + if (!entry) { + continue } + + fileNames.push(entry.getFileName()) } - return entries[0] ? entries[0].getFileName() : undefined + return fileNames } diff --git a/lib/transport.js b/lib/transport.js index 4f1ab5165..8ec4b7ac1 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -1,7 +1,7 @@ 'use strict' const { createRequire } = require('module') -const getCaller = require('./caller') +const getCallers = require('./caller') const { join, isAbsolute } = require('path') let onExit @@ -79,9 +79,11 @@ function autoEnd (stream) { } function transport (fullOptions) { - const { pipeline, targets, options = {}, worker = {}, caller = getCaller() } = fullOptions - // This function call MUST stay in the top-level function of this module - const callerRequire = createRequire(caller) + const { pipeline, targets, options = {}, worker = {}, caller = getCallers() } = fullOptions + + // Backwards compatibility + const callers = typeof caller === 'string' ? [caller] : caller + // This will be eventually modified by bundlers const bundlerOverrides = '__bundlerPathsOverrides' in globalThis ? globalThis.__bundlerPathsOverrides : {} @@ -118,16 +120,27 @@ function transport (fullOptions) { return origin } - switch (origin) { - // This hack should not be needed, however it would not work otherwise - // when testing it from this module and in examples. - case 'pino/file': - return join(__dirname, '..', 'file.js') - /* istanbul ignore next */ - default: - // TODO we cannot test this on Windows.. might not work. - return callerRequire.resolve(origin) + if (origin === 'pino/file') { + return join(__dirname, '..', 'file.js') } + + let fixTarget + + for (const filePath of callers) { + try { + fixTarget = createRequire(filePath).resolve(origin) + break + } catch (err) { + // Silent catch + continue + } + } + + if (!fixTarget) { + throw new Error(`unable to determine transport target for "${origin}"`) + } + + return fixTarget } } diff --git a/test/transport/core.test.js b/test/transport/core.test.js index a1ee9966f..fdda91b37 100644 --- a/test/transport/core.test.js +++ b/test/transport/core.test.js @@ -324,6 +324,18 @@ test('pino.transport with target pino/file and append option', async ({ same, te }) }) +test('pino.transport should error with unknown target', async ({ fail, equal }) => { + try { + pino.transport({ + target: 'origin', + caller: 'unknown-file.js' + }) + fail('must throw') + } catch (err) { + equal(err.message, 'unable to determine transport target for "origin"') + } +}) + test('pino.transport with target pino-pretty', async ({ match, teardown }) => { const destination = join( os.tmpdir(),