mranney/node_redis — pull request #533 by chrishamant (2013-12-17 20:49:14+00:00)
Title: removing flush_and_error from on_error handler

There are few pull requests (#463 , #362 and #280 (maybe more)) which have been around a while which both attempt (in slightly different ways) to allow for persistence of commands in the offline_queue after a connection error. I'm honestly in support of either case - this pull request is yet another way of handling this.

Looking at the code I noticed that 'on_error' will call 'connection_gone' after it returns from emitting the Error (implication being that the user is listening for the error event). The 'connection_gone' method calls 'flush_and_error' also (it was already called in 'on_error' so it is kind of redundant I'm guessing). If the first call to 'flush_and_error' is just removed as done in this pull request - the command_queue and offline_queue will still be around on the client when the 'error' event is fired which gives the user time to buffer up the commands from these queues in any way they see fit while handling the error event (kind of moving the solutions in #463 and #362 into userland). Implication of this means that the top level 'error' event is fired before the individual command errors. I can conjure some hypothetical scenarios where this might be a problem (maybe someone started listening for 'error' only after getting error on command?) - but I don't think it would be an actual problem in practice.

Just an idea...