bithavoc / express-winston

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

expressFormat option seems to ignore colorize: false #86

Closed asmod3us closed 8 years ago

asmod3us commented 9 years ago

I'm using a winston logger in my app:

var logger = new winston.Logger({
  transports: [ new winston.transports.File({
          filename: 'test.log',
          level: 'debug',
          prettyPrint: true,
          colorize: false,
          timestamp: true,
          json: false
        })
  ]
});

In addition, I use a request logger with express-winston configured to use the same winston instance:

var requestLogger = expressWinston.logger({
  winstonInstance: logger,
  expressFormat: true
});

Despite colorize: false being set the requestLogger seems to produce ANSI escape sequences for colors:

info: ESC[90mPOST /clientsESC[39m ESC[33m400ESC[39m ESC[90m1123msESC[39m
{ req:
   { url: '/clients',
...

My understanding of the option (based on the comment in the code sample: "Use the default Express/morgan request formatting, with the same colors. Enabling this will override any msg and colorStatus if true. Will only output colors on transports with colorize set to true") was that it should not output these escape sequences for a transport with the given config. Is this correct?

floatingLomas commented 9 years ago

I think your understanding is correct, so I think this is a bug. I'll try to have a look at it in the next couple of days.

Thanks!

andresgarza commented 8 years ago

:+1:

floatingLomas commented 8 years ago

Sorry, life is busy. So this is wholly related to the condition right here:

if (options.colorStatus || options.expressFormat) {
    // Make things colorful...
}

I don't think that options.expressFormat should be considered in that decision at all; if someone wants a colored expressFormat, they can ask for it.

If either of you want to remove the || options.expressFormat and submit a pull request, I'll happily merge it. It's possible that some tests might be checking for that too, so make sure they all pass before you submit the request.

wellsjo commented 8 years ago

has this been resolved? what's the workaround? I don't even see colorize in the code...

rosston commented 8 years ago

Sorry, this is on me. It is not resolved. Resolving it will require a breaking change (roughly along these lines: https://github.com/bithavoc/express-winston/pull/109/commits/c464572e6db5ea85d063f7e878099b16044d95f2) and a major version bump (which will probably also include another breaking change or two).

I think the only workaround at the moment is to not use expressFormat.

The colorize option mentioned in the issue is a winston-level option. In the original issue, it is on the winston transport passed to express-winston.

rosston commented 8 years ago

Fixed and released in 2.0.0.