From cd5cfb4a8eea1b8ad5a3efb8de0187a1929774ed Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 12:32:07 +0200
Subject: [PATCH 01/11] Emit an error when connection permanently goes down

Closes #724 and #615
---
 README.md | 5 +++++
 index.js  | 7 ++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index 0e1d444420..b44d52227f 100644
--- a/README.md
+++ b/README.md
@@ -113,6 +113,11 @@ then replayed just before this event is emitted.
 is set. If this options is set, `connect` will be emitted when the stream is connected, and then
 you are free to try to send commands.
 
+### "reconnecting"
+
+`client` will emit `reconnecting` when trying to reconnect to the Redis server after losing the connection. Listeners
+are passed an object containing `delay` (in ms) and `attempt` (the attempt #) attributes.
+
 ### "error"
 
 `client` will emit `error` when encountering an error connecting to the Redis server.
diff --git a/index.js b/index.js
index 0011ef289c..28c88a53ab 100644
--- a/index.js
+++ b/index.js
@@ -463,9 +463,7 @@ RedisClient.prototype.connection_gone = function (why) {
 
     if (this.max_attempts && this.attempts >= this.max_attempts) {
         this.retry_timer = null;
-        // TODO - some people need a "Redis is Broken mode" for future commands that errors immediately, and others
-        // want the program to exit.  Right now, we just log, which doesn't really help in either case.
-        debug("Couldn't get Redis connection after " + this.max_attempts + " attempts.");
+        this.emit('error', new Error("Redis connection in broken state: maximum connection attempts exceeded."));
         return;
     }
 
@@ -481,8 +479,7 @@ RedisClient.prototype.connection_gone = function (why) {
 
         if (self.connect_timeout && self.retry_totaltime >= self.connect_timeout) {
             self.retry_timer = null;
-            // TODO - engage Redis is Broken mode for future commands, or whatever
-            debug("Couldn't get Redis connection after " + self.retry_totaltime + "ms.");
+            this.emit('error', new Error("Redis connection in broken state: connection timeout exceeded."));
             return;
         }
 

From 04c986a4cd6e8a2968bd9319a5394bcd9ee9a8bd Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 16:17:09 +0200
Subject: [PATCH 02/11] Lowering the first retry_delay and begin from that
 value

Earlier the first value was never used, as it was first multiplied by 1.7
---
 README.md | 4 ++--
 index.js  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index b44d52227f..4e51156cae 100644
--- a/README.md
+++ b/README.md
@@ -581,12 +581,12 @@ some kind of maximum queue depth for pre-connection commands.
 
 ## client.retry_delay
 
-Current delay in milliseconds before a connection retry will be attempted. This starts at `250`.
+Current delay in milliseconds before a connection retry will be attempted. This starts at `200`.
 
 ## client.retry_backoff
 
 Multiplier for future retry timeouts. This should be larger than 1 to add more time between retries.
-Defaults to 1.7. The default initial connection retry is 250, so the second retry will be 425, followed by 723.5, etc.
+Defaults to 1.7. The default initial connection retry is 200, so the second retry will be 340, followed by 578, etc.
 
 ### Commands with Optional and Keyword arguments
 
diff --git a/index.js b/index.js
index 28c88a53ab..cf6a862a21 100644
--- a/index.js
+++ b/index.js
@@ -123,7 +123,7 @@ RedisClient.prototype.install_stream_listeners = function() {
 RedisClient.prototype.initialize_retry_vars = function () {
     this.retry_timer = null;
     this.retry_totaltime = 0;
-    this.retry_delay = 150;
+    this.retry_delay = 200;
     this.retry_backoff = 1.7;
     this.attempts = 1;
 };

From f2ee8dbc9ebe9690061dc93357ede0d85364fa39 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 17:59:25 +0200
Subject: [PATCH 03/11] Add a connection timeout of 24h as new default for
 maximum reconnecting

---
 README.md | 5 +++--
 index.js  | 4 +---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index 4e51156cae..51523b21f5 100644
--- a/README.md
+++ b/README.md
@@ -194,8 +194,9 @@ with an error, or an error will be thrown if no callback is specified.
 * `retry_max_delay`: defaults to `null`. By default every time the client tries to connect and fails time before
 reconnection (delay) almost doubles. This delay normally grows infinitely, but setting `retry_max_delay` limits delay
 to maximum value, provided in milliseconds.
-* `connect_timeout` defaults to `false`. By default client will try reconnecting until connected. Setting `connect_timeout`
-limits total time for client to reconnect. Value is provided in milliseconds and is counted once the disconnect occured.
+* `connect_timeout` defaults to `86400000`. Setting `connect_timeout` limits total time for client to reconnect.
+Value is provided in milliseconds and is counted once the disconnect occured. The last retry is going to happen once after the connect_timeout value is exceeded.
+That way the default is to try reconnecting until at least 24h passed.
 * `max_attempts` defaults to `null`. By default client will try reconnecting until connected. Setting `max_attempts`
 limits total amount of reconnects.
 * `auth_pass` defaults to `null`. By default client will try connecting without auth. If set, client will run redis auth command on connect.
diff --git a/index.js b/index.js
index cf6a862a21..7b4170ad79 100644
--- a/index.js
+++ b/index.js
@@ -58,9 +58,7 @@ function RedisClient(stream, options) {
     this.command_queue = new Queue(); // holds sent commands to de-pipeline them
     this.offline_queue = new Queue(); // holds commands issued but not able to be sent
     this.commands_sent = 0;
-    if (options.connect_timeout && options.connect_timeout > 0) {
-        this.connect_timeout = +options.connect_timeout;
-    }
+    this.connect_timeout = +options.connect_timeout || 86400000; // 24 * 60 * 60 * 1000 ms
     this.enable_offline_queue = true;
     if (this.options.enable_offline_queue === false) {
         this.enable_offline_queue = false;

From 1e0421ac3b3af3944e45c0f1a7286057e3891e24 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 18:00:57 +0200
Subject: [PATCH 04/11] Emit errors if the connection timeout / maximum retry
 attempts have been exceeded

Accept setting max_attempts to zero.
The reconnection event is now emitted when trying to reconnect instead of earlier.
The connection timeout is now going to trigger once after exceeding the maximum timeout instead of stopping earlier.
---
 README.md               |  2 +-
 index.js                | 39 ++++++++++++++++++++-------------------
 test/node_redis.spec.js |  4 ++--
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/README.md b/README.md
index 51523b21f5..7e75d5e630 100644
--- a/README.md
+++ b/README.md
@@ -198,7 +198,7 @@ to maximum value, provided in milliseconds.
 Value is provided in milliseconds and is counted once the disconnect occured. The last retry is going to happen once after the connect_timeout value is exceeded.
 That way the default is to try reconnecting until at least 24h passed.
 * `max_attempts` defaults to `null`. By default client will try reconnecting until connected. Setting `max_attempts`
-limits total amount of reconnects.
+limits total amount of reconnects. Setting this to 0 will prevent any reconnect tries.
 * `auth_pass` defaults to `null`. By default client will try connecting without auth. If set, client will run redis auth command on connect.
 * `family` defaults to `IPv4`. The client connects in IPv4 if not specified or if the DNS resolution returns an IPv4 address.
 You can force an IPv6 if you set the family to 'IPv6'. See nodejs net or dns modules how to use the family type.
diff --git a/index.js b/index.js
index 7b4170ad79..3ad63fa1c5 100644
--- a/index.js
+++ b/index.js
@@ -52,7 +52,8 @@ function RedisClient(stream, options) {
     this.should_buffer = false;
     this.command_queue_high_water = this.options.command_queue_high_water || 1000;
     this.command_queue_low_water = this.options.command_queue_low_water || 0;
-    if (options.max_attempts && options.max_attempts > 0) {
+    this.max_attempts = null;
+    if (options.max_attempts || options.max_attempts === 0 || options.max_attempts === '0') {
         this.max_attempts = +options.max_attempts;
     }
     this.command_queue = new Queue(); // holds sent commands to de-pipeline them
@@ -419,6 +420,16 @@ RedisClient.prototype.connection_gone = function (why) {
         return;
     }
 
+    if (this.max_attempts !== null && this.attempts > this.max_attempts) {
+        this.emit('error', new Error("Redis connection in broken state: maximum connection attempts exceeded."));
+        return;
+    }
+
+    if (this.retry_totaltime > this.connect_timeout) {
+        this.emit('error', new Error("Redis connection in broken state: connection timeout exceeded."));
+        return;
+    }
+
     debug("Redis connection is gone from " + why + " event.");
     this.connected = false;
     this.ready = false;
@@ -436,7 +447,7 @@ RedisClient.prototype.connection_gone = function (why) {
     }
 
     // since we are collapsing end and close, users don't expect to be called twice
-    if (! this.emitted_end) {
+    if (!this.emitted_end) {
         this.emit("end");
         this.emitted_end = true;
     }
@@ -459,30 +470,20 @@ RedisClient.prototype.connection_gone = function (why) {
 
     debug("Retry connection in " + this.retry_delay + " ms");
 
-    if (this.max_attempts && this.attempts >= this.max_attempts) {
-        this.retry_timer = null;
-        this.emit('error', new Error("Redis connection in broken state: maximum connection attempts exceeded."));
-        return;
-    }
-
-    this.attempts += 1;
-    this.emit("reconnecting", {
-        delay: self.retry_delay,
-        attempt: self.attempts
-    });
     this.retry_timer = setTimeout(function () {
         debug("Retrying connection...");
 
-        self.retry_totaltime += self.retry_delay;
+        self.emit("reconnecting", {
+            delay: self.retry_delay,
+            attempt: self.attempts
+        });
 
-        if (self.connect_timeout && self.retry_totaltime >= self.connect_timeout) {
-            self.retry_timer = null;
-            this.emit('error', new Error("Redis connection in broken state: connection timeout exceeded."));
-            return;
-        }
+        self.retry_totaltime += self.retry_delay;
+        self.attempts += 1;
 
         self.stream = net.createConnection(self.connectionOption);
         self.install_stream_listeners();
+
         self.retry_timer = null;
     }, this.retry_delay);
 };
diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js
index bad7439687..38a65f8ecd 100644
--- a/test/node_redis.spec.js
+++ b/test/node_redis.spec.js
@@ -687,7 +687,7 @@ describe("The node_redis client", function () {
                 describe('true', function () {
                     it("does not return an error and enqueues operation", function (done) {
                         var client = redis.createClient(9999, null, {
-                            max_attempts: 1,
+                            max_attempts: 0,
                             parser: parser
                         });
 
@@ -715,7 +715,7 @@ describe("The node_redis client", function () {
                     it("does not emit an error and enqueues operation", function (done) {
                         var client = redis.createClient(9999, null, {
                             parser: parser,
-                            max_attempts: 1,
+                            max_attempts: 0,
                             enable_offline_queue: false
                         });
 

From 0b8705abe9e3a5c4c7bc0e3e020246de2d3a013b Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 18:06:50 +0200
Subject: [PATCH 05/11] Do not run all tests with every single connection (if
 one connection works, the others are going to be fine too)

---
 test/auth.spec.js             |  5 +++--
 test/commands/hgetall.spec.js |  1 -
 test/helper.js                | 35 ++++++++++++++++++++---------------
 test/node_redis.spec.js       |  7 ++++---
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/test/auth.spec.js b/test/auth.spec.js
index 6eb96e0f87..d29b605f27 100644
--- a/test/auth.spec.js
+++ b/test/auth.spec.js
@@ -12,10 +12,11 @@ describe("client authentication", function () {
         });
     });
 
-    helper.allTests(function(parser, ip, args) {
+    helper.allTests({
+        allConnections: true
+    }, function(parser, ip, args) {
 
         describe("using " + parser + " and " + ip, function () {
-            var args = config.configureClient(parser, ip);
             var auth = 'porkchopsandwiches';
             var client = null;
 
diff --git a/test/commands/hgetall.spec.js b/test/commands/hgetall.spec.js
index e45995aa35..958e717c8c 100644
--- a/test/commands/hgetall.spec.js
+++ b/test/commands/hgetall.spec.js
@@ -13,7 +13,6 @@ describe("The 'hgetall' method", function () {
             var client;
 
             describe('regular client', function () {
-                var args = config.configureClient(parser, ip);
 
                 beforeEach(function (done) {
                     client = redis.createClient.apply(redis.createClient, args);
diff --git a/test/helper.js b/test/helper.js
index 5f4d7b4624..7e9fb5ff2a 100644
--- a/test/helper.js
+++ b/test/helper.js
@@ -108,22 +108,27 @@ module.exports = {
         }
         return true;
     },
-    allTests: function (cb) {
-        [undefined].forEach(function (options) { // add buffer option at some point
-            describe(options && options.return_buffers ? "returning buffers" : "returning strings", function () {
-                var parsers = ['javascript'];
-                var protocols = ['IPv4'];
-                if (process.platform !== 'win32') {
-                    parsers.push('hiredis');
-                    protocols.push('IPv6');
+    allTests: function (options, cb) {
+        if (!cb) {
+            cb = options;
+            options = {};
+        }
+        // TODO: Test all different option cases at some point (e.g. buffers)
+        // [undefined, { return_buffers: true }].forEach(function (config_options) {
+        //     describe(config_options && config_options.return_buffers ? "returning buffers" : "returning strings", function () {
+        //     });
+        // });
+        var parsers = ['javascript'];
+        var protocols = ['IPv4'];
+        if (process.platform !== 'win32') {
+            parsers.push('hiredis');
+            protocols.push('IPv6', '/tmp/redis.sock');
+        }
+        parsers.forEach(function (parser) {
+            protocols.forEach(function (ip, i) {
+                if (i === 0 || options.allConnections) {
+                    cb(parser, ip, config.configureClient(parser, ip));
                 }
-
-                parsers.forEach(function (parser) {
-                    if (process.platform !== 'win32') cb(parser, "/tmp/redis.sock", config.configureClient(parser, "/tmp/redis.sock", options));
-                    protocols.forEach(function (ip) {
-                        cb(parser, ip, config.configureClient(parser, ip, options));
-                    });
-                });
             });
         });
     },
diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js
index 38a65f8ecd..9725cce67b 100644
--- a/test/node_redis.spec.js
+++ b/test/node_redis.spec.js
@@ -8,7 +8,9 @@ var redis = config.redis;
 
 describe("The node_redis client", function () {
 
-    helper.allTests(function(parser, ip, args) {
+    helper.allTests({
+        allConnections: true
+    }, function(parser, ip, args) {
 
         if (args[2]) { // skip if options are undefined
             describe("testing parser existence", function () {
@@ -624,7 +626,6 @@ describe("The node_redis client", function () {
 
                 describe('defaults to true', function () {
                     var client;
-                    var args = config.configureClient(parser, ip);
 
                     it("fires client.on('ready')", function (done) {
                         client = redis.createClient.apply(redis.createClient, args);
@@ -703,7 +704,7 @@ describe("The node_redis client", function () {
                                 if (err) return done(err);
                             });
 
-                            return setTimeout(function(){
+                            return setTimeout(function() {
                                 assert.strictEqual(client.offline_queue.length, 1);
                                 return done();
                             }, 25);

From 03e8c035034327c3e8ddd71dccd9d34f8921cc5f Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 18:08:07 +0200
Subject: [PATCH 06/11] Add connection timeout and max attempts tests

---
 test/connection.spec.js | 76 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 test/connection.spec.js

diff --git a/test/connection.spec.js b/test/connection.spec.js
new file mode 100644
index 0000000000..af48d2f9d6
--- /dev/null
+++ b/test/connection.spec.js
@@ -0,0 +1,76 @@
+'use strict';
+
+var assert = require("assert");
+var config = require("./lib/config");
+var helper = require('./helper');
+var redis = config.redis;
+
+describe("on lost connection", function () {
+    helper.allTests(function(parser, ip, args) {
+
+        describe("using " + parser + " and " + ip, function () {
+
+            it("emit an error after max retry attempts and do not try to reconnect afterwards", function (done) {
+                var max_attempts = 3;
+                var client = redis.createClient({
+                    parser: parser,
+                    max_attempts: max_attempts
+                });
+                var calls = 0;
+
+                client.once('ready', function() {
+                    // Pretend that redis can't reconnect
+                    client.on_connect = client.on_error;
+                    client.stream.destroy();
+                });
+
+                client.on("reconnecting", function (params) {
+                    calls++;
+                });
+
+                client.on('error', function(err) {
+                    if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) {
+                        setTimeout(function () {
+                            assert.strictEqual(calls, max_attempts);
+                            done();
+                        }, 1500);
+                    }
+                });
+            });
+
+            it("emit an error after max retry timeout and do not try to reconnect afterwards", function (done) {
+                var connect_timeout = 1000; // in ms
+                var client = redis.createClient({
+                    parser: parser,
+                    connect_timeout: connect_timeout
+                });
+                var time = 0;
+                var multiplier = 0;
+
+                client.once('ready', function() {
+                    // Pretend that redis can't reconnect
+                    client.on_connect = client.on_error;
+                    client.stream.destroy();
+                });
+
+                client.on("reconnecting", function (params) {
+                    if (time > 0 && multiplier === 0) {
+                        multiplier = params.delay / time;
+                    }
+                    time += params.delay;
+                });
+
+                client.on('error', function(err) {
+                    if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) {
+                        setTimeout(function () {
+                            assert(time > connect_timeout);
+                            assert(time / multiplier < connect_timeout);
+                            done();
+                        }, 1500);
+                    }
+                });
+            });
+
+        });
+    });
+});

From a9e7663affb7677e2761db10d4d147abed2aac2f Mon Sep 17 00:00:00 2001
From: Chris Hamant <chris@hamant.net>
Date: Tue, 17 Dec 2013 15:31:53 -0500
Subject: [PATCH 07/11] removing flush_and_error from on_error handler

---
 index.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/index.js b/index.js
index 3ad63fa1c5..db6ca5154a 100644
--- a/index.js
+++ b/index.js
@@ -171,8 +171,6 @@ RedisClient.prototype.on_error = function (msg) {
 
     debug(message);
 
-    this.flush_and_error(message);
-
     this.connected = false;
     this.ready = false;
 

From 3c2ba8c3736fd61fe947dfff365f7150dd639f45 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 18:40:43 +0200
Subject: [PATCH 08/11] Try exactly until the connection timeout has been
 reached

Fixes #587
---
 README.md               |  4 ++--
 index.js                | 11 ++++++-----
 test/connection.spec.js |  7 +------
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/README.md b/README.md
index 7e75d5e630..528a0b894c 100644
--- a/README.md
+++ b/README.md
@@ -195,8 +195,8 @@ with an error, or an error will be thrown if no callback is specified.
 reconnection (delay) almost doubles. This delay normally grows infinitely, but setting `retry_max_delay` limits delay
 to maximum value, provided in milliseconds.
 * `connect_timeout` defaults to `86400000`. Setting `connect_timeout` limits total time for client to reconnect.
-Value is provided in milliseconds and is counted once the disconnect occured. The last retry is going to happen once after the connect_timeout value is exceeded.
-That way the default is to try reconnecting until at least 24h passed.
+Value is provided in milliseconds and is counted once the disconnect occured. The last retry is going to happen exactly at the timeout time.
+That way the default is to try reconnecting until 24h passed.
 * `max_attempts` defaults to `null`. By default client will try reconnecting until connected. Setting `max_attempts`
 limits total amount of reconnects. Setting this to 0 will prevent any reconnect tries.
 * `auth_pass` defaults to `null`. By default client will try connecting without auth. If set, client will run redis auth command on connect.
diff --git a/index.js b/index.js
index db6ca5154a..e20577a684 100644
--- a/index.js
+++ b/index.js
@@ -423,7 +423,7 @@ RedisClient.prototype.connection_gone = function (why) {
         return;
     }
 
-    if (this.retry_totaltime > this.connect_timeout) {
+    if (this.retry_totaltime >= this.connect_timeout) {
         this.emit('error', new Error("Redis connection in broken state: connection timeout exceeded."));
         return;
     }
@@ -459,11 +459,12 @@ RedisClient.prototype.connection_gone = function (why) {
         return;
     }
 
-    var nextDelay = Math.floor(this.retry_delay * this.retry_backoff);
-    if (this.retry_max_delay !== null && nextDelay > this.retry_max_delay) {
+    this.retry_delay = Math.floor(this.retry_delay * this.retry_backoff);
+    if (this.retry_max_delay !== null && this.retry_delay > this.retry_max_delay) {
         this.retry_delay = this.retry_max_delay;
-    } else {
-        this.retry_delay = nextDelay;
+    } else if (this.retry_totaltime + this.retry_delay > this.connect_timeout) {
+        // Do not exceed the maximum
+        this.retry_delay = this.connect_timeout - this.retry_totaltime;
     }
 
     debug("Retry connection in " + this.retry_delay + " ms");
diff --git a/test/connection.spec.js b/test/connection.spec.js
index af48d2f9d6..d76a059941 100644
--- a/test/connection.spec.js
+++ b/test/connection.spec.js
@@ -45,7 +45,6 @@ describe("on lost connection", function () {
                     connect_timeout: connect_timeout
                 });
                 var time = 0;
-                var multiplier = 0;
 
                 client.once('ready', function() {
                     // Pretend that redis can't reconnect
@@ -54,17 +53,13 @@ describe("on lost connection", function () {
                 });
 
                 client.on("reconnecting", function (params) {
-                    if (time > 0 && multiplier === 0) {
-                        multiplier = params.delay / time;
-                    }
                     time += params.delay;
                 });
 
                 client.on('error', function(err) {
                     if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) {
                         setTimeout(function () {
-                            assert(time > connect_timeout);
-                            assert(time / multiplier < connect_timeout);
+                            assert(time === connect_timeout);
                             done();
                         }, 1500);
                     }

From 55d0036eaeb9ccc0b4c71c81249bafc847dd520e Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 10 Sep 2015 19:19:09 +0200
Subject: [PATCH 09/11] Add test and fix keeping the offline queue

Use a new delay after reconnecting
---
 README.md               |  4 ++--
 index.js                | 35 +++++++++++++++--------------------
 test/connection.spec.js |  6 +++---
 test/node_redis.spec.js | 30 +++++++++++++++++++++++++++++-
 4 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/README.md b/README.md
index 528a0b894c..a29c6ae134 100644
--- a/README.md
+++ b/README.md
@@ -197,8 +197,8 @@ to maximum value, provided in milliseconds.
 * `connect_timeout` defaults to `86400000`. Setting `connect_timeout` limits total time for client to reconnect.
 Value is provided in milliseconds and is counted once the disconnect occured. The last retry is going to happen exactly at the timeout time.
 That way the default is to try reconnecting until 24h passed.
-* `max_attempts` defaults to `null`. By default client will try reconnecting until connected. Setting `max_attempts`
-limits total amount of reconnects. Setting this to 0 will prevent any reconnect tries.
+* `max_attempts` defaults to `0`. By default client will try reconnecting until connected. Setting `max_attempts`
+limits total amount of connection tries. Setting this to 1 will prevent any reconnect tries.
 * `auth_pass` defaults to `null`. By default client will try connecting without auth. If set, client will run redis auth command on connect.
 * `family` defaults to `IPv4`. The client connects in IPv4 if not specified or if the DNS resolution returns an IPv4 address.
 You can force an IPv6 if you set the family to 'IPv6'. See nodejs net or dns modules how to use the family type.
diff --git a/index.js b/index.js
index e20577a684..25ebb5c015 100644
--- a/index.js
+++ b/index.js
@@ -52,10 +52,7 @@ function RedisClient(stream, options) {
     this.should_buffer = false;
     this.command_queue_high_water = this.options.command_queue_high_water || 1000;
     this.command_queue_low_water = this.options.command_queue_low_water || 0;
-    this.max_attempts = null;
-    if (options.max_attempts || options.max_attempts === 0 || options.max_attempts === '0') {
-        this.max_attempts = +options.max_attempts;
-    }
+    this.max_attempts = +options.max_attempts || 0;
     this.command_queue = new Queue(); // holds sent commands to de-pipeline them
     this.offline_queue = new Queue(); // holds commands issued but not able to be sent
     this.commands_sent = 0;
@@ -418,16 +415,6 @@ RedisClient.prototype.connection_gone = function (why) {
         return;
     }
 
-    if (this.max_attempts !== null && this.attempts > this.max_attempts) {
-        this.emit('error', new Error("Redis connection in broken state: maximum connection attempts exceeded."));
-        return;
-    }
-
-    if (this.retry_totaltime >= this.connect_timeout) {
-        this.emit('error', new Error("Redis connection in broken state: connection timeout exceeded."));
-        return;
-    }
-
     debug("Redis connection is gone from " + why + " event.");
     this.connected = false;
     this.ready = false;
@@ -450,16 +437,23 @@ RedisClient.prototype.connection_gone = function (why) {
         this.emitted_end = true;
     }
 
-    this.flush_and_error("Redis connection gone from " + why + " event.");
-
     // If this is a requested shutdown, then don't retry
     if (this.closing) {
-        this.retry_timer = null;
-        debug("Connection ended from quit command, not retrying.");
+        debug("connection ended from quit command, not retrying.");
+        this.flush_and_error("Redis connection gone from " + why + " event.");
+        return;
+    }
+
+    if (this.max_attempts !== 0 && this.attempts >= this.max_attempts || this.retry_totaltime >= this.connect_timeout) {
+        this.flush_and_error("Redis connection gone from " + why + " event.");
+        this.end();
+        var message = this.retry_totaltime >= this.connect_timeout ?
+            'connection timeout exceeded.' :
+            'maximum connection attempts exceeded.';
+        this.emit('error', new Error("Redis connection in broken state: " + message));
         return;
     }
 
-    this.retry_delay = Math.floor(this.retry_delay * this.retry_backoff);
     if (this.retry_max_delay !== null && this.retry_delay > this.retry_max_delay) {
         this.retry_delay = this.retry_max_delay;
     } else if (this.retry_totaltime + this.retry_delay > this.connect_timeout) {
@@ -479,6 +473,7 @@ RedisClient.prototype.connection_gone = function (why) {
 
         self.retry_totaltime += self.retry_delay;
         self.attempts += 1;
+        self.retry_delay = Math.round(self.retry_delay * self.retry_backoff);
 
         self.stream = net.createConnection(self.connectionOption);
         self.install_stream_listeners();
@@ -834,7 +829,7 @@ RedisClient.prototype.end = function () {
     //clear retry_timer
     if(this.retry_timer){
         clearTimeout(this.retry_timer);
-        this.retry_timer=null;
+        this.retry_timer = null;
     }
     this.stream.on("error", function(){});
 
diff --git a/test/connection.spec.js b/test/connection.spec.js
index d76a059941..d29214ce1a 100644
--- a/test/connection.spec.js
+++ b/test/connection.spec.js
@@ -11,7 +11,7 @@ describe("on lost connection", function () {
         describe("using " + parser + " and " + ip, function () {
 
             it("emit an error after max retry attempts and do not try to reconnect afterwards", function (done) {
-                var max_attempts = 3;
+                var max_attempts = 4;
                 var client = redis.createClient({
                     parser: parser,
                     max_attempts: max_attempts
@@ -31,7 +31,7 @@ describe("on lost connection", function () {
                 client.on('error', function(err) {
                     if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) {
                         setTimeout(function () {
-                            assert.strictEqual(calls, max_attempts);
+                            assert.strictEqual(calls, max_attempts - 1);
                             done();
                         }, 1500);
                     }
@@ -40,7 +40,7 @@ describe("on lost connection", function () {
 
             it("emit an error after max retry timeout and do not try to reconnect afterwards", function (done) {
                 var connect_timeout = 1000; // in ms
-                var client = redis.createClient({
+                client = redis.createClient({
                     parser: parser,
                     connect_timeout: connect_timeout
                 });
diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js
index 9725cce67b..deb79c3af8 100644
--- a/test/node_redis.spec.js
+++ b/test/node_redis.spec.js
@@ -688,7 +688,7 @@ describe("The node_redis client", function () {
                 describe('true', function () {
                     it("does not return an error and enqueues operation", function (done) {
                         var client = redis.createClient(9999, null, {
-                            max_attempts: 0,
+                            max_attempts: 1,
                             parser: parser
                         });
 
@@ -710,6 +710,34 @@ describe("The node_redis client", function () {
                             }, 25);
                         }, 50);
                     });
+
+                    it("enqueues operation and keep the queue while trying to reconnect", function (done) {
+                        var client = redis.createClient(9999, null, {
+                            max_attempts: 4,
+                            parser: parser
+                        });
+                        var i = 0;
+
+                        client.on('error', function(err) {
+                            if (err.message === 'Redis connection in broken state: maximum connection attempts exceeded.') {
+                                assert(i, 3);
+                                assert.strictEqual(client.offline_queue.length, 0);
+                                done();
+                            }
+                        });
+
+                        client.on('reconnecting', function(params) {
+                            i++;
+                            assert.equal(params.attempt, i);
+                            assert.strictEqual(client.offline_queue.length, 1);
+                        });
+
+                        client.set('foo', 'bar', function(err, result) {
+                            assert(i, 3);
+                            assert('Redis connection gone from error event', err.message);
+                            assert.strictEqual(client.offline_queue.length, 0);
+                        });
+                    });
                 });
 
                 describe('false', function () {

From 30ec1cd6a2095fc09893e2eaf154d83e8274abee Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Sun, 13 Sep 2015 00:26:21 +0200
Subject: [PATCH 10/11] shift in the while loop

---
 .jshintrc |  2 +-
 index.js  | 16 +++++++---------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/.jshintrc b/.jshintrc
index c5caf777dd..92441eb976 100644
--- a/.jshintrc
+++ b/.jshintrc
@@ -17,7 +17,7 @@
     "mocha": true,
 
     // Relaxing options
-    "boss": true, // Accept things like `while (command = keys.shift()) { ... }`
+    "boss": true, // Accept statements like `while (key = keys.pop()) {}`
 
     "overrides": {
         "examples/*.js": {
diff --git a/index.js b/index.js
index 25ebb5c015..113c2141b2 100644
--- a/index.js
+++ b/index.js
@@ -142,16 +142,14 @@ RedisClient.prototype.flush_and_error = function (message) {
 
     error = new Error(message);
 
-    while (this.offline_queue.length > 0) {
-        command_obj = this.offline_queue.shift();
+    while (command_obj = this.offline_queue.shift()) {
         if (typeof command_obj.callback === "function") {
             command_obj.callback(error);
         }
     }
     this.offline_queue = new Queue();
 
-    while (this.command_queue.length > 0) {
-        command_obj = this.command_queue.shift();
+    while (command_obj = this.command_queue.shift()) {
         if (typeof command_obj.callback === "function") {
             command_obj.callback(error);
         }
@@ -393,8 +391,8 @@ RedisClient.prototype.ready_check = function () {
 RedisClient.prototype.send_offline_queue = function () {
     var command_obj, buffered_writes = 0;
 
-    while (this.offline_queue.length > 0) {
-        command_obj = this.offline_queue.shift();
+    // TODO: Implement queue.pop() as it should be faster than shift and evaluate petka antonovs queue
+    while (command_obj = this.offline_queue.shift()) {
         debug("Sending offline command: " + command_obj.command);
         buffered_writes += !this.send_command(command_obj.command, command_obj.args, command_obj.callback);
     }
@@ -826,12 +824,12 @@ RedisClient.prototype.pub_sub_command = function (command_obj) {
 RedisClient.prototype.end = function () {
     this.stream._events = {};
 
-    //clear retry_timer
-    if(this.retry_timer){
+    // Clear retry_timer
+    if (this.retry_timer){
         clearTimeout(this.retry_timer);
         this.retry_timer = null;
     }
-    this.stream.on("error", function(){});
+    this.stream.on("error", function noop(){});
 
     this.connected = false;
     this.ready = false;

From 89c8dd056bdb548f6d667303f4f47f9777be7125 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Sun, 13 Sep 2015 00:27:50 +0200
Subject: [PATCH 11/11] Do not emit the broken mode twice if exec is called

Add more tests
---
 index.js                | 21 ++++++++++++---------
 test/connection.spec.js | 27 ++++++++++++++++++++-------
 test/helper.js          | 10 ++++++++++
 test/node_redis.spec.js | 38 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 79 insertions(+), 17 deletions(-)

diff --git a/index.js b/index.js
index 113c2141b2..400ac95285 100644
--- a/index.js
+++ b/index.js
@@ -137,10 +137,8 @@ RedisClient.prototype.unref = function () {
 };
 
 // flush offline_queue and command_queue, erroring any items with a callback first
-RedisClient.prototype.flush_and_error = function (message) {
-    var command_obj, error;
-
-    error = new Error(message);
+RedisClient.prototype.flush_and_error = function (error) {
+    var command_obj;
 
     while (command_obj = this.offline_queue.shift()) {
         if (typeof command_obj.callback === "function") {
@@ -438,17 +436,19 @@ RedisClient.prototype.connection_gone = function (why) {
     // If this is a requested shutdown, then don't retry
     if (this.closing) {
         debug("connection ended from quit command, not retrying.");
-        this.flush_and_error("Redis connection gone from " + why + " event.");
+        this.flush_and_error(new Error("Redis connection gone from " + why + " event."));
         return;
     }
 
     if (this.max_attempts !== 0 && this.attempts >= this.max_attempts || this.retry_totaltime >= this.connect_timeout) {
-        this.flush_and_error("Redis connection gone from " + why + " event.");
-        this.end();
         var message = this.retry_totaltime >= this.connect_timeout ?
             'connection timeout exceeded.' :
             'maximum connection attempts exceeded.';
-        this.emit('error', new Error("Redis connection in broken state: " + message));
+        var error = new Error("Redis connection in broken state: " + message);
+        error.code = 'CONNECTION_BROKEN';
+        this.flush_and_error(error);
+        this.emit('error', error);
+        this.end();
         return;
     }
 
@@ -1035,7 +1035,7 @@ Multi.prototype.exec = Multi.prototype.EXEC = function (callback) {
 
     // TODO - make this callback part of Multi.prototype instead of creating it each time
     return this._client.send_command("exec", [], function (err, replies) {
-        if (err) {
+        if (err && !err.code) {
             if (callback) {
                 errors.push(err);
                 callback(errors);
@@ -1071,6 +1071,9 @@ Multi.prototype.exec = Multi.prototype.EXEC = function (callback) {
 
         if (callback) {
             callback(null, replies);
+        } else if (err && err.code !== 'CONNECTION_BROKEN') {
+            // Exclude CONNECTION_BROKEN so that error won't be emitted twice
+            self._client.emit('error', err);
         }
     });
 };
diff --git a/test/connection.spec.js b/test/connection.spec.js
index d29214ce1a..a59ff8deaf 100644
--- a/test/connection.spec.js
+++ b/test/connection.spec.js
@@ -19,9 +19,7 @@ describe("on lost connection", function () {
                 var calls = 0;
 
                 client.once('ready', function() {
-                    // Pretend that redis can't reconnect
-                    client.on_connect = client.on_error;
-                    client.stream.destroy();
+                    helper.killConnection(client);
                 });
 
                 client.on("reconnecting", function (params) {
@@ -40,16 +38,14 @@ describe("on lost connection", function () {
 
             it("emit an error after max retry timeout and do not try to reconnect afterwards", function (done) {
                 var connect_timeout = 1000; // in ms
-                client = redis.createClient({
+                var client = redis.createClient({
                     parser: parser,
                     connect_timeout: connect_timeout
                 });
                 var time = 0;
 
                 client.once('ready', function() {
-                    // Pretend that redis can't reconnect
-                    client.on_connect = client.on_error;
-                    client.stream.destroy();
+                    helper.killConnection(client);
                 });
 
                 client.on("reconnecting", function (params) {
@@ -66,6 +62,23 @@ describe("on lost connection", function () {
                 });
             });
 
+            it("end connection while retry is still ongoing", function (done) {
+                var connect_timeout = 1000; // in ms
+                var client = redis.createClient({
+                    parser: parser,
+                    connect_timeout: connect_timeout
+                });
+
+                client.once('ready', function() {
+                    helper.killConnection(client);
+                });
+
+                client.on("reconnecting", function (params) {
+                    client.end();
+                    setTimeout(done, 100);
+                });
+            });
+
         });
     });
 });
diff --git a/test/helper.js b/test/helper.js
index 7e9fb5ff2a..b57f31b3a1 100644
--- a/test/helper.js
+++ b/test/helper.js
@@ -145,5 +145,15 @@ module.exports = {
                 func();
             }
         };
+    },
+    killConnection: function (client) {
+        // Change the connection option to a non existing one and destroy the stream
+        client.connectionOption = {
+            port: 6370,
+            host: '127.0.0.2',
+            family: 4
+        };
+        client.address = '127.0.0.2:6370';
+        client.stream.destroy();
     }
 };
diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js
index deb79c3af8..14d9913b17 100644
--- a/test/node_redis.spec.js
+++ b/test/node_redis.spec.js
@@ -729,9 +729,11 @@ describe("The node_redis client", function () {
                         client.on('reconnecting', function(params) {
                             i++;
                             assert.equal(params.attempt, i);
-                            assert.strictEqual(client.offline_queue.length, 1);
+                            assert.strictEqual(client.offline_queue.length, 2);
                         });
 
+                        // Should work with either a callback or without
+                        client.set('baz', 13);
                         client.set('foo', 'bar', function(err, result) {
                             assert(i, 3);
                             assert('Redis connection gone from error event', err.message);
@@ -764,6 +766,40 @@ describe("The node_redis client", function () {
                             });
                         });
                     });
+
+                    it("flushes the command queue connection if in broken connection mode", function (done) {
+                        var client = redis.createClient({
+                            parser: parser,
+                            max_attempts: 2,
+                            enable_offline_queue: false
+                        });
+
+                        client.once('ready', function() {
+                            var multi = client.multi();
+                            multi.config("bar");
+                            var cb = function(err, reply) {
+                                assert.equal(err.code, 'CONNECTION_BROKEN');
+                            };
+                            for (var i = 0; i < 10; i += 2) {
+                                multi.set("foo" + i, "bar" + i);
+                                multi.set("foo" + (i + 1), "bar" + (i + 1), cb);
+                            }
+                            multi.exec();
+                            assert.equal(client.command_queue.length, 13);
+                            helper.killConnection(client);
+                        });
+
+                        client.on("reconnecting", function (params) {
+                            assert.equal(client.command_queue.length, 13);
+                        });
+
+                        client.on('error', function(err) {
+                            if (/Redis connection in broken state:/.test(err.message)) {
+                                assert.equal(client.command_queue.length, 0);
+                                done();
+                            }
+                        });
+                    });
                 });
             });