pino: pino crashes the server when logging an error from request-promise
Hi,
When logging an error from request-promise
, pino
does not handle it correctly and causes the server to crash.
Steps to reproduce it:
Create a server which returns an error
var restify = require('restify');
var server = restify.createServer();
server.get('/boom', (req, res, next) => {
console.log('GET /boom');
res.send(500, 'yay! 500');
return next(false);
});
server.listen(1337, function() {
console.log('Listening at localhost:1337');
});
Create a server which calls the previously created server
var restify = require('restify');
var rp = require('request-promise');
var pino = require('pino');
var log = pino();
var server = restify.createServer();
server.get('/oops', (req, res, next) => {
rp({
url: 'http://localhost:1337/boom',
simple: true
})
.then(() => {
log.info('SUCCESS');
res.send(200);
})
.catch((err) => {
log.error(err);
res.send(500);
});
});
server.listen(1338, function() {
console.log('Listening at localhost:1338');
});
Send a request
curl -i localhost:1338/oops
The curl request just hand and hitting Ctrl-C
cause the server to crash.
Environment
I am using node v6.9.2
. You can find my package.json below:
{
"name": "bug",
"version": "0.0.1",
"dependencies": {
"pino": "^4.0.2",
"request": "^2.80.0",
"request-promise": "^4.1.1",
"restify": "^4.3.0"
}
}
Root Cause
The root cause seems to be at this line: https://github.com/pinojs/pino/blob/master/pino.js#L189 where fast-safe-stringify
is called. It looks like in the error object there is a response
object with some non-serializable properties (e.g. socket, connection, etc…). A possibility might be that fast-safe-stringify
is not handling this properly. There is a related closed issue here: https://github.com/davidmarkclements/fast-safe-stringify/issues/4 .
Even if this error seems related to fast-safe-stringify
, I have open the issue here for two reasons:
- visibility to pino users.
- I think pino should not try to serialise all the fields of an error object, as the risk to hit these kind of problems are quite high.
Apologies if I missed something, I would be happy to help you out fixing this.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Comments: 28 (19 by maintainers)
Commits related to this issue
- bump fast-safe-stringify, closes #203 — committed to pinojs/pino by deleted user 7 years ago
- bump fast-safe-stringify, closes #203 — committed to pinojs/pino by deleted user 7 years ago
- bump fast-safe-stringify, closes #203 — committed to pinojs/pino by deleted user 7 years ago
- bump fast-safe-stringify, closes #203 — committed to pinojs/pino by deleted user 7 years ago
- Merge pull request #204 from pinojs/serialization-edge-fix bump fast-safe-stringify, closes #203 — committed to pinojs/pino by deleted user 7 years ago
Sure thing! thanks for the quick response @davidmarkclements.