Fix explicitly passing undefined as callback

changelog.md

@@ -14,6 +14,7 @@ Bugfixes:
 -  Fix multi.hmset key not being type converted if used with an object and key not being a string (@BridgeAR)
 -  Fix parser errors not being catched properly (@BridgeAR)
 -  Fix a crash that could occur if a redis server does return the info command as usual #541 (@BridgeAR)
+-  Explicitly passing undefined as a callback statement will work again. E.g. client.publish('channel', 'message', undefined); (@BridgeAR)
 
 ## v2.0.1 - Sep 24, 2015
 

index.js

@@ -629,6 +629,7 @@ RedisClient.prototype.return_reply = function (reply) {
             return elem.replace(/\\"/g, '"');
         });
         this.emit("monitor", timestamp, args);
+    /* istanbul ignore else: this is a safety check that we should not be able to trigger */
     } else {
         var err = new Error("node_redis command queue state error. If you can reproduce this, please report it.");
         err.command_obj = command_obj;
@@ -639,21 +640,14 @@ RedisClient.prototype.return_reply = function (reply) {
 RedisClient.prototype.send_command = function (command, args, callback) {
     var arg, command_obj, i, elem_count, buffer_args, stream = this.stream, command_str = "", buffered_writes = 0, err;
 
-    // if (typeof callback === "function") {}
-    // probably the fastest way:
-    //     client.command([arg1, arg2], cb);  (straight passthrough)
-    //         send_command(command, [arg1, arg2], cb);
     if (args === undefined) {
         args = [];
-    } else if (!callback && typeof args[args.length - 1] === "function") {
-        // most people find this variable argument length form more convenient, but it uses arguments, which is slower
-        //     client.command(arg1, arg2, cb);   (wraps up arguments into an array)
-        //       send_command(command, [arg1, arg2, cb]);
-        //     client.command(arg1, arg2);   (callback is optional)
-        //       send_command(command, [arg1, arg2]);
-        //     client.command(arg1, arg2, undefined);   (callback is undefined)
-        //       send_command(command, [arg1, arg2, undefined]);
-        callback = args.pop();
+    } else if (!callback) {
+        if (typeof args[args.length - 1] === "function") {
+            callback = args.pop();
+        } else if (typeof args[args.length - 1] === "undefined") {
+            args.pop();
+        }
     }
 
     if (process.domain && callback) {

test/commands/geoadd.spec.js.future → test/commands/geoadd.spec.js

@@ -4,7 +4,7 @@ var config = require("../lib/config");
 var helper = require("../helper");
 var redis = config.redis;
 
-describe("The 'getoadd' method", function () {
+describe("The 'geoadd' method", function () {
 
     helper.allTests(function(parser, ip, args) {
 
@@ -19,6 +19,7 @@ describe("The 'getoadd' method", function () {
             });
 
             it('returns 1 if the key exists', function (done) {
+                helper.serverVersionAtLeast.call(this, client, [3, 2, 0]);
                 client.geoadd("mycity:21:0:location", "13.361389","38.115556","COR", function(err, res) {
                     console.log(err, res);
                     // geoadd is still in the unstable branch. As soon as it reaches the stable one, activate this test

test/commands/set.spec.js

@@ -89,6 +89,16 @@ describe("The 'set' method", function () {
                             }, 100);
                         });
 
+                        it("sets the value correctly even if the callback is explicitly set to undefined", function (done) {
+                            client.set(key, value, undefined);
+                            setTimeout(function () {
+                                client.get(key, function (err, res) {
+                                    helper.isString(value)(err, res);
+                                    done();
+                                });
+                            }, 100);
+                        });
+
                         it("sets the value correctly with the array syntax", function (done) {
                             client.set([key, value]);
                             setTimeout(function () {

test/commands/zadd.spec.js

@@ -20,12 +20,16 @@ describe("The 'zadd' method", function () {
             });
 
             it('reports an error', function (done) {
+                if (helper.redisProcess().spawnFailed()) this.skip();
                 client.zadd('infinity', [+'5t', "should not be possible"], helper.isError(done));
             });
 
             it('return inf / -inf', function (done) {
+                if (helper.redisProcess().spawnFailed()) this.skip();
+                helper.serverVersionAtLeast.call(this, client, [3, 0, 2]);
                 client.zadd('infinity', [+Infinity, "should be inf"], helper.isNumber(1));
-                client.zadd('infinity', [-Infinity, "should be negative inf"], helper.isNumber(1));
+                client.zadd('infinity', ['inf', 'should be also be inf'], helper.isNumber(1));
+                client.zadd('infinity', -Infinity, "should be negative inf", helper.isNumber(1));
                 client.zadd('infinity', [99999999999999999999999, "should not be inf"], helper.isNumber(1));
                 client.zrange('infinity', 0, -1, 'WITHSCORES', function (err, res) {
                     assert.equal(res[5], 'inf');

test/detect_buffers.spec.js

@@ -0,0 +1,209 @@
+'use strict';
+
+var assert = require("assert");
+var config = require("./lib/config");
+var helper = require('./helper');
+var redis = config.redis;
+
+describe("detect_buffers", function () {
+
+    helper.allTests(function(parser, ip, args) {
+
+        describe("using " + parser + " and " + ip, function () {
+            var client;
+            var args = config.configureClient(parser, ip, {
+                detect_buffers: true
+            });
+
+            beforeEach(function (done) {
+                client = redis.createClient.apply(redis.createClient, args);
+                client.once("error", done);
+                client.once("connect", function () {
+                    client.flushdb(function (err) {
+                        client.hmset("hash key 2", "key 1", "val 1", "key 2", "val 2");
+                        client.set("string key 1", "string value");
+                        return done(err);
+                    });
+                });
+            });
+
+            describe('get', function () {
+                describe('first argument is a string', function () {
+                    it('returns a string', function (done) {
+                        client.get("string key 1", helper.isString("string value", done));
+                    });
+
+                    it('returns a string when executed as part of transaction', function (done) {
+                        client.multi().get("string key 1").exec(helper.isString("string value", done));
+                    });
+                });
+
+                describe('first argument is a buffer', function () {
+                    it('returns a buffer', function (done) {
+                        client.get(new Buffer("string key 1"), function (err, reply) {
+                            assert.strictEqual(true, Buffer.isBuffer(reply));
+                            assert.strictEqual("<Buffer 73 74 72 69 6e 67 20 76 61 6c 75 65>", reply.inspect());
+                            return done(err);
+                        });
+                    });
+
+                    it('returns a bufffer when executed as part of transaction', function (done) {
+                        client.multi().get(new Buffer("string key 1")).exec(function (err, reply) {
+                            assert.strictEqual(1, reply.length);
+                            assert.strictEqual(true, Buffer.isBuffer(reply[0]));
+                            assert.strictEqual("<Buffer 73 74 72 69 6e 67 20 76 61 6c 75 65>", reply[0].inspect());
+                            return done(err);
+                        });
+                    });
+                });
+            });
+
+            describe('multi.hget', function () {
+                it('can interleave string and buffer results', function (done) {
+                    client.multi()
+                        .hget("hash key 2", "key 1")
+                        .hget(new Buffer("hash key 2"), "key 1")
+                        .hget("hash key 2", new Buffer("key 2"))
+                        .hget("hash key 2", "key 2")
+                        .exec(function (err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(4, reply.length);
+                            assert.strictEqual("val 1", reply[0]);
+                            assert.strictEqual(true, Buffer.isBuffer(reply[1]));
+                            assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[1].inspect());
+                            assert.strictEqual(true, Buffer.isBuffer(reply[2]));
+                            assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[2].inspect());
+                            assert.strictEqual("val 2", reply[3]);
+                            return done(err);
+                        });
+                });
+            });
+
+            describe('hmget', function () {
+                describe('first argument is a string', function () {
+                    it('returns strings for keys requested', function (done) {
+                        client.hmget("hash key 2", "key 1", "key 2", function (err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(2, reply.length);
+                            assert.strictEqual("val 1", reply[0]);
+                            assert.strictEqual("val 2", reply[1]);
+                            return done(err);
+                        });
+                    });
+
+                    it('returns strings for keys requested in transaction', function (done) {
+                        client.multi().hmget("hash key 2", "key 1", "key 2").exec(function (err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(1, reply.length);
+                            assert.strictEqual(2, reply[0].length);
+                            assert.strictEqual("val 1", reply[0][0]);
+                            assert.strictEqual("val 2", reply[0][1]);
+                            return done(err);
+                        });
+                    });
+
+                    it('handles array of strings with undefined values (repro #344)', function (done) {
+                        client.hmget("hash key 2", "key 3", "key 4", function(err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(2, reply.length);
+                            assert.equal(null, reply[0]);
+                            assert.equal(null, reply[1]);
+                            return done(err);
+                        });
+                    });
+
+                    it('handles array of strings with undefined values in transaction (repro #344)', function (done) {
+                        client.multi().hmget("hash key 2", "key 3", "key 4").exec(function(err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(1, reply.length);
+                            assert.strictEqual(2, reply[0].length);
+                            assert.equal(null, reply[0][0]);
+                            assert.equal(null, reply[0][1]);
+                            return done(err);
+                        });
+                    });
+                });
+
+                describe('first argument is a buffer', function () {
+                    it('returns buffers for keys requested', function (done) {
+                        client.hmget(new Buffer("hash key 2"), "key 1", "key 2", function (err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(2, reply.length);
+                            assert.strictEqual(true, Buffer.isBuffer(reply[0]));
+                            assert.strictEqual(true, Buffer.isBuffer(reply[1]));
+                            assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0].inspect());
+                            assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[1].inspect());
+                            return done(err);
+                        });
+                    });
+
+                    it("returns buffers for keys requested in transaction", function (done) {
+                        client.multi().hmget(new Buffer("hash key 2"), "key 1", "key 2").exec(function (err, reply) {
+                            assert.strictEqual(true, Array.isArray(reply));
+                            assert.strictEqual(1, reply.length);
+                            assert.strictEqual(2, reply[0].length);
+                            assert.strictEqual(true, Buffer.isBuffer(reply[0][0]));
+                            assert.strictEqual(true, Buffer.isBuffer(reply[0][1]));
+                            assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0][0].inspect());
+                            assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[0][1].inspect());
+                            return done(err);
+                        });
+                    });
+                });
+            });
+
+            describe('hgetall', function (done) {
+                describe('first argument is a string', function () {
+                    it('returns string values', function (done) {
+                        client.hgetall("hash key 2", function (err, reply) {
+                            assert.strictEqual("object", typeof reply);
+                            assert.strictEqual(2, Object.keys(reply).length);
+                            assert.strictEqual("val 1", reply["key 1"]);
+                            assert.strictEqual("val 2", reply["key 2"]);
+                            return done(err);
+                        });
+                    });
+
+                    it('returns string values when executed in transaction', function (done) {
+                        client.multi().hgetall("hash key 2").exec(function (err, reply) {
+                            assert.strictEqual(1, reply.length);
+                            assert.strictEqual("object", typeof reply[0]);
+                            assert.strictEqual(2, Object.keys(reply[0]).length);
+                            assert.strictEqual("val 1", reply[0]["key 1"]);
+                            assert.strictEqual("val 2", reply[0]["key 2"]);
+                            return done(err);
+                        });
+                    });
+                });
+
+                describe('first argument is a buffer', function () {
+                    it('returns buffer values', function (done) {
+                        client.hgetall(new Buffer("hash key 2"), function (err, reply) {
+                            assert.strictEqual(null, err);
+                            assert.strictEqual("object", typeof reply);
+                            assert.strictEqual(2, Object.keys(reply).length);
+                            assert.strictEqual(true, Buffer.isBuffer(reply["key 1"]));
+                            assert.strictEqual(true, Buffer.isBuffer(reply["key 2"]));
+                            assert.strictEqual("<Buffer 76 61 6c 20 31>", reply["key 1"].inspect());
+                            assert.strictEqual("<Buffer 76 61 6c 20 32>", reply["key 2"].inspect());
+                            return done(err);
+                        });
+                    });
+
+                    it('returns buffer values when executed in transaction', function (done) {
+                        client.multi().hgetall(new Buffer("hash key 2")).exec(function (err, reply) {
+                            assert.strictEqual(1, reply.length);
+                            assert.strictEqual("object", typeof reply);
+                            assert.strictEqual(2, Object.keys(reply[0]).length);
+                            assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 1"]));
+                            assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 2"]));
+                            assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0]["key 1"].inspect());
+                            assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[0]["key 2"].inspect());
+                            return done(err);
+                        });
+                    });
+                });
+            });
+        });
+    });
+});

test/node_redis.spec.js

@@ -293,12 +293,12 @@ describe("The node_redis client", function () {
                         assert.strictEqual(client.monitoring, false, "monitoring off at start");
                         client.set("recon 1", "one");
                         client.monitor(function (err, res) {
-                                assert.strictEqual(client.monitoring, true, "monitoring on after monitor()");
-                                client.set("recon 2", "two", function (err, res) {
-                                    // Do not do this in normal programs. This is to simulate the server closing on us.
-                                    // For orderly shutdown in normal programs, do client.quit()
-                                    client.stream.destroy();
-                                });
+                            assert.strictEqual(client.monitoring, true, "monitoring on after monitor()");
+                            client.set("recon 2", "two", function (err, res) {
+                                // Do not do this in normal programs. This is to simulate the server closing on us.
+                                // For orderly shutdown in normal programs, do client.quit()
+                                client.stream.destroy();
+                            });
                         });
                     });
 
@@ -415,203 +415,6 @@ describe("The node_redis client", function () {
                 });
             });
 
-            describe('detect_buffers', function () {
-                var client;
-                var args = config.configureClient(parser, ip, {
-                    detect_buffers: true
-                });
-
-                beforeEach(function (done) {
-                    client = redis.createClient.apply(redis.createClient, args);
-                    client.once("error", done);
-                    client.once("connect", function () {
-                        client.flushdb(function (err) {
-                            client.hmset("hash key 2", "key 1", "val 1", "key 2", "val 2");
-                            client.set("string key 1", "string value");
-                            return done(err);
-                        });
-                    });
-                });
-
-                describe('get', function () {
-                    describe('first argument is a string', function () {
-                        it('returns a string', function (done) {
-                            client.get("string key 1", helper.isString("string value", done));
-                        });
-
-                        it('returns a string when executed as part of transaction', function (done) {
-                            client.multi().get("string key 1").exec(helper.isString("string value", done));
-                        });
-                    });
-
-                    describe('first argument is a buffer', function () {
-                        it('returns a buffer', function (done) {
-                            client.get(new Buffer("string key 1"), function (err, reply) {
-                                assert.strictEqual(true, Buffer.isBuffer(reply));
-                                assert.strictEqual("<Buffer 73 74 72 69 6e 67 20 76 61 6c 75 65>", reply.inspect());
-                                return done(err);
-                            });
-                        });
-
-                        it('returns a bufffer when executed as part of transaction', function (done) {
-                            client.multi().get(new Buffer("string key 1")).exec(function (err, reply) {
-                                assert.strictEqual(1, reply.length);
-                                assert.strictEqual(true, Buffer.isBuffer(reply[0]));
-                                assert.strictEqual("<Buffer 73 74 72 69 6e 67 20 76 61 6c 75 65>", reply[0].inspect());
-                                return done(err);
-                            });
-                        });
-                    });
-                });
-
-                describe('multi.hget', function () {
-                    it('can interleave string and buffer results', function (done) {
-                        client.multi()
-                            .hget("hash key 2", "key 1")
-                            .hget(new Buffer("hash key 2"), "key 1")
-                            .hget("hash key 2", new Buffer("key 2"))
-                            .hget("hash key 2", "key 2")
-                            .exec(function (err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(4, reply.length);
-                                assert.strictEqual("val 1", reply[0]);
-                                assert.strictEqual(true, Buffer.isBuffer(reply[1]));
-                                assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[1].inspect());
-                                assert.strictEqual(true, Buffer.isBuffer(reply[2]));
-                                assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[2].inspect());
-                                assert.strictEqual("val 2", reply[3]);
-                                return done(err);
-                            });
-                    });
-                });
-
-                describe('hmget', function () {
-                    describe('first argument is a string', function () {
-                        it('returns strings for keys requested', function (done) {
-                            client.hmget("hash key 2", "key 1", "key 2", function (err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(2, reply.length);
-                                assert.strictEqual("val 1", reply[0]);
-                                assert.strictEqual("val 2", reply[1]);
-                                return done(err);
-                            });
-                        });
-
-                        it('returns strings for keys requested in transaction', function (done) {
-                            client.multi().hmget("hash key 2", "key 1", "key 2").exec(function (err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(1, reply.length);
-                                assert.strictEqual(2, reply[0].length);
-                                assert.strictEqual("val 1", reply[0][0]);
-                                assert.strictEqual("val 2", reply[0][1]);
-                                return done(err);
-                            });
-                        });
-
-                        it('handles array of strings with undefined values (repro #344)', function (done) {
-                            client.hmget("hash key 2", "key 3", "key 4", function(err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(2, reply.length);
-                                assert.equal(null, reply[0]);
-                                assert.equal(null, reply[1]);
-                                return done(err);
-                            });
-                        });
-
-                        it('handles array of strings with undefined values in transaction (repro #344)', function (done) {
-                            client.multi().hmget("hash key 2", "key 3", "key 4").exec(function(err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(1, reply.length);
-                                assert.strictEqual(2, reply[0].length);
-                                assert.equal(null, reply[0][0]);
-                                assert.equal(null, reply[0][1]);
-                                return done(err);
-                            });
-                        });
-                    });
-
-                    describe('first argument is a buffer', function () {
-                        it('returns buffers for keys requested', function (done) {
-                            client.hmget(new Buffer("hash key 2"), "key 1", "key 2", function (err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(2, reply.length);
-                                assert.strictEqual(true, Buffer.isBuffer(reply[0]));
-                                assert.strictEqual(true, Buffer.isBuffer(reply[1]));
-                                assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0].inspect());
-                                assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[1].inspect());
-                                return done(err);
-                            });
-                        });
-
-                        it("returns buffers for keys requested in transaction", function (done) {
-                            client.multi().hmget(new Buffer("hash key 2"), "key 1", "key 2").exec(function (err, reply) {
-                                assert.strictEqual(true, Array.isArray(reply));
-                                assert.strictEqual(1, reply.length);
-                                assert.strictEqual(2, reply[0].length);
-                                assert.strictEqual(true, Buffer.isBuffer(reply[0][0]));
-                                assert.strictEqual(true, Buffer.isBuffer(reply[0][1]));
-                                assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0][0].inspect());
-                                assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[0][1].inspect());
-                                return done(err);
-                            });
-                        });
-                    });
-                });
-
-                describe('hgetall', function (done) {
-                    describe('first argument is a string', function () {
-                        it('returns string values', function (done) {
-                            client.hgetall("hash key 2", function (err, reply) {
-                                assert.strictEqual("object", typeof reply);
-                                assert.strictEqual(2, Object.keys(reply).length);
-                                assert.strictEqual("val 1", reply["key 1"]);
-                                assert.strictEqual("val 2", reply["key 2"]);
-                                return done(err);
-                            });
-                        });
-
-                        it('returns string values when executed in transaction', function (done) {
-                            client.multi().hgetall("hash key 2").exec(function (err, reply) {
-                                assert.strictEqual(1, reply.length);
-                                assert.strictEqual("object", typeof reply[0]);
-                                assert.strictEqual(2, Object.keys(reply[0]).length);
-                                assert.strictEqual("val 1", reply[0]["key 1"]);
-                                assert.strictEqual("val 2", reply[0]["key 2"]);
-                                return done(err);
-                            });
-                        });
-                    });
-
-                    describe('first argument is a buffer', function () {
-                        it('returns buffer values', function (done) {
-                            client.hgetall(new Buffer("hash key 2"), function (err, reply) {
-                                assert.strictEqual(null, err);
-                                assert.strictEqual("object", typeof reply);
-                                assert.strictEqual(2, Object.keys(reply).length);
-                                assert.strictEqual(true, Buffer.isBuffer(reply["key 1"]));
-                                assert.strictEqual(true, Buffer.isBuffer(reply["key 2"]));
-                                assert.strictEqual("<Buffer 76 61 6c 20 31>", reply["key 1"].inspect());
-                                assert.strictEqual("<Buffer 76 61 6c 20 32>", reply["key 2"].inspect());
-                                return done(err);
-                            });
-                        });
-
-                        it('returns buffer values when executed in transaction', function (done) {
-                            client.multi().hgetall(new Buffer("hash key 2")).exec(function (err, reply) {
-                                assert.strictEqual(1, reply.length);
-                                assert.strictEqual("object", typeof reply);
-                                assert.strictEqual(2, Object.keys(reply[0]).length);
-                                assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 1"]));
-                                assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 2"]));
-                                assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0]["key 1"].inspect());
-                                assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[0]["key 2"].inspect());
-                                return done(err);
-                            });
-                        });
-                    });
-                });
-            });
-
             describe('unref', function () {
                 it('exits subprocess as soon as final command is processed', function (done) {
                     var args = config.HOST[ip] ? [config.HOST[ip], config.PORT] : [ip];