winston-logstash: Shouldn't throw exception when it fails to log to logstash?

My application crashes if winston-logstash fails to connect to the logstash.

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: Max retries reached, transport in silent mode, OFFLINE
    at Socket.<anonymous> (/home/hayashis/git/isdp/node_modules/winston-logstash/lib/winston-logstash.js:197:26)
    at Socket.emit (events.js:95:17)
    at TCP.close (net.js:466:12)
16 Aug 20:18:50 - [nodemon] app crashed - waiting for file changes before starting...

In my own opinion, a logging library shouldn’t cause the application to crash (don’t throw exception) if it fails to log… but I am not sure if the author of this library agrees with that or not?

About this issue

  • Original URL
  • State: closed
  • Created 9 years ago
  • Comments: 22 (8 by maintainers)

Most upvoted comments

If you want to handle logstash errors, you can do so like this:

var logstash = new winston.transports.Logstash({ ... });
logstash.on('error', function(err) {
    console.error(err); // replace with your own functionality here
});

@Alino , @soichih: Beside the @joshje solution, you can use max_connect_retries = -1, according winston-logstash docs:

max_connect_retries:

Max number of attempts to reconnect to logstash before going into silence. -1 means retry forever. Default: 4

Best,

@joshje

Thanks for the info.

The reason why I reported this issue is not that I couldn’t handle the error, but that winston-logstash should not cause application to crash under any circumstance; unhanded logging error / log file missing / log file full / can’t write to log, etc, etc…

If you check the implementation of that feature in the Winston, it’s attaching a process exit hook which prevents the exit; I feel that I wouldn’t use that approach in any production code because it could leave the application in an unpredicted state which is in my experience always a bad situation to be.

You always have multiple a solution, as this is open source project 😄

  • You can fork the project, make any changes you want, and use that solution.
  • You could create PR which addresses this issue in a backwards-compatible manner with tests and documentation, and we can continue this discussion. If you feel your solution would help others as this project has helped you.
  • Write your transport which perfectly meets your requirements

While this is a relatively complex transport with an internal state and IO, I think the current solution is the yet best approach to network failure. Transport is transparent about the errors and lets the user decide what to do in the case of an error. Suppose the developer chooses not to address them, it fallback to a safe default, an exception that stops the process. I think this way; it’s simple but not always easy. This is my rationale for the functionality.