pinojs / pino-noir

🌲 pino log redaction 🍷
MIT License
66 stars 17 forks source link

pino noir crashing hapi server #10

Closed kishoresanke closed 6 years ago

kishoresanke commented 6 years ago

pino noir crashing hapi server. It could be related to - https://github.com/pinojs/pino-noir/issues/1


const pino = require('hapi-pino');
const pinoNoir = require('pino-noir');
...
{
      register: pino,
      options: {
        serializers: pinoNoir([
          'created',
          'req.headers.authorization'
        ], '[Redacted]')
      }
    },```

Versions i'm using :-

├─┬ hapi-pino@2.1.1
│ └─┬ pino@4.10.3
├── pino-noir@2.0.0
mcollina commented 6 years ago

what does the stacktrace look like?

kishoresanke commented 6 years ago

like mentioned in the referenced ticket, it redacts the "created" field but when a request is made, it just hangs. there is no stack trace .. it works if we replace 'req.headers.authorization' with 'req' and the whole req object is redacted. But looks like pino-noir is getting stuck when we specify path with more than one level.

davidmarkclements commented 6 years ago

Just a note to move forward in analysis, this is specific to hapi-pino, the issue isn't occurring with pino-http

var pinoNoir = require('pino-noir')
var pinoHttp = require('pino-http')
var http = require('http')

var logger = pinoHttp({
  serializers: pinoNoir([
      'created',
      'req.headers.host'
    ], '[Redacted]')
})

http.createServer((req, res) => {
  logger(req, res)

  req.log.info({created: 'abc'})

  res.end('done\n')
}).listen(3000)
$ curl http://localhost:3000
done

log output:

{"level":30,"time":1516903252346,"msg":"request completed","pid":86651,"hostname":"Davids-MacBook-Pro-3.local","req":{"id":2,"method":"GET","url":"/","headers":{"host":"[Redacted]","user-agent":"curl/7.49.1","accept":"*/*"},"remoteAddress":"::ffff:127.0.0.1","remotePort":63312},"res":{"statusCode":200,"header":"HTTP/1.1 200 OK\r\nDate: Thu, 25 Jan 2018 18:00:52 GMT\r\nConnection: keep-alive\r\nContent-Length: 4\r\n\r\n"},"responseTime":1,"v":1}
kishoresanke commented 6 years ago

yes, but its not working on hapi-js for some reason. any pointers to resolving this issue ?

davidmarkclements commented 6 years ago

hey @kishoresanke if you like – if you produce a hapi + hapi pino of my example and post here that'll speed things up

davidmarkclements commented 6 years ago

I believe this is fixed, closing but feel free to open if I'm wrong