Add exponential backoff retry strategy as default and fix tests
Motivation
I realized that the default setting of winston-logstash caused it to retry 4 times within 400ms, and then never retry again. For most production services, I think this is not the ideal default.
I thought about just changing this to retry infinitely, but reconnecting every 100ms seems a bit excessive, especially if the server is currently down.
Fix
- Refactor the retry options into a single
retryStrategyoption, while maintaining backwards compatibility with the old options - Add support for
exponentialBackoffstrategy, where we start with a 100ms delay, but double it each time. This allows us to reconnect quickly in most cases, but avoid hammering the server if it's overloaded - Refactor tests to make them work properly
- I realized the old tests for the retry logic didn't actually test anything! None of the retry functions were getting called since it was using a stubbed out connection class
- Created a new mock PlainConnection that properly handles events
- Used that in the tests to test the new retry logic
- Side improvement: Call
socket.end()beforesocket.destroy()to try to gracefully close the connection
Testing
- Added automated tests
- Installed package and verified logging restarted successfully after disconnecting Internet from my local machine
Thanks for the contribution 👍 It's quite a big change and it's taking some time for me to go through it. I'll try to find some time beginning next week to go through the change. I am hesitant to change the current default behaviour, and we will change it. We need to release a new major version because it's not backwards compatible by default. But I get back to you soon as I have time to think. Sorry for the delay.
One alternative (if we don't want to increment major versions, although I think exponential back-off is generally a better and more predictable default so deserves one) is to just change it so that the default is the old behavior and require explicitly passing in the exponentialBackoff strategy to use it