Redacting "responseTime" leads to invalid JSON

[tuckbick]

In the configuration options below, responseTime is a redacted path:

          "redact": {
            "paths": [
                "pid",
                "hostname",
                "app",
                "responseTime",
                "req.id",
                "req.method",
                "req.headers",
                "req.remoteAddress",
                "req.remotePort",
                "res"
            ],
            "remove": true
          }

This results in a log that contains invalid JSON (see undefined):

{"level":30,"time":1576099135935,"req":{"url":"http://localhost:8080/todos?completed=true"},"req":{"url":"http://localhost:8080/todos?completed=true"},"responseTime":undefined,"msg":"request completed","v":1}

I recognize that this is probably a pino issue, but it's only occurring for me when redacting responseTime, a value calculated in hapi-pino.

I believe the issue has to do with the stringifier in pino here.

[mcollina]

There might be some bugs! Would you like to upload an example that reproduce the problem? Also, sending a PR would be handy.

I think this might be a bug in https://www.npmjs.com/package/fast-redact.

cc @davidmarkclements

[tuckbick]

I wouldn't expect any software to be completely free of bugs 😁
Here's a repo here that reproduces the problem: https://github.com/tuckbick/test-hapi-pino
This could instead be a problem in pino-pretty, as this only occurs while piping stdout through pino-pretty instead of using the prettyPrint option.

[mcollina]

I’m failing to understand how using prettyPrint internally can avoid this. Anyway, it seems you have identified where there is a bug.. would you like to send a PR to pino to fix?

[tuckbick]

Yep, sure thing. I'll try to get PR together this week!

[tuckbick]

Haven't gotten to a PR yet -- not quite sure where to put a fix yet. However, I've done some investigation:

• When pinoPretty: true, pino does some programmatic redaction here, stringifying the object, thus removing undefined properties.
• Otherwise, when pinoPretty: false, the object is logged to stdout containing the undefined properties.

[mcollina]

You need to add another if in https://github.com/pinojs/pino/blob/master/lib/tools.js#L107 to handle the case that value could be undefined and in that case not include the key as well.

[tuckbick]

Thanks for the direction! Please see the submitted PR.

[tuckbick]

Merged in pinojs/pino#756