Add monitor transaction warning / error

Author: Ruben Bridgewater

lib/individualCommands.js

@@ -71,14 +71,9 @@ Multi.prototype.monitor = Multi.prototype.MONITOR = function monitor (callback)
         this.queue.push(['monitor', [], monitor_callback(this._client, callback)]);
         return this;
     }
-    var err = new Error(
-        'You used the monitor command in combination with a transaction. Due to faulty return values of ' +
-        'Redis in this context, the monitor command is now executed without transaction instead and ignored ' +
-        'in the multi statement.'
-    );
-    err.command = 'MONITOR';
-    utils.reply_in_order(this._client, callback, err);
-    this._client.monitor('monitor', callback);
+    // Set multi monitoring to indicate the exec that it should abort
+    // Remove this "hack" as soon as Redis might fix this
+    this.monitoring = true;
     return this;
 };
 

lib/multi.js

@@ -85,6 +85,14 @@ function multi_callback (self, err, replies) {
 }
 
 Multi.prototype.exec_transaction = function exec_transaction (callback) {
+    if (this.monitoring || this._client.monitoring) {
+        var err = new Error(
+            'Using transaction with a client that is in monitor mode does not work due to faulty return values of Redis.'
+        );
+        err.command = 'EXEC';
+        err.code = 'EXECABORT';
+        return utils.reply_in_order(this._client, callback, err);
+    }
     var self = this;
     var len = self.queue.length;
     self.errors = [];

test/multi.spec.js

@@ -3,6 +3,7 @@
 var assert = require('assert');
 var config = require('./lib/config');
 var helper = require('./helper');
+var utils = require('../lib/utils');
 var redis = config.redis;
 var zlib = require('zlib');
 var client;
@@ -129,6 +130,52 @@ describe("The 'multi' method", function () {
                     client = redis.createClient.apply(null, args);
                 });
 
+                describe('monitor and transactions do not work together', function () {
+
+                    it('results in a execabort', function (done) {
+                        // Check that transactions in combination with monitor result in an error
+                        client.monitor(function (e) {
+                            var multi = client.multi();
+                            multi.set('hello', 'world');
+                            multi.exec();
+                            client.on('error', function (err) {
+                                assert.strictEqual(err.code, 'EXECABORT');
+                                done(e);
+                            });
+                        });
+                    });
+
+                    it('results in a execabort #2', function (done) {
+                        // Check that using monitor with a transactions results in an error
+                        client.multi().monitor().set('foo', 'bar').exec(function (err, res) {
+                            assert.strictEqual(err.code, 'EXECABORT');
+                            done();
+                        });
+                    });
+
+                    it('sanity check', function (done) {
+                        // Check if Redis still has the error
+                        client.monitor();
+                        client.send_command('multi');
+                        client.send_command('set', ['foo', 'bar']);
+                        client.send_command('get', ['foo']);
+                        client.send_command('exec', function (err, res) {
+                            // res[0] is going to be the monitor result of set
+                            // res[1] is going to be the result of the set command
+                            assert(utils.monitor_regex.test(res[0]));
+                            assert.strictEqual(res[1], 'OK');
+                            assert.strictEqual(res.length, 2);
+                        });
+                        // Remove the listener and add it back again after the error
+                        var mochaListener = helper.removeMochaListener();
+                        process.on('uncaughtException', function (err) {
+                            helper.removeMochaListener();
+                            process.on('uncaughtException', mochaListener);
+                            done();
+                        });
+                    });
+                });
+
                 it('executes a pipelined multi properly in combination with the offline queue', function (done) {
                     var multi1 = client.multi();
                     multi1.set('m1', '123');