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

Make the nestedKey only take effect in the serialized object and fix … #885

Merged
merged 7 commits into from
May 19, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 3 additions & 1 deletion lib/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const endSym = Symbol('pino.end')
const formatOptsSym = Symbol('pino.formatOpts')
const messageKeySym = Symbol('pino.messageKey')
const nestedKeySym = Symbol('pino.nestedKey')
const nestedKeyStrSym = Symbol('pino.nestedKeyStr')

const wildcardFirstSym = Symbol('pino.wildcardFirst')

Expand Down Expand Up @@ -60,5 +61,6 @@ module.exports = {
needsMetadataGsym,
useOnlyCustomLevelsSym,
formattersSym,
hooksSym
hooksSym,
nestedKeyStrSym
}
46 changes: 39 additions & 7 deletions lib/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const {
streamSym,
nestedKeySym,
formattersSym,
messageKeySym
messageKeySym,
nestedKeyStrSym
} = require('./symbols')

function noop () {}
Expand All @@ -44,7 +45,6 @@ function genLog (level, hook) {
o = mapHttpResponse(o)
}
}
if (this[nestedKeySym]) o = { [this[nestedKeySym]]: o }
var formatParams
if (msg === null && n.length === 0) {
formatParams = [null]
Expand Down Expand Up @@ -108,10 +108,8 @@ function asJson (obj, msg, num, time) {
if (formatters.log) {
obj = formatters.log(obj)
}
if (msg !== undefined) {
obj[messageKey] = msg
}
const wildcardStringifier = stringifiers[wildcardFirstSym]
let propStr = ''
for (var key in obj) {
value = obj[key]
if ((notHasOwnProperty || obj.hasOwnProperty(key)) && value !== undefined) {
Expand Down Expand Up @@ -139,11 +137,45 @@ function asJson (obj, msg, num, time) {
value = (stringifier || stringify)(value)
}
if (value === undefined) continue
data += ',"' + key + '":' + value
propStr += ',"' + key + '":' + value
}
}

let msgStr = ''
if (msg !== undefined) {
value = serializers[messageKey] ? serializers[messageKey](msg) : msg
const stringifier = stringifiers[messageKey] || wildcardStringifier

switch (typeof value) {
case 'function':
mihai1voicescu marked this conversation as resolved.
Show resolved Hide resolved
break
case 'number':
/* eslint no-fallthrough: "off" */
if (Number.isFinite(value) === false) {
value = null
}
// this case explicitly falls through to the next one
case 'boolean':
if (stringifier) value = stringifier(value)
msgStr = ',"' + messageKey + '":' + value
break
case 'string':
value = (stringifier || asString)(value)
msgStr = ',"' + messageKey + '":' + value
break
default:
value = (stringifier || stringify)(value)
msgStr = ',"' + messageKey + '":' + value
mihai1voicescu marked this conversation as resolved.
Show resolved Hide resolved
}
}

return data + end
if (this[nestedKeySym]) {
// place all the obj properties under the specified key
// the nested key is already formatted from the constructor
return data + this[nestedKeyStrSym] + propStr.slice(1) + '}' + msgStr + end
} else {
return data + propStr + msgStr + end
}
}

function asChindings (instance, bindings) {
Expand Down
5 changes: 4 additions & 1 deletion pino.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const {
mixinSym,
useOnlyCustomLevelsSym,
formattersSym,
hooksSym
hooksSym,
nestedKeyStrSym
} = symbols
const { epochTime, nullTime } = time
const { pid } = process
Expand Down Expand Up @@ -162,6 +163,8 @@ function pino (...args) {
[formatOptsSym]: formatOpts,
[messageKeySym]: messageKey,
[nestedKeySym]: nestedKey,
// protect against injection
[nestedKeyStrSym]: nestedKey ? `,${JSON.stringify(nestedKey)}:{` : '',
[serializersSym]: serializers,
[mixinSym]: mixin,
[chindingsSym]: chindings,
Expand Down
33 changes: 33 additions & 0 deletions test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -681,3 +681,36 @@ test('correctly log NaN', async (t) => {
const { num } = await once(stream, 'data')
t.is(num, null)
})

test('correctly skip function', async (t) => {
const stream = sink()
const instance = pino(stream)

const o = { num: NaN }
instance.info(o, () => {})

const { msg } = await once(stream, 'data')
t.is(msg, undefined)
})

test('correctly skip Infinity', async (t) => {
const stream = sink()
const instance = pino(stream)

const o = { num: NaN }
instance.info(o, Infinity)

const { msg } = await once(stream, 'data')
t.is(msg, null)
})

test('correctly skip Infinity', async (t) => {
const stream = sink()
const instance = pino(stream)

const o = { num: NaN }
instance.info(o, 42)
mihai1voicescu marked this conversation as resolved.
Show resolved Hide resolved

const { msg } = await once(stream, 'data')
t.is(msg, 42)
})
46 changes: 46 additions & 0 deletions test/error.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,49 @@ test('correctly ignores toString on errors', async ({ same }) => {
stack: err.stack
})
})

test('correctly adds error information when nestedKey is used', async ({ same }) => {
const err = new Error('myerror')
err.toString = () => undefined
const stream = sink()
const instance = pino({
test: 'this',
nestedKey: 'obj'
}, stream)
instance.fatal(err)
const result = await once(stream, 'data')
delete result.time
same(result, {
pid,
hostname,
level: 60,
obj: {
type: 'Error',
stack: err.stack
},
msg: err.message
})
})

test('correctly adds msg on error when nestedKey is used', async ({ same }) => {
const err = new Error('myerror')
err.toString = () => undefined
const stream = sink()
const instance = pino({
test: 'this',
nestedKey: 'obj'
}, stream)
instance.fatal(err, 'msg message')
const result = await once(stream, 'data')
delete result.time
same(result, {
pid,
hostname,
level: 60,
obj: {
type: 'Error',
stack: err.stack
},
msg: 'msg message'
})
})
6 changes: 3 additions & 3 deletions test/metadata.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test('metadata works', async ({ ok, same, is }) => {
is(30, this.lastLevel)
is('a msg', this.lastMsg)
ok(Number(this.lastTime) >= now)
same(this.lastObj, { hello: 'world', msg: 'a msg' })
same(this.lastObj, { hello: 'world' })
const result = JSON.parse(chunk)
ok(new Date(result.time) <= new Date(), 'time is greater than Date.now()')
delete result.time
Expand All @@ -40,7 +40,7 @@ test('child loggers works', async ({ ok, same, is }) => {
is(child, this.lastLogger)
is(30, this.lastLevel)
is('a msg', this.lastMsg)
same(this.lastObj, { from: 'child', msg: 'a msg' })
same(this.lastObj, { from: 'child' })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling this might be a semver-major change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, should I update the version in the package.json?

const result = JSON.parse(chunk)
ok(new Date(result.time) <= new Date(), 'time is greater than Date.now()')
delete result.time
Expand All @@ -66,7 +66,7 @@ test('without object', async ({ ok, same, is }) => {
is(instance, this.lastLogger)
is(30, this.lastLevel)
is('a msg', this.lastMsg)
same({ msg: 'a msg' }, this.lastObj)
same({ }, this.lastObj)
const result = JSON.parse(chunk)
ok(new Date(result.time) <= new Date(), 'time is greater than Date.now()')
delete result.time
Expand Down
12 changes: 12 additions & 0 deletions test/redact.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,3 +744,15 @@ test('child bindings are redacted using wildcard and plain path keys', async ({
is(req.headers.cookie, '[Redacted]')
is(req.method, '[Redacted]')
})

test('redacts boolean at the top level', async ({ is }) => {
const stream = sink()
const instance = pino({ redact: ['msg'] }, stream)
const obj = {
s: 's'
}
instance.info(obj, true)
const o = await once(stream, 'data')
is(o.s, 's')
is(o.msg, '[Redacted]')
})
34 changes: 34 additions & 0 deletions test/serializers.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,29 @@ test('custom serializer overrides default err namespace error serializer', async
is(typeof o.err.s, 'string')
})

test('custom serializer overrides default err namespace error serializer when nestedKey is on', async ({ is }) => {
const stream = sink()
const parent = pino({
nestedKey: 'obj',
serializers: {
err: (e) => {
return {
t: e.constructor.name,
m: e.message,
s: e.stack
}
}
}
}, stream)

parent.info({ err: ReferenceError('test') })
const o = await once(stream, 'data')
is(typeof o.obj.err, 'object')
is(o.obj.err.t, 'ReferenceError')
is(o.obj.err.m, 'test')
is(typeof o.obj.err.s, 'string')
})

test('null overrides default err namespace error serializer', async ({ is }) => {
const stream = sink()
const parent = pino({ serializers: { err: null } }, stream)
Expand Down Expand Up @@ -206,3 +229,14 @@ test('non-overridden serializers are available in the children', async ({ is })
parent.fatal({ onlyChild: 'test' })
is((await o4).onlyChild, 'test')
})

test('custom serializer for messageKey', async (t) => {
const stream = sink()
const instance = pino({ serializers: { msg: () => '422' } }, stream)

const o = { num: NaN }
instance.info(o, 42)

const { msg } = await once(stream, 'data')
t.is(msg, '422')
})