bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
798 stars 186 forks source link

Add `skip` option for errorLogger #131

Closed vmasto closed 7 years ago

vmasto commented 7 years ago

It would be really helpful to be able to conditionally skip logging in the errorLogger just like in the default logger, e.g. in unit tests.

For now I'm doing the hacky level: process.env.NODE_ENV === 'test' ? 'silly' : 'error'.

rosston commented 7 years ago

I don't really want to enable per-error skipping of error logging, which is what a skip option would do. I think that error logging should be all or nothing (because errors are that important).

It sounds like your use case (unit tests) is also an all-or-nothing scenario. Is there any reason not to just add an if statement around your setup of the error logger? I'm thinking the following is easy/comprehensive enough that it doesn't need to be handled in-module:

if (process.env.NODE_ENV !== 'test') {
  app.use(expressWinston.errorLogger({
    // ... other options here
  }))
}

Let me know if there's something I'm missing about this.

vmasto commented 7 years ago

Yeap you're totally right, this is mostly a code style thing. Just thought since skip is available in logger it would produce cleaner/more consistent code, for example:

const requestLogger = expressWinston.logger({
  ...
  skip: () => process.env.NODE_ENV === 'test'
});
const errorLogger = expressWinston.errorLogger({
  ...
  skip: () => process.env.NODE_ENV === 'test'
});
app.use(logger.requestLogger);
app.use('/', routes);
app.use(logger.errorLogger);

instead of wrapping everything (or half) in if statements.

Anyway just food thought, nothing too major. Thanks for taking the time to respond! :)

rosston commented 7 years ago

Yeah, your example is definitely cleaner. I'll admit that! Thanks for the feedback.

priedthirdeye commented 6 years ago

I too was looking for a way to skip certain errors.

I'm using custom validation errors. This has resulted in much cleaner code for us, but I don't want these custom errors logged in the same way as exceptions.

My use case was adapted from these articles:

This is my current errorLogger config

expressWinston.errorLogger({
    transports: [
      new winston.transports.Console({
        colorize: true,
        level: 'error',
        handleExceptions: true,
        humanReadableUnhandledException: true,
      }),
      new winston.transports.DailyRotateFile({
        filename: `${logDirExceptions}/log`,
        datePattern: 'yyyy-MM-dd_error.',
        prepend: true,
        level: 'error',
        handleExceptions: true,
        humanReadableUnhandledException: true,
      }),
    ],
  })

Is there any way to have control of which errors are logged?

priedthirdeye commented 6 years ago

Disregard previous post.

I see the skip functionality will soon be added in this pull request

https://github.com/bithavoc/express-winston/pull/147

Great news!