From 3e95c55a0301104c9cd6451d71fa5527b7797792 Mon Sep 17 00:00:00 2001 From: Matt Ranney Date: Thu, 11 Aug 2011 11:00:02 -0700 Subject: [PATCH] Fix for [GH-127] Re-initialize the reply parser for every new connection. If a connection is terminated, the parser could be left in a bad state. After the auto-reconnect magic kicks in, it tries to reuse the old parser, which will not work. This change is visible to client programs if you depend on client.reply_parser.name being set immediately. It will now only be set after a connection is established. Thanks to @jhurliman for reporting and @pietern for the fix suggestion. --- changelog.md | 10 +++++++ index.js | 81 +++++++++++++++++++++++++++----------------------- multi_bench.js | 9 +++--- test.js | 3 +- 4 files changed, 60 insertions(+), 43 deletions(-) diff --git a/changelog.md b/changelog.md index 524b1251855..f9a0b204ebd 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,16 @@ Changelog ========= +## v0.6.7 - July 30, 2011 + +(accidentally skipped v0.6.6) + +Fix and test for [GH-123] + +Passing an Array as as the last argument should expand as users +expect. The old behavior was to coerce the arguments into Strings, +which did surprising things with Arrays. + ## v0.6.5 - July 6, 2011 Contributed changes: diff --git a/index.js b/index.js index 8728ceaeac6..a1dd40fe977 100644 --- a/index.js +++ b/index.js @@ -46,44 +46,9 @@ function RedisClient(stream, options) { this.closing = false; this.server_info = {}; this.auth_pass = null; + this.parser_module = null; - var parser_module, self = this; - - if (self.options.parser) { - if (! parsers.some(function (parser) { - if (parser.name === self.options.parser) { - parser_module = parser; - if (exports.debug_mode) { - console.log("Using parser module: " + parser_module.name); - } - return true; - } - })) { - throw new Error("Couldn't find named parser " + self.options.parser + " on this system"); - } - } else { - if (exports.debug_mode) { - console.log("Using default parser module: " + parsers[0].name); - } - parser_module = parsers[0]; - } - - parser_module.debug_mode = exports.debug_mode; - this.reply_parser = new parser_module.Parser({ - return_buffers: self.options.return_buffers || false - }); - - // "reply error" is an error sent back by Redis - this.reply_parser.on("reply error", function (reply) { - self.return_error(new Error(reply)); - }); - this.reply_parser.on("reply", function (reply) { - self.return_reply(reply); - }); - // "error" is bad. Somehow the parser got confused. It'll try to reset and continue. - this.reply_parser.on("error", function (err) { - self.emit("error", new Error("Redis reply parser error: " + err.stack)); - }); + var self = this; this.stream.on("connect", function () { self.on_connect(); @@ -204,6 +169,8 @@ RedisClient.prototype.on_connect = function () { this.stream.setNoDelay(); this.stream.setTimeout(0); + this.init_parser(); + if (this.auth_pass) { this.do_auth(); } else { @@ -218,6 +185,46 @@ RedisClient.prototype.on_connect = function () { } }; +RedisClient.prototype.init_parser = function () { + var self = this; + + if (this.options.parser) { + if (! parsers.some(function (parser) { + if (parser.name === self.options.parser) { + this.parser_module = parser; + if (exports.debug_mode) { + console.log("Using parser module: " + self.parser_module.name); + } + return true; + } + })) { + throw new Error("Couldn't find named parser " + self.options.parser + " on this system"); + } + } else { + if (exports.debug_mode) { + console.log("Using default parser module: " + parsers[0].name); + } + this.parser_module = parsers[0]; + } + + this.parser_module.debug_mode = exports.debug_mode; + + this.reply_parser = new this.parser_module.Parser({ + return_buffers: self.options.return_buffers || false + }); + // "reply error" is an error sent back by Redis + this.reply_parser.on("reply error", function (reply) { + self.return_error(new Error(reply)); + }); + this.reply_parser.on("reply", function (reply) { + self.return_reply(reply); + }); + // "error" is bad. Somehow the parser got confused. It'll try to reset and continue. + this.reply_parser.on("error", function (err) { + self.emit("error", new Error("Redis reply parser error: " + err.stack)); + }); +}; + RedisClient.prototype.ready_check = function () { var self = this; diff --git a/multi_bench.js b/multi_bench.js index b78c126e5e5..4f9b926cce0 100644 --- a/multi_bench.js +++ b/multi_bench.js @@ -58,11 +58,12 @@ function create_clients(callback) { while (active_clients < num_clients) { client = clients[active_clients++] = redis.createClient(6379, "127.0.0.1", client_options); - if (! parser_logged) { - console.log("Using reply parser " + client.reply_parser.name); - parser_logged = true; - } client.on("connect", function () { + if (! parser_logged) { + console.log("Using reply parser " + client.reply_parser.name); + parser_logged = true; + } + // Fire callback when all clients are connected connected += 1; if (connected === num_clients) { diff --git a/test.js b/test.js index 7489e0c6524..d909faebd05 100644 --- a/test.js +++ b/test.js @@ -1194,10 +1194,9 @@ run_next_test = function run_next_test() { } }; -console.log("Using reply parser " + client.reply_parser.name); - client.once("ready", function start_tests() { console.log("Connected to " + client.host + ":" + client.port + ", Redis server version " + client.server_info.redis_version + "\n"); + console.log("Using reply parser " + client.reply_parser.name); run_next_test();