Fix for [redisGH-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

@@ -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:

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;
 

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) {

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();