Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: test for multiple caller entries #1304

Merged
merged 3 commits into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions lib/caller.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
}
39 changes: 26 additions & 13 deletions lib/transport.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 : {}

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

Expand Down
12 changes: 12 additions & 0 deletions test/transport/core.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down