Invalid error JSON logged if stack is undefined

[disintegrator]

To reproduce:

const logger = require('pino')();
const err = new Error();
delete err.stack;
logger.error(err);

/*
{"level":50,"time":1532087320790,"msg":"","pid":20631,"hostname":"dev.local","type":"Error","stack":undefined,"v":1}
*/

Note the section "stack":undefined which is not valid JSON.

Real world example: Apollo GraphQL has a custom error class called ApolloError that does not hold stack property. When logging an ApolloError we see the issue above which breaks pino's pretty printer among other things.

Pino version: 4.17.3

[mcollina]

That’s definitely a bug! Out of curiosity.. why are you removing .stack?

[disintegrator]

Hey @mcollina. Thanks for the quick reply. The code attached is strictly a way to reproduce the issue. The real world case affecting me is trying to log instances of ApolloError which are an instance of Error but do not have a stack property set for some reason.

[disintegrator]

For more context this what an ApolloError looks like: https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-errors/src/index.ts#L3

[disintegrator]

I'm guessing this line is the culprit: https://github.com/pinojs/pino/blob/v4.17.3/pino.js#L149

data += ',"type":"Error","stack":' + this.stringify(obj.stack)

might have to change to:

data += ',"type":"Error"'
data += obj.stack ? ',"stack":' + this.stringify(obj.stack) : ''

If that's the case then I'm happy to submit a PR

[mcollina]

Please do so! Would you mind adding a unit test as well? Il giorno ven 20 lug 2018 alle 14:46 George Haidar <notifications@github.com> ha scritto:

…

I'm guessing this line is the culprit: https://github.com/pinojs/pino/blob/v4.17.3/pino.js#L149 data += ',"type":"Error","stack":' + this.stringify(obj.stack) might have to change to: data += ',"type":"Error"' data += obj.stack ? ',"stack":' + this.stringify(obj.stack) : '' If that's the case then I'm happy to submit a PR — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#456 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADL4-Dw6Y0XbetJYniY3_UwUK4buxLNks5uIdEPgaJpZM4VX6Sx> .

[davidmarkclements]

pino-integration has picked up that this broke a test in pino-pretty

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.