Logs lost on ECONNRESET
mitchjmiller opened this issue · 1 comments
Hey,
I'm using this winston transport and am coming across an issue that's a bit of an edge case.
Essentially I have my instance setup with winston and winston-logstash and a seperate logstash server.
However, I'm experiencing issues after even a short period of being idle.
I have max_connect_retries set to -1 (unlimited).
After the idle period when I attempt to send my logs, the connection to the server results in an ECONNRESET error. However, before this point the connected boolean is set to true. The issue is that this ECONNRESET response can take as much as 30seconds to come back and during this time winston-logstash thinks it's connected so it bypasses the queue and attempts to send the logs.
When the ECONNRESET error finally arrives it behaves correctly by setting connected to false and attempting to reconnect, in turn adding all incoming logs to the queue. But again, only after the error is received back. Any logs sent during this time is essentially lost. For my API server which may not be active for a number of minutes and then a request comes in, its possible for all these logs to be lost while the connection waits to fail before re-establishing. Agreed, the TCP connection issue is my own problem, but it would be nice if winston-logstash was able to queue these logs as well in case there are problems.
I've tried a few fixes so far but no full solution just yet but some things that might be worth considering:
- setting keep alive to true for the socket ( self.socket.setKeepAlive(true); ), or maybe as an option.
- a method in which to add logs to the queue on failure. Current implementation is almost fire and forget.
Again, the above is an edge case but has been very troublesome.
Edit:
Working solution for me was to add self.socket.setKeepAlive(true, 60 * 1000) to line 182 of winston-logstash.js. This needs to specifically be applied after the socket is connected otherwise it doesn't seem to have any effect. This little necessity doesn't seem clear in the node.js net documentation. Anyway, hope this is useful and worth considering. Could be useful to have an options parameter.
I'm getting a lot of ETIMEDOUT errors using this lib :(
May try your fix to see if it helps.