Custom log levels can be silent unless level is also specified

[ofrobots]

It is easy to write the following code and not realize that one has made a mistake:

const logger = winston.createLogger({
  levels: { eeny: 0, meeny: 1, miney: 2, moe: 3},
  transports: [
    new winston.transports.Console(),
  ]
});

logger.meeny('catch a tiger by the toe'); // never hollers

This could be addressed by the following check in the Logger constructor:

    if (!Object.keys(this.levels).includes(this.level)) {
      throw new Error(`Logger level ${this.level} is not in the levels: ${this.levels}`);
    }

I can open a PR if people agree it is worth fixing.

[indexzero]

This makes sense –– 👍 to a PR.

[arpitsingh94]

Hi, I raised a PR to resolve this where the logger constructor will throw an error if "level" is not one of the custom log levels.

However this change causes more issues that can be seen through the following example:

const logger1 = winston.createLogger({
  levels: { eeny: 0, meeny: 1, miney: 2, moe: 3},
  transports: [
    new winston.transports.Console(),
  ]
});

const logger2 = winston.createLogger({
  levels: { eeny: 0, meeny: 1, miney: 2, moe: 3},
  transports: [
    new winston.transports.Console({level: "meeny"}),
  ]
});

const logger3 = winston.createLogger({
  levels: { eeny: 0, meeny: 1, miney: 2, moe: 3}
});
logger3.add(new winston.transports.Console({level: "miney"}));

logger1.meeny("some message"); //does not log
logger2.meeny("some message"); //logs
logger3.meeny("some message"); //logs

logger2 and logger3 are perfectly valid ways of constructing loggers. However, if an error is thrown for logger1 as discussed previously in this thread, it will also throw for logger2 and logger3.

I do not see an easy resolution for this issue hence.

[kishan210patel]

As a bandaid fix, would it be beneficial to turn the error into a normal console.log statement? It'd be helpful for debugging situations like logger1 where finding where the error occurred can be difficult. It would also promote for engineers to have the level be defined across the board, which would standardize the code. Should I make a PR?

[ckujawa]

I see a couple people have taken a look at this, but I figured I'd take a stab at it as well...

Would something like this work?

if (!Object.keys(this.levels).includes(this.level)){
      const message = [
        '[winston] Log level <${this.level}> is not defined in levels definition.',
        'Please double check the setup of your transports and/or custom log levels.',
        'This transport will be set to silent.',
        'See https://github.com/winstonjs/winston#logging-levels for more information.'
      ]
      console.warn(message.join('\n'));
      this.silent = true;
    }

The other option I had considered was to just set the log level to 0, which would essentially be silent yet still log errors?

Let me know if I should open a PR or modify the approach...

[ckujawa]

I created said PR since I haven't heard back.

[Muhammad-Abdullah-shahzad]

I have raised a PR addressing this issue , since i use this library daily for production ready applications I often face this problem , when I wont specify level property when using custom levels , logs wont work , I have written some code and tested it so that it solves this problem .If maintainers of this library can review my PR and give me feedback , that would be great :).