Winston: Improve custom transport exemple

Created on 4 Jul 2018  路  3Comments  路  Source: winstonjs/winston

In Adding Custom Transports code exemple, in the log method, there's no explanation on the purpose of both the callback and setImmediate calls.

log(info, callback) {
    // what is this for? Does Winston need it?
    setImmediate(() => {
      this.emit('logged', info);
    });

    // Perform the writing to the remote service
    callback(); // is it required to include this cb in all transports? Does Winston need it?
  }

Most helpful comment

I agree. I also think the example could be improved. Considering the comment // Perform the writing to the remote service I'd assume the remote service is an async call. After looking at other custom transports, shouldn't the example rather read like this?

log(info, callback = () => {}) {
    callRemoteService(info)
        .then(result => {
            callback(null, result);
            this.emit('logged', info);
        })
        .catch(error => {
            callback(error);
            this.emit('error', error);
        })
}

The default value for the callback I found in the transports.File code.

All 3 comments

I agree. I also think the example could be improved. Considering the comment // Perform the writing to the remote service I'd assume the remote service is an async call. After looking at other custom transports, shouldn't the example rather read like this?

log(info, callback = () => {}) {
    callRemoteService(info)
        .then(result => {
            callback(null, result);
            this.emit('logged', info);
        })
        .catch(error => {
            callback(error);
            this.emit('error', error);
        })
}

The default value for the callback I found in the transports.File code.

Would find this really helpful. The example here is limited as is the example in the https://github.com/winstonjs/winston-transport repository.

Looking through a number of custom transports and no two are the same.

Looks like Callback Implementation is required. Thanks for mentioning this and bringing up this point

Was this page helpful?
0 / 5 - 0 ratings