Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expressFormat option seems to ignore colorize: false #86

Closed
asmod3us opened this issue Aug 12, 2015 · 6 comments
Closed

expressFormat option seems to ignore colorize: false #86

asmod3us opened this issue Aug 12, 2015 · 6 comments

Comments

@asmod3us
Copy link

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
Copy link
Collaborator

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
Copy link

👍

@floatingLomas
Copy link
Collaborator

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
Copy link

wellsjo commented Jul 12, 2016

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

@rosston
Copy link
Collaborator

rosston commented Jul 13, 2016

Sorry, this is on me. It is not resolved. Resolving it will require a breaking change (roughly along these lines: c464572) 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 rosston mentioned this issue Jul 26, 2016
Merged
@rosston
Copy link
Collaborator

rosston commented Aug 4, 2016

Fixed and released in 2.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants