Skip to content

Commit b9540d4

Browse files
author
Ruben Bridgewater
committed
Fix monitor mode not working with IPv6, sockets or lua scripts
Fixes #1189 Fixes #1037
1 parent 4b27f79 commit b9540d4

File tree

5 files changed

+220
-187
lines changed

5 files changed

+220
-187
lines changed

‎changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
Changelog
22
=========
33

4+
## v.2.6.4 - 12 Jan, 2017
5+
6+
Bugfixes
7+
8+
- Fixed monitor mode not working in combination with IPv6, sockets or lua scripts (2.6.0 regression)
9+
410
## v.2.6.3 - 31 Oct, 2016
511

612
Bugfixes

‎index.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -791,18 +791,14 @@ function return_pub_sub (self, reply) {
791791
}
792792

793793
RedisClient.prototype.return_reply = function (reply) {
794-
// If in monitor mode, all normal commands are still working and we only want to emit the streamlined commands
795-
// As this is not the average use case and monitor is expensive anyway, let's change the code here, to improve
796-
// the average performance of all other commands in case of no monitor mode
797794
if (this.monitoring) {
798795
var replyStr;
799796
if (this.buffers && Buffer.isBuffer(reply)) {
800797
replyStr = reply.toString();
801798
} else {
802799
replyStr = reply;
803800
}
804-
// While reconnecting the redis server does not recognize the client as in monitor mode anymore
805-
// Therefore the monitor command has to finish before it catches further commands
801+
// If in monitor mode, all normal commands are still working and we only want to emit the streamlined commands
806802
if (typeof replyStr === 'string' && utils.monitor_regex.test(replyStr)) {
807803
var timestamp = replyStr.slice(0, replyStr.indexOf(' '));
808804
var args = replyStr.slice(replyStr.indexOf('"') + 1, -1).split('" "').map(function (elem) {

‎lib/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ module.exports = {
127127
reply_to_object: replyToObject,
128128
print: print,
129129
err_code: /^([A-Z]+)\s+(.+)$/,
130-
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}\].*/,
130+
monitor_regex: /^[0-9]{10,11}\.[0-9]+ \[[0-9]{1,3} (.(?!\]))+.\]( ".+?")+$/,
131131
clone: convenienceClone,
132132
callback_or_emit: callbackOrEmit,
133133
reply_in_order: replyInOrder

‎test/commands/monitor.spec.js

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
'use strict';
2+
3+
var assert = require('assert');
4+
var config = require('../lib/config');
5+
var helper = require('../helper');
6+
var utils = require('../../lib/utils');
7+
var redis = config.redis;
8+
9+
describe("The 'monitor' method", function () {
10+
11+
helper.allTests(function (parser, ip, args) {
12+
13+
var client;
14+
15+
afterEach(function () {
16+
client.end(true);
17+
});
18+
19+
beforeEach(function (done) {
20+
client = redis.createClient.apply(null, args);
21+
client.once('connect', function () {
22+
client.flushdb(done);
23+
});
24+
});
25+
26+
it('monitors commands on all redis clients and works in the correct order', function (done) {
27+
var monitorClient = redis.createClient.apply(null, args);
28+
var responses = [
29+
['mget', 'some', 'keys', 'foo', 'bar'],
30+
['set', 'json', '{"foo":"123","bar":"sdflkdfsjk","another":false}'],
31+
['eval', "return redis.call('set', 'sha', 'test')", '0'],
32+
['set', 'sha', 'test'],
33+
['get', 'baz'],
34+
['set', 'foo', 'bar" "s are " " good!"'],
35+
['mget', 'foo', 'baz'],
36+
['subscribe', 'foo', 'baz']
37+
];
38+
var end = helper.callFuncAfter(done, 5);
39+
40+
monitorClient.set('foo', 'bar');
41+
monitorClient.flushdb();
42+
monitorClient.monitor(function (err, res) {
43+
assert.strictEqual(res, 'OK');
44+
client.mget('some', 'keys', 'foo', 'bar');
45+
client.set('json', JSON.stringify({
46+
foo: '123',
47+
bar: 'sdflkdfsjk',
48+
another: false
49+
}));
50+
client.eval("return redis.call('set', 'sha', 'test')", 0);
51+
monitorClient.get('baz', function (err, res) {
52+
assert.strictEqual(res, null);
53+
end(err);
54+
});
55+
monitorClient.set('foo', 'bar" "s are " " good!"', function (err, res) {
56+
assert.strictEqual(res, 'OK');
57+
end(err);
58+
});
59+
monitorClient.mget('foo', 'baz', function (err, res) {
60+
assert.strictEqual(res[0], 'bar" "s are " " good!"');
61+
assert.strictEqual(res[1], null);
62+
end(err);
63+
});
64+
monitorClient.subscribe('foo', 'baz', function (err, res) {
65+
// The return value might change in v.3
66+
// assert.strictEqual(res, 'baz');
67+
// TODO: Fix the return value of subscribe calls
68+
end(err);
69+
});
70+
});
71+
72+
monitorClient.on('monitor', function (time, args, rawOutput) {
73+
assert.strictEqual(monitorClient.monitoring, true);
74+
assert.deepEqual(args, responses.shift());
75+
assert(utils.monitor_regex.test(rawOutput), rawOutput);
76+
if (responses.length === 0) {
77+
monitorClient.quit(end);
78+
}
79+
});
80+
});
81+
82+
it('monitors returns strings in the rawOutput even with return_buffers activated', function (done) {
83+
var monitorClient = redis.createClient({
84+
return_buffers: true,
85+
path: '/tmp/redis.sock'
86+
});
87+
88+
monitorClient.MONITOR(function (err, res) {
89+
assert.strictEqual(monitorClient.monitoring, true);
90+
assert.strictEqual(res.inspect(), new Buffer('OK').inspect());
91+
monitorClient.mget('hello', new Buffer('world'));
92+
});
93+
94+
monitorClient.on('monitor', function (time, args, rawOutput) {
95+
assert.strictEqual(typeof rawOutput, 'string');
96+
assert(utils.monitor_regex.test(rawOutput), rawOutput);
97+
assert.deepEqual(args, ['mget', 'hello', 'world']);
98+
// Quit immediatly ends monitoring mode and therefore does not stream back the quit command
99+
monitorClient.quit(done);
100+
});
101+
});
102+
103+
it('monitors reconnects properly and works with the offline queue', function (done) {
104+
var called = false;
105+
client.MONITOR(helper.isString('OK'));
106+
client.mget('hello', 'world');
107+
client.on('monitor', function (time, args, rawOutput) {
108+
assert.strictEqual(client.monitoring, true);
109+
assert(utils.monitor_regex.test(rawOutput), rawOutput);
110+
assert.deepEqual(args, ['mget', 'hello', 'world']);
111+
if (called) {
112+
// End after a reconnect
113+
return done();
114+
}
115+
client.stream.destroy();
116+
client.mget('hello', 'world');
117+
called = true;
118+
});
119+
});
120+
121+
it('monitors reconnects properly and works with the offline queue in a batch statement', function (done) {
122+
var called = false;
123+
var multi = client.batch();
124+
multi.MONITOR(helper.isString('OK'));
125+
multi.mget('hello', 'world');
126+
multi.exec(function (err, res) {
127+
assert.deepEqual(res, ['OK', [null, null]]);
128+
});
129+
client.on('monitor', function (time, args, rawOutput) {
130+
assert.strictEqual(client.monitoring, true);
131+
assert(utils.monitor_regex.test(rawOutput), rawOutput);
132+
assert.deepEqual(args, ['mget', 'hello', 'world']);
133+
if (called) {
134+
// End after a reconnect
135+
return done();
136+
}
137+
client.stream.destroy();
138+
client.mget('hello', 'world');
139+
called = true;
140+
});
141+
});
142+
143+
it('monitor activates even if the command could not be processed properly after a reconnect', function (done) {
144+
client.MONITOR(function (err, res) {
145+
assert.strictEqual(err.code, 'UNCERTAIN_STATE');
146+
});
147+
client.on('error', function (err) {}); // Ignore error here
148+
client.stream.destroy();
149+
var end = helper.callFuncAfter(done, 2);
150+
client.on('monitor', function (time, args, rawOutput) {
151+
assert.strictEqual(client.monitoring, true);
152+
end();
153+
});
154+
client.on('reconnecting', function () {
155+
client.get('foo', function (err, res) {
156+
assert(!err);
157+
assert.strictEqual(client.monitoring, true);
158+
end();
159+
});
160+
});
161+
});
162+
163+
it('monitors works in combination with the pub sub mode and the offline queue', function (done) {
164+
var responses = [
165+
['subscribe', '/foo', '/bar'],
166+
['unsubscribe', '/bar'],
167+
['get', 'foo'],
168+
['subscribe', '/foo'],
169+
['subscribe', 'baz'],
170+
['unsubscribe', 'baz'],
171+
['publish', '/foo', 'hello world']
172+
];
173+
var pub = redis.createClient();
174+
pub.on('ready', function () {
175+
client.MONITOR(function (err, res) {
176+
assert.strictEqual(res, 'OK');
177+
pub.get('foo', helper.isNull());
178+
});
179+
client.subscribe('/foo', '/bar');
180+
client.unsubscribe('/bar');
181+
setTimeout(function () {
182+
client.stream.destroy();
183+
client.once('ready', function () {
184+
pub.publish('/foo', 'hello world');
185+
});
186+
client.set('foo', 'bar', helper.isError());
187+
client.subscribe('baz');
188+
client.unsubscribe('baz');
189+
}, 150);
190+
var called = false;
191+
client.on('monitor', function (time, args, rawOutput) {
192+
assert.deepEqual(args, responses.shift());
193+
assert(utils.monitor_regex.test(rawOutput), rawOutput);
194+
if (responses.length === 0) {
195+
// The publish is called right after the reconnect and the monitor is called before the message is emitted.
196+
// Therefore we have to wait till the next tick
197+
process.nextTick(function () {
198+
assert(called);
199+
client.quit(done);
200+
pub.end(false);
201+
});
202+
}
203+
});
204+
client.on('message', function (channel, msg) {
205+
assert.strictEqual(channel, '/foo');
206+
assert.strictEqual(msg, 'hello world');
207+
called = true;
208+
});
209+
});
210+
});
211+
});
212+
});

0 commit comments

Comments
 (0)