Errors from Axios are treated as unhandled exceptions and crash the application

[TheAppleFreak]

@jbojbo reached out to me on Twitter with an interesting issue where if for whatever reason Axios throws an error, it'll be treated as an unhandled exception by Node and will cause the entire application to crash. Among other things, this could happen if Slack was to return an API error such as a 429 Too Many Requests. I'd noticed this issue in my own applications in the form of unpredictable and intermittent crashes, but I had never actually identified the issue myself.

The source of the issue boils down to where the post request to the webhook is actually made, but what confuses me is that there is an error handler on that already. Why is Node treating it as an unhandled error?

[TheAppleFreak]

Toying around with this, and I think the this.emit("error", err) line might be the culprit? While testing, I found that throwing an error in the same place but commenting out the emit resulted in it just working properly, so... maybe that requires its own error handler? Or maybe the error needs to be handled a bit differently. Reading up on the Winston docs to see how emit("error") should be used best, if at all.

Checked a few other transport plugins similar to this one as well to see how they handle it, and the results are largely "we don't do that." winston-graylog2, humio-winston, logdna-winston, newrelic-winston don't have any error handling on send and don't include the emit("error") bit at all, and winston-datadog explicitly ignores errors altogether.

[TheAppleFreak]

Ah, so that's what it does.

Think the best approach would be to put that behind a flag that you'd toggle in the constructor settings, so if you want to be able to handle Slack errors you can do so. Default will be false to match the expected behavior.