Rcomian / bunyan-rotating-file-stream

Is an endpoint for bunyan that lets you control how much disk space logs take.
Other
29 stars 15 forks source link

App crash #10

Closed abhishekb21 closed 7 years ago

abhishekb21 commented 7 years ago

This is the error log I am getting. Not sure what is causing the issue. But my app started crashing suddenly. When I removed the rotating file stream it started working again.

~/node_modules/express/lib/request.js:307 var proto = this.connection.encrypted var proto = this.connection.encrypted ^

TypeError: Cannot read property 'encrypted' of undefined at IncomingMessage.protocol (~/node_modules/express/lib/request.js:307:30) at Object.stringify (native) at fastJsonify (~/node_modules/bunyan-rotating-file-stream/lib/rotatingfilestream.js:62:21) at writer (~/node_modules/bunyan-rotating-file-stream/lib/rotatingfilestream.js:119:27) at ~/node_modules/bunyan-rotating-file-stream/lib/limitedqueue.js:49:9 at Immediate.q.process [as _onImmediate] (~/node_modules/async/lib/async.js:953:21) at processImmediate [as _immediateCallback] (timers.js:383:17)

abhishekb21 commented 7 years ago

My error. I was logging in a format not supported by Bunyan.

However the app should not have crashed due to that.

Rcomian commented 7 years ago

Sorry for the late response, as you had worked around the issue I didn't rush to look in detail.

The problem appears to be that the object that's being passed to the log is crashing when it's being Jsonified.

The question we've got is: "If we JSONify an object, but that object throws an exception during that process, what should we do?"

The options I can think of are:

  1. Let the exception leak to the calling application
  2. Log nothing and swallow the exception
  3. Log the exception instead of the original log

Of these answers, if we chose 2, we'd never know there was a problem when logging, except we'd be scratching our heads over why certain critical logs weren't appearing even though we were definitely logging them and we really need them to figure out what's going wrong.

If we chose 3, we need to work out what to log. We can't log the original data at all, so we have very little context, and the log format would be completely different to what you were expecting. It's also not obvious that something is going wrong until you actually look at the logs, so this might go undetected for a long time.

1 seems the safest option here, but I'm willing to take feedback if you think there's a better way to handle it.

I'll close this for now, feel free to re-open, raise another issue or pull-request.

Cheers