Skip to content

Commit

Permalink
fix: test for multiple caller entries (#1304)
Browse files Browse the repository at this point in the history
* fix: test for multiple caller entries

* fix: Add test for thrown error

* fix: stick to original api
  • Loading branch information
moshie authored Jan 20, 2022
1 parent cdd7154 commit 6030b26
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 25 deletions.
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

0 comments on commit 6030b26

Please sign in to comment.