From 0c143a7299c5a875e599afb5e9bf4f0fe971806f Mon Sep 17 00:00:00 2001 From: Jeff Barczewski Date: Tue, 26 Mar 2013 10:26:12 -0500 Subject: [PATCH 1/3] failing tests for empty unsub and punsub When unsubscribe or punsubscribe is called and there is nothing to unsubscribe from, the reply[1] argument is a null which causes a TypeError Cannot call method 'toString' of null ``` TypeError: Cannot call method 'toString' of null at RedisClient.return_reply (/Users/barczewskij/projects/node_redis/index.js:633:65) at ReplyParser.RedisClient.init_parser (/Users/barczewskij/projects/node_redis/index.js:266:14) at ReplyParser.EventEmitter.emit (events.js:96:17) at ReplyParser.send_reply (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:300:10) at ReplyParser.execute (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:211:22) at RedisClient.on_data (/Users/barczewskij/projects/node_redis/index.js:483:27) at Socket. (/Users/barczewskij/projects/node_redis/index.js:82:14) at Socket.EventEmitter.emit (events.js:96:17) at TCP.onread (net.js:396:14) ``` --- test.js | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/test.js b/test.js index 4b26dca9c9..08fea7f5f6 100644 --- a/test.js +++ b/test.js @@ -614,7 +614,7 @@ tests.detect_buffers = function () { assert.strictEqual("", reply[0].inspect(), name); assert.strictEqual("", reply[1].inspect(), name); }); - + // array of strings with undefined values (repro #344) detect_client.hmget("hash key 2", "key 3", "key 4", function(err, reply) { assert.strictEqual(null, err, name); @@ -864,6 +864,44 @@ tests.SUBSCRIBE = function () { }); }; +tests.UNSUB_EMPTY = function () { + // test situation where unsubscribe reply[1] is null + var name = "UNSUB_EMPTY"; + client3.unsubscribe(); // unsubscribe from all so can test null + client3.unsubscribe(); // reply[1] will be null + next(name); +}; + +tests.PUNSUB_EMPTY = function () { + // test situation where punsubscribe reply[1] is null + var name = "PUNSUB_EMPTY"; + client3.punsubscribe(); // punsubscribe from all so can test null + client3.punsubscribe(); // reply[1] will be null + next(name); +}; + +tests.UNSUB_EMPTY_CB = function () { + // test situation where unsubscribe reply[1] is null + var name = "UNSUB_EMPTY_CB"; + client3.unsubscribe(); // unsubscribe from all so can test null + client3.unsubscribe(function (err, results) { + // reply[1] will be null + assert.strictEqual(null, err, "unexpected error: " + err); + next(name); + }); +}; + +tests.PUNSUB_EMPTY_CB = function () { + // test situation where punsubscribe reply[1] is null + var name = "PUNSUB_EMPTY_CB"; + client3.punsubscribe(); // punsubscribe from all so can test null + client3.punsubscribe(function (err, results) { + // reply[1] will be null + assert.strictEqual(null, err, "unexpected error: " + err); + next(name); + }); +}; + tests.SUB_UNSUB_SUB = function () { var name = "SUB_UNSUB_SUB"; client3.subscribe('chan3'); From 655681f79001b471b93b601ea2716957b54ee2d3 Mon Sep 17 00:00:00 2001 From: Jeff Barczewski Date: Tue, 26 Mar 2013 10:49:13 -0500 Subject: [PATCH 2/3] fix empty unsub/punsub TypeError When unsubscribe or punsubscribe is called and it has no subscriptions, the reply[1] is a null which causes `TypeError: Cannot call method 'toString' of null` Check if reply[1] is null before calling toString otherwise just pass null. --- index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index e82d2e5f49..6dab1a73c8 100644 --- a/index.js +++ b/index.js @@ -629,10 +629,12 @@ RedisClient.prototype.return_reply = function (reply) { } // subscribe commands take an optional callback and also emit an event, but only the first response is included in the callback // TODO - document this or fix it so it works in a more obvious way + // reply[1] can be null + var reply1String = (reply[1] === null) ? null : reply[1].toString(); if (command_obj && typeof command_obj.callback === "function") { - try_callback(command_obj.callback, reply[1].toString()); + try_callback(command_obj.callback, reply1String); } - this.emit(type, reply[1].toString(), reply[2]); // channel, count + this.emit(type, reply1String, reply[2]); // channel, count } else { throw new Error("subscriptions are active but got unknown reply type " + type); } @@ -708,7 +710,7 @@ RedisClient.prototype.send_command = function (command, args, callback) { return callback(err); } } - + buffer_args = false; for (i = 0, il = args.length, arg; i < il; i += 1) { if (Buffer.isBuffer(args[i])) { From 383bafd2cf57e7b3ba54dc97504e5e26321ff2f5 Mon Sep 17 00:00:00 2001 From: Jeff Barczewski Date: Wed, 27 Mar 2013 13:39:43 -0500 Subject: [PATCH 3/3] limit cbtests to 2.6.11 and above Test hangs on older versions of Redis --- test.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test.js b/test.js index 08fea7f5f6..cde711482a 100644 --- a/test.js +++ b/test.js @@ -881,8 +881,11 @@ tests.PUNSUB_EMPTY = function () { }; tests.UNSUB_EMPTY_CB = function () { - // test situation where unsubscribe reply[1] is null var name = "UNSUB_EMPTY_CB"; + // test hangs on older versions of redis, so skip + if (!server_version_at_least(client, [2, 6, 11])) return next(name); + + // test situation where unsubscribe reply[1] is null client3.unsubscribe(); // unsubscribe from all so can test null client3.unsubscribe(function (err, results) { // reply[1] will be null @@ -892,8 +895,11 @@ tests.UNSUB_EMPTY_CB = function () { }; tests.PUNSUB_EMPTY_CB = function () { - // test situation where punsubscribe reply[1] is null var name = "PUNSUB_EMPTY_CB"; + // test hangs on older versions of redis, so skip + if (!server_version_at_least(client, [2, 6, 11])) return next(name); + + // test situation where punsubscribe reply[1] is null client3.punsubscribe(); // punsubscribe from all so can test null client3.punsubscribe(function (err, results) { // reply[1] will be null