From e9e8dfa5ea14953bf7c62de3bc34143388a073ff Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Wed, 19 Jan 2022 19:32:53 +0000 Subject: [PATCH 1/3] fix: test for multiple caller entries --- lib/caller.js | 20 ++++++++------------ lib/transport.js | 36 +++++++++++++++++++++++------------- 2 files changed, 31 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..e5c958cce 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,8 @@ 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 = {}, callers = getCallers() } = fullOptions + // This will be eventually modified by bundlers const bundlerOverrides = '__bundlerPathsOverrides' in globalThis ? globalThis.__bundlerPathsOverrides : {} @@ -118,16 +117,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 caller of callers) { + try { + fixTarget = createRequire(caller).resolve(origin) + break + } catch (err) { + // Silent catch + continue + } + } + + if (!fixTarget) { + throw new Error(`unable to determine transport target for "${origin}"`) } + + return fixTarget } } From 92df07484896977b85aae8e9c72742b363a2a723 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Wed, 19 Jan 2022 20:09:01 +0000 Subject: [PATCH 2/3] fix: Add test for thrown error --- test/transport/core.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/transport/core.test.js b/test/transport/core.test.js index a1ee9966f..f59248041 100644 --- a/test/transport/core.test.js +++ b/test/transport/core.test.js @@ -324,6 +324,20 @@ 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', + callers: [ + '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(), From ca480ed6345e555b98cedf8955033d9d5e82c88d Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Thu, 20 Jan 2022 10:08:22 +0000 Subject: [PATCH 3/3] fix: stick to original api --- lib/transport.js | 9 ++++++--- test/transport/core.test.js | 4 +--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/transport.js b/lib/transport.js index e5c958cce..8ec4b7ac1 100644 --- a/lib/transport.js +++ b/lib/transport.js @@ -79,7 +79,10 @@ function autoEnd (stream) { } function transport (fullOptions) { - const { pipeline, targets, options = {}, worker = {}, callers = getCallers() } = fullOptions + 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 : {} @@ -123,9 +126,9 @@ function transport (fullOptions) { let fixTarget - for (const caller of callers) { + for (const filePath of callers) { try { - fixTarget = createRequire(caller).resolve(origin) + fixTarget = createRequire(filePath).resolve(origin) break } catch (err) { // Silent catch diff --git a/test/transport/core.test.js b/test/transport/core.test.js index f59248041..fdda91b37 100644 --- a/test/transport/core.test.js +++ b/test/transport/core.test.js @@ -328,9 +328,7 @@ test('pino.transport should error with unknown target', async ({ fail, equal }) try { pino.transport({ target: 'origin', - callers: [ - 'unknown-file.js' - ] + caller: 'unknown-file.js' }) fail('must throw') } catch (err) {