winstonjs / winston

A logger for just about everything.
http://github.com/winstonjs/winston
MIT License
22.87k stars 1.81k forks source link

logger.close & transport.close should return promises #1783

Open mlandisbqs opened 4 years ago

mlandisbqs commented 4 years ago

Please tell us about your environment:

What is the problem?

I have a seeding script that runs locally and in builds. It relies on a winston logger that can be configured with multiple transports, including a remote sink over UDP (papertrail), and a daily rotate file transport. When the main function completes and is done with the logger the last line executes logger.close(). The code hangs waiting for the transports to close.

It's very important for the seeding script to exit cleanly and accurately. The build should fail if there is a seeding error - which is based on the return code.

Also: I could not find any clear documentation on how to shut down your logger (and it's transports). I spent a lot of time today dissecting winston code to figure out what is happening.

Lacking a contract for shutting down, each Transport handles things differently.

My workaround below takes advantage of this, but since it relies on the implementation detail and not a contract, the code could break when we upgrade winston.

What do you expect to happen instead?

Other information

Here is a workaround... the winston logger is wrapped in a nestjs LoggerService

LoggerService Snippet

  async close() {
    const promises: Promise<any>[] = [];

    // close all transports -- transports dont use promises...
    // syslog close function emits 'closed' when done
    // daily-rotate-file close function emits 'finish' when done
    for (const transport of this.logger.transports) {
      if (transport.close) {
        const promise = new Promise(resolve => {
          transport.once('closed', () => {
            resolve();
          });
          transport.once('finish', () => {
            resolve();
          });
        });
        promises.push(promise);
        // transport.close();  <-- invoked by logger.close()
      }
    }

    this.logger.close();
    return Promise.all(promises);
  }

Calling Code Snippet:

    try {
      await logger.close();
    } catch (err) {
      console.log(err);
      process.exit(1);
    }
    process.exit(0);
JohnHardy commented 4 years ago

Thank you for posting your close() function - it works like a charm for me (also in a wrapper with a console, daily-rotate-file, and custom sentry transport). πŸ‘

Waiting for a log to close is basic and the lack of these contracts is a weakness that makes the library difficult to deal with, so thanks for posting your workaround.