Fix monitoring mode not always activating soon enough

Author: Ruben Bridgewater

lib/individualCommands.js

@@ -55,8 +55,8 @@ RedisClient.prototype.monitor = RedisClient.prototype.MONITOR = function monitor
     // Use a individual command, as this is a special case that does not has to be checked for any other command
     var self = this;
     var call_on_write = function () {
-        // Activating monitor mode has to happen before Redis returned the callback,
-        // as the client could receive monitoring commands before the callback returned through a race condition
+        // Activating monitor mode has to happen before Redis returned the callback. The monitor result is returned first.
+        // Therefore we expect the command to be properly processed. If this is not the case, it's not an issue either.
         self.monitoring = true;
     };
     return this.internal_send_command(new Command('monitor', [], callback, call_on_write));

test/node_redis.spec.js

@@ -336,6 +336,26 @@ describe('The node_redis client', function () {
                         });
                     });
 
+                    it('should retry all commands even if the offline queue is disabled', function (done) {
+                        var bclient = redis.createClient({
+                            parser: parser,
+                            enableOfflineQueue: false,
+                            retryUnfulfilledCommands: true
+                        });
+                        bclient.once('ready', function () {
+                            bclient.blpop('blocking list 2', 5, function (err, value) {
+                                assert.strictEqual(value[0], 'blocking list 2');
+                                assert.strictEqual(value[1], 'initial value');
+                                bclient.end(true);
+                                done(err);
+                            });
+                            setTimeout(function () {
+                                bclient.stream.destroy();
+                                client.rpush('blocking list 2', 'initial value', helper.isNumber(1));
+                            }, 100);
+                        });
+                    });
+
                 });
 
                 describe('.end', function () {
@@ -650,6 +670,7 @@ describe('The node_redis client', function () {
                         });
 
                         monitorClient.on('monitor', function (time, args, rawOutput) {
+                            assert.strictEqual(monitorClient.monitoring, true);
                             responses.push(args);
                             assert(utils.monitor_regex.test(rawOutput), rawOutput);
                             if (responses.length === 6) {
@@ -670,6 +691,7 @@ describe('The node_redis client', function () {
                         });
 
                         monitorClient.MONITOR(function (err, res) {
+                            assert.strictEqual(monitorClient.monitoring, true);
                             assert.strictEqual(res.inspect(), new Buffer('OK').inspect());
                             client.mget('hello', new Buffer('world'));
                         });
@@ -688,6 +710,7 @@ describe('The node_redis client', function () {
                         client.MONITOR(helper.isString('OK'));
                         client.mget('hello', 'world');
                         client.on('monitor', function (time, args, rawOutput) {
+                            assert.strictEqual(client.monitoring, true);
                             assert(utils.monitor_regex.test(rawOutput), rawOutput);
                             assert.deepEqual(args, ['mget', 'hello', 'world']);
                             if (i++ === 2) {
@@ -708,6 +731,7 @@ describe('The node_redis client', function () {
                             assert.deepEqual(res, ['OK', [null, null]]);
                         });
                         client.on('monitor', function (time, args, rawOutput) {
+                            assert.strictEqual(client.monitoring, true);
                             assert(utils.monitor_regex.test(rawOutput), rawOutput);
                             assert.deepEqual(args, ['mget', 'hello', 'world']);
                             if (i++ === 2) {
@@ -719,20 +743,22 @@ describe('The node_redis client', function () {
                         });
                     });
 
-                    it('monitor does not activate if the command could not be processed properly', function (done) {
+                    it('monitor activates even if the command could not be processed properly after a reconnect', function (done) {
                         client.MONITOR(function (err, res) {
                             assert.strictEqual(err.code, 'UNCERTAIN_STATE');
                         });
                         client.on('error', function (err) {}); // Ignore error here
                         client.stream.destroy();
+                        var end = helper.callFuncAfter(done, 2);
                         client.on('monitor', function (time, args, rawOutput) {
-                            done(new Error('failed')); // Should not be activated
+                            assert.strictEqual(client.monitoring, true);
+                            end();
                         });
                         client.on('reconnecting', function () {
                             client.get('foo', function (err, res) {
                                 assert(!err);
-                                assert.strictEqual(client.monitoring, false);
-                                setTimeout(done, 10); // The monitor command might be returned a tiny bit later
+                                assert.strictEqual(client.monitoring, true);
+                                end();
                             });
                         });
                     });