diff --git a/changelog.md b/changelog.md index e90f258966e..add6d9e5ada 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,12 @@ Changelog ========= +## v.2.6.4 - 12 Jan, 2017 + +Bugfixes + +- Fixed monitor mode not working in combination with IPv6, sockets or lua scripts (2.6.0 regression) + ## v.2.6.3 - 31 Oct, 2016 Bugfixes diff --git a/index.js b/index.js index 2f9c2490128..f81519d6011 100644 --- a/index.js +++ b/index.js @@ -791,9 +791,6 @@ function return_pub_sub (self, reply) { } RedisClient.prototype.return_reply = function (reply) { - // If in monitor mode, all normal commands are still working and we only want to emit the streamlined commands - // As this is not the average use case and monitor is expensive anyway, let's change the code here, to improve - // the average performance of all other commands in case of no monitor mode if (this.monitoring) { var replyStr; if (this.buffers && Buffer.isBuffer(reply)) { @@ -801,8 +798,7 @@ RedisClient.prototype.return_reply = function (reply) { } else { replyStr = reply; } - // While reconnecting the redis server does not recognize the client as in monitor mode anymore - // Therefore the monitor command has to finish before it catches further commands + // If in monitor mode, all normal commands are still working and we only want to emit the streamlined commands if (typeof replyStr === 'string' && utils.monitor_regex.test(replyStr)) { var timestamp = replyStr.slice(0, replyStr.indexOf(' ')); var args = replyStr.slice(replyStr.indexOf('"') + 1, -1).split('" "').map(function (elem) { diff --git a/lib/utils.js b/lib/utils.js index e44ed89ea6e..84f05119695 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -127,7 +127,7 @@ module.exports = { reply_to_object: replyToObject, print: print, err_code: /^([A-Z]+)\s+(.+)$/, - monitor_regex: /^[0-9]{10,11}\.[0-9]+ \[[0-9]{1,3} [0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}:[0-9]{1,5}\].*/, + monitor_regex: /^[0-9]{10,11}\.[0-9]+ \[[0-9]{1,3} (.(?!\]))+.\]( ".+?")+$/, clone: convenienceClone, callback_or_emit: callbackOrEmit, reply_in_order: replyInOrder diff --git a/test/commands/monitor.spec.js b/test/commands/monitor.spec.js new file mode 100644 index 00000000000..ec287ccba71 --- /dev/null +++ b/test/commands/monitor.spec.js @@ -0,0 +1,212 @@ +'use strict'; + +var assert = require('assert'); +var config = require('../lib/config'); +var helper = require('../helper'); +var utils = require('../../lib/utils'); +var redis = config.redis; + +describe("The 'monitor' method", function () { + + helper.allTests(function (parser, ip, args) { + + var client; + + afterEach(function () { + client.end(true); + }); + + beforeEach(function (done) { + client = redis.createClient.apply(null, args); + client.once('connect', function () { + client.flushdb(done); + }); + }); + + it('monitors commands on all redis clients and works in the correct order', function (done) { + var monitorClient = redis.createClient.apply(null, args); + var responses = [ + ['mget', 'some', 'keys', 'foo', 'bar'], + ['set', 'json', '{"foo":"123","bar":"sdflkdfsjk","another":false}'], + ['eval', "return redis.call('set', 'sha', 'test')", '0'], + ['set', 'sha', 'test'], + ['get', 'baz'], + ['set', 'foo', 'bar" "s are " " good!"'], + ['mget', 'foo', 'baz'], + ['subscribe', 'foo', 'baz'] + ]; + var end = helper.callFuncAfter(done, 5); + + monitorClient.set('foo', 'bar'); + monitorClient.flushdb(); + monitorClient.monitor(function (err, res) { + assert.strictEqual(res, 'OK'); + client.mget('some', 'keys', 'foo', 'bar'); + client.set('json', JSON.stringify({ + foo: '123', + bar: 'sdflkdfsjk', + another: false + })); + client.eval("return redis.call('set', 'sha', 'test')", 0); + monitorClient.get('baz', function (err, res) { + assert.strictEqual(res, null); + end(err); + }); + monitorClient.set('foo', 'bar" "s are " " good!"', function (err, res) { + assert.strictEqual(res, 'OK'); + end(err); + }); + monitorClient.mget('foo', 'baz', function (err, res) { + assert.strictEqual(res[0], 'bar" "s are " " good!"'); + assert.strictEqual(res[1], null); + end(err); + }); + monitorClient.subscribe('foo', 'baz', function (err, res) { + // The return value might change in v.3 + // assert.strictEqual(res, 'baz'); + // TODO: Fix the return value of subscribe calls + end(err); + }); + }); + + monitorClient.on('monitor', function (time, args, rawOutput) { + assert.strictEqual(monitorClient.monitoring, true); + assert.deepEqual(args, responses.shift()); + assert(utils.monitor_regex.test(rawOutput), rawOutput); + if (responses.length === 0) { + monitorClient.quit(end); + } + }); + }); + + it('monitors returns strings in the rawOutput even with return_buffers activated', function (done) { + var monitorClient = redis.createClient({ + return_buffers: true, + path: '/tmp/redis.sock' + }); + + monitorClient.MONITOR(function (err, res) { + assert.strictEqual(monitorClient.monitoring, true); + assert.strictEqual(res.inspect(), new Buffer('OK').inspect()); + monitorClient.mget('hello', new Buffer('world')); + }); + + monitorClient.on('monitor', function (time, args, rawOutput) { + assert.strictEqual(typeof rawOutput, 'string'); + assert(utils.monitor_regex.test(rawOutput), rawOutput); + assert.deepEqual(args, ['mget', 'hello', 'world']); + // Quit immediatly ends monitoring mode and therefore does not stream back the quit command + monitorClient.quit(done); + }); + }); + + it('monitors reconnects properly and works with the offline queue', function (done) { + var called = false; + 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 (called) { + // End after a reconnect + return done(); + } + client.stream.destroy(); + client.mget('hello', 'world'); + called = true; + }); + }); + + it('monitors reconnects properly and works with the offline queue in a batch statement', function (done) { + var called = false; + var multi = client.batch(); + multi.MONITOR(helper.isString('OK')); + multi.mget('hello', 'world'); + multi.exec(function (err, res) { + 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 (called) { + // End after a reconnect + return done(); + } + client.stream.destroy(); + client.mget('hello', 'world'); + called = true; + }); + }); + + 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) { + assert.strictEqual(client.monitoring, true); + end(); + }); + client.on('reconnecting', function () { + client.get('foo', function (err, res) { + assert(!err); + assert.strictEqual(client.monitoring, true); + end(); + }); + }); + }); + + it('monitors works in combination with the pub sub mode and the offline queue', function (done) { + var responses = [ + ['subscribe', '/foo', '/bar'], + ['unsubscribe', '/bar'], + ['get', 'foo'], + ['subscribe', '/foo'], + ['subscribe', 'baz'], + ['unsubscribe', 'baz'], + ['publish', '/foo', 'hello world'] + ]; + var pub = redis.createClient(); + pub.on('ready', function () { + client.MONITOR(function (err, res) { + assert.strictEqual(res, 'OK'); + pub.get('foo', helper.isNull()); + }); + client.subscribe('/foo', '/bar'); + client.unsubscribe('/bar'); + setTimeout(function () { + client.stream.destroy(); + client.once('ready', function () { + pub.publish('/foo', 'hello world'); + }); + client.set('foo', 'bar', helper.isError()); + client.subscribe('baz'); + client.unsubscribe('baz'); + }, 150); + var called = false; + client.on('monitor', function (time, args, rawOutput) { + assert.deepEqual(args, responses.shift()); + assert(utils.monitor_regex.test(rawOutput), rawOutput); + if (responses.length === 0) { + // The publish is called right after the reconnect and the monitor is called before the message is emitted. + // Therefore we have to wait till the next tick + process.nextTick(function () { + assert(called); + client.quit(done); + pub.end(false); + }); + } + }); + client.on('message', function (channel, msg) { + assert.strictEqual(channel, '/foo'); + assert.strictEqual(msg, 'hello world'); + called = true; + }); + }); + }); + }); +}); diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index e85b3fe347f..e6e5f0987fd 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -6,7 +6,6 @@ var path = require('path'); var intercept = require('intercept-stdout'); var config = require('./lib/config'); var helper = require('./helper'); -var utils = require('../lib/utils'); var fork = require('child_process').fork; var redis = config.redis; var client; @@ -654,186 +653,6 @@ describe('The node_redis client', function () { }); }); - describe('monitor', function () { - it('monitors commands on all redis clients and works in the correct order', function (done) { - var monitorClient = redis.createClient.apply(null, args); - var responses = []; - var end = helper.callFuncAfter(done, 5); - - monitorClient.set('foo', 'bar'); - monitorClient.flushdb(); - monitorClient.monitor(function (err, res) { - assert.strictEqual(res, 'OK'); - client.mget('some', 'keys', 'foo', 'bar'); - client.set('json', JSON.stringify({ - foo: '123', - bar: 'sdflkdfsjk', - another: false - })); - monitorClient.get('baz', function (err, res) { - assert.strictEqual(res, null); - end(err); - }); - monitorClient.set('foo', 'bar" "s are " " good!"', function (err, res) { - assert.strictEqual(res, 'OK'); - end(err); - }); - monitorClient.mget('foo', 'baz', function (err, res) { - assert.strictEqual(res[0], 'bar" "s are " " good!"'); - assert.strictEqual(res[1], null); - end(err); - }); - monitorClient.subscribe('foo', 'baz', function (err, res) { - // The return value might change in v.3 - // assert.strictEqual(res, 'baz'); - // TODO: Fix the return value of subscribe calls - end(err); - }); - }); - - 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) { - assert.deepEqual(responses[0], ['mget', 'some', 'keys', 'foo', 'bar']); - assert.deepEqual(responses[1], ['set', 'json', '{"foo":"123","bar":"sdflkdfsjk","another":false}']); - assert.deepEqual(responses[2], ['get', 'baz']); - assert.deepEqual(responses[3], ['set', 'foo', 'bar" "s are " " good!"']); - assert.deepEqual(responses[4], ['mget', 'foo', 'baz']); - assert.deepEqual(responses[5], ['subscribe', 'foo', 'baz']); - monitorClient.quit(end); - } - }); - }); - - it('monitors returns strings in the rawOutput even with return_buffers activated', function (done) { - var monitorClient = redis.createClient({ - return_buffers: true - }); - - monitorClient.MONITOR(function (err, res) { - assert.strictEqual(monitorClient.monitoring, true); - assert.strictEqual(res.inspect(), new Buffer('OK').inspect()); - client.mget('hello', new Buffer('world')); - }); - - monitorClient.on('monitor', function (time, args, rawOutput) { - assert.strictEqual(typeof rawOutput, 'string'); - assert(utils.monitor_regex.test(rawOutput), rawOutput); - assert.deepEqual(args, ['mget', 'hello', 'world']); - // Quit immediatly ends monitoring mode and therefore does not stream back the quit command - monitorClient.quit(done); - }); - }); - - it('monitors reconnects properly and works with the offline queue', function (done) { - var i = 0; - 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) { - // End after two reconnects - return done(); - } - client.stream.destroy(); - client.mget('hello', 'world'); - }); - }); - - it('monitors reconnects properly and works with the offline queue in a batch statement', function (done) { - var i = 0; - var multi = client.batch(); - multi.MONITOR(helper.isString('OK')); - multi.mget('hello', 'world'); - multi.exec(function (err, res) { - 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) { - // End after two reconnects - return done(); - } - client.stream.destroy(); - client.mget('hello', 'world'); - }); - }); - - 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) { - assert.strictEqual(client.monitoring, true); - end(); - }); - client.on('reconnecting', function () { - client.get('foo', function (err, res) { - assert(!err); - assert.strictEqual(client.monitoring, true); - end(); - }); - }); - }); - - it('monitors works in combination with the pub sub mode and the offline queue', function (done) { - var responses = []; - var pub = redis.createClient(); - pub.on('ready', function () { - client.MONITOR(function (err, res) { - assert.strictEqual(res, 'OK'); - pub.get('foo', helper.isNull()); - }); - client.subscribe('/foo', '/bar'); - client.unsubscribe('/bar'); - setTimeout(function () { - client.stream.destroy(); - client.once('ready', function () { - pub.publish('/foo', 'hello world'); - }); - client.set('foo', 'bar', helper.isError()); - client.subscribe('baz'); - client.unsubscribe('baz'); - }, 150); - var called = false; - client.on('monitor', function (time, args, rawOutput) { - responses.push(args); - assert(utils.monitor_regex.test(rawOutput), rawOutput); - if (responses.length === 7) { - assert.deepEqual(responses[0], ['subscribe', '/foo', '/bar']); - assert.deepEqual(responses[1], ['unsubscribe', '/bar']); - assert.deepEqual(responses[2], ['get', 'foo']); - assert.deepEqual(responses[3], ['subscribe', '/foo']); - assert.deepEqual(responses[4], ['subscribe', 'baz']); - assert.deepEqual(responses[5], ['unsubscribe', 'baz']); - assert.deepEqual(responses[6], ['publish', '/foo', 'hello world']); - // The publish is called right after the reconnect and the monitor is called before the message is emitted. - // Therefore we have to wait till the next tick - process.nextTick(function () { - assert(called); - client.quit(done); - pub.end(false); - }); - } - }); - client.on('message', function (channel, msg) { - assert.strictEqual(channel, '/foo'); - assert.strictEqual(msg, 'hello world'); - called = true; - }); - }); - }); - }); - describe('idle', function () { it('emits idle as soon as there are no outstanding commands', function (done) { var end = helper.callFuncAfter(done, 2);