From eae16938cdfb498fb6201eb0539cfa7cb7827a65 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben.bridgewater@fintura.de>
Date: Thu, 21 Apr 2016 02:54:42 +0200
Subject: [PATCH] Add monitor transaction warning / error

---
 lib/individualCommands.js | 11 +++------
 lib/multi.js              |  8 +++++++
 test/multi.spec.js        | 48 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/lib/individualCommands.js b/lib/individualCommands.js
index f1b1984f07..c9cef49b1b 100644
--- a/lib/individualCommands.js
+++ b/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;
 };
 
diff --git a/lib/multi.js b/lib/multi.js
index a4399473c9..2a7431e9b6 100644
--- a/lib/multi.js
+++ b/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 = [];
diff --git a/test/multi.spec.js b/test/multi.spec.js
index 48acf3f63a..e49e3390da 100644
--- a/test/multi.spec.js
+++ b/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,53 @@ 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) {
+                            client.on('error', function (err) {
+                                assert.strictEqual(err.code, 'EXECABORT');
+                                done();
+                            });
+                            var multi = client.multi();
+                            multi.set('hello', 'world');
+                            multi.exec();
+                        });
+                    });
+
+                    it('results in a execabort #2', function (done) {
+                        // Check that using monitor with a transactions results in an error
+                        client.multi().set('foo', 'bar').monitor().exec(function (err, res) {
+                            assert.strictEqual(err.code, 'EXECABORT');
+                            done();
+                        });
+                    });
+
+                    it('sanity check', function (done) {
+                        // 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();
+                        });
+                        // 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);
+                            client.end(false);
+                        });
+                    });
+                });
+
                 it('executes a pipelined multi properly in combination with the offline queue', function (done) {
                     var multi1 = client.multi();
                     multi1.set('m1', '123');