Skip to content

Commit 2ca4241

Browse files
author
Ruben Bridgewater
committed
Fix explicitly passing undefined as callback
1 parent 977d4db commit 2ca4241

File tree

7 files changed

+240
-218
lines changed

7 files changed

+240
-218
lines changed

‎changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Bugfixes:
1414
- Fix multi.hmset key not being type converted if used with an object and key not being a string (@BridgeAR)
1515
- Fix parser errors not being catched properly (@BridgeAR)
1616
- Fix a crash that could occur if a redis server does return the info command as usual #541 (@BridgeAR)
17+
- Explicitly passing undefined as a callback statement will work again. E.g. client.publish('channel', 'message', undefined); (@BridgeAR)
1718

1819
## v2.0.1 - Sep 24, 2015
1920

‎index.js

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,7 @@ RedisClient.prototype.return_reply = function (reply) {
629629
return elem.replace(/\\"/g, '"');
630630
});
631631
this.emit("monitor", timestamp, args);
632+
/* istanbul ignore else: this is a safety check that we should not be able to trigger */
632633
} else {
633634
var err = new Error("node_redis command queue state error. If you can reproduce this, please report it.");
634635
err.command_obj = command_obj;
@@ -639,21 +640,14 @@ RedisClient.prototype.return_reply = function (reply) {
639640
RedisClient.prototype.send_command = function (command, args, callback) {
640641
var arg, command_obj, i, elem_count, buffer_args, stream = this.stream, command_str = "", buffered_writes = 0, err;
641642

642-
// if (typeof callback === "function") {}
643-
// probably the fastest way:
644-
// client.command([arg1, arg2], cb); (straight passthrough)
645-
// send_command(command, [arg1, arg2], cb);
646643
if (args === undefined) {
647644
args = [];
648-
} else if (!callback && typeof args[args.length - 1] === "function") {
649-
// most people find this variable argument length form more convenient, but it uses arguments, which is slower
650-
// client.command(arg1, arg2, cb); (wraps up arguments into an array)
651-
// send_command(command, [arg1, arg2, cb]);
652-
// client.command(arg1, arg2); (callback is optional)
653-
// send_command(command, [arg1, arg2]);
654-
// client.command(arg1, arg2, undefined); (callback is undefined)
655-
// send_command(command, [arg1, arg2, undefined]);
656-
callback = args.pop();
645+
} else if (!callback) {
646+
if (typeof args[args.length - 1] === "function") {
647+
callback = args.pop();
648+
} else if (typeof args[args.length - 1] === "undefined") {
649+
args.pop();
650+
}
657651
}
658652

659653
if (process.domain && callback) {

‎test/commands/geoadd.spec.js.future renamed to ‎test/commands/geoadd.spec.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ var config = require("../lib/config");
44
var helper = require("../helper");
55
var redis = config.redis;
66

7-
describe("The 'getoadd' method", function () {
7+
describe("The 'geoadd' method", function () {
88

99
helper.allTests(function(parser, ip, args) {
1010

@@ -19,6 +19,7 @@ describe("The 'getoadd' method", function () {
1919
});
2020

2121
it('returns 1 if the key exists', function (done) {
22+
helper.serverVersionAtLeast.call(this, client, [3, 2, 0]);
2223
client.geoadd("mycity:21:0:location", "13.361389","38.115556","COR", function(err, res) {
2324
console.log(err, res);
2425
// geoadd is still in the unstable branch. As soon as it reaches the stable one, activate this test

‎test/commands/set.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ describe("The 'set' method", function () {
8989
}, 100);
9090
});
9191

92+
it("sets the value correctly even if the callback is explicitly set to undefined", function (done) {
93+
client.set(key, value, undefined);
94+
setTimeout(function () {
95+
client.get(key, function (err, res) {
96+
helper.isString(value)(err, res);
97+
done();
98+
});
99+
}, 100);
100+
});
101+
92102
it("sets the value correctly with the array syntax", function (done) {
93103
client.set([key, value]);
94104
setTimeout(function () {

‎test/commands/zadd.spec.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@ describe("The 'zadd' method", function () {
2020
});
2121

2222
it('reports an error', function (done) {
23+
if (helper.redisProcess().spawnFailed()) this.skip();
2324
client.zadd('infinity', [+'5t', "should not be possible"], helper.isError(done));
2425
});
2526

2627
it('return inf / -inf', function (done) {
28+
if (helper.redisProcess().spawnFailed()) this.skip();
29+
helper.serverVersionAtLeast.call(this, client, [3, 0, 2]);
2730
client.zadd('infinity', [+Infinity, "should be inf"], helper.isNumber(1));
28-
client.zadd('infinity', [-Infinity, "should be negative inf"], helper.isNumber(1));
31+
client.zadd('infinity', ['inf', 'should be also be inf'], helper.isNumber(1));
32+
client.zadd('infinity', -Infinity, "should be negative inf", helper.isNumber(1));
2933
client.zadd('infinity', [99999999999999999999999, "should not be inf"], helper.isNumber(1));
3034
client.zrange('infinity', 0, -1, 'WITHSCORES', function (err, res) {
3135
assert.equal(res[5], 'inf');

‎test/detect_buffers.spec.js

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
'use strict';
2+
3+
var assert = require("assert");
4+
var config = require("./lib/config");
5+
var helper = require('./helper');
6+
var redis = config.redis;
7+
8+
describe("detect_buffers", function () {
9+
10+
helper.allTests(function(parser, ip, args) {
11+
12+
describe("using " + parser + " and " + ip, function () {
13+
var client;
14+
var args = config.configureClient(parser, ip, {
15+
detect_buffers: true
16+
});
17+
18+
beforeEach(function (done) {
19+
client = redis.createClient.apply(redis.createClient, args);
20+
client.once("error", done);
21+
client.once("connect", function () {
22+
client.flushdb(function (err) {
23+
client.hmset("hash key 2", "key 1", "val 1", "key 2", "val 2");
24+
client.set("string key 1", "string value");
25+
return done(err);
26+
});
27+
});
28+
});
29+
30+
describe('get', function () {
31+
describe('first argument is a string', function () {
32+
it('returns a string', function (done) {
33+
client.get("string key 1", helper.isString("string value", done));
34+
});
35+
36+
it('returns a string when executed as part of transaction', function (done) {
37+
client.multi().get("string key 1").exec(helper.isString("string value", done));
38+
});
39+
});
40+
41+
describe('first argument is a buffer', function () {
42+
it('returns a buffer', function (done) {
43+
client.get(new Buffer("string key 1"), function (err, reply) {
44+
assert.strictEqual(true, Buffer.isBuffer(reply));
45+
assert.strictEqual("<Buffer 73 74 72 69 6e 67 20 76 61 6c 75 65>", reply.inspect());
46+
return done(err);
47+
});
48+
});
49+
50+
it('returns a bufffer when executed as part of transaction', function (done) {
51+
client.multi().get(new Buffer("string key 1")).exec(function (err, reply) {
52+
assert.strictEqual(1, reply.length);
53+
assert.strictEqual(true, Buffer.isBuffer(reply[0]));
54+
assert.strictEqual("<Buffer 73 74 72 69 6e 67 20 76 61 6c 75 65>", reply[0].inspect());
55+
return done(err);
56+
});
57+
});
58+
});
59+
});
60+
61+
describe('multi.hget', function () {
62+
it('can interleave string and buffer results', function (done) {
63+
client.multi()
64+
.hget("hash key 2", "key 1")
65+
.hget(new Buffer("hash key 2"), "key 1")
66+
.hget("hash key 2", new Buffer("key 2"))
67+
.hget("hash key 2", "key 2")
68+
.exec(function (err, reply) {
69+
assert.strictEqual(true, Array.isArray(reply));
70+
assert.strictEqual(4, reply.length);
71+
assert.strictEqual("val 1", reply[0]);
72+
assert.strictEqual(true, Buffer.isBuffer(reply[1]));
73+
assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[1].inspect());
74+
assert.strictEqual(true, Buffer.isBuffer(reply[2]));
75+
assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[2].inspect());
76+
assert.strictEqual("val 2", reply[3]);
77+
return done(err);
78+
});
79+
});
80+
});
81+
82+
describe('hmget', function () {
83+
describe('first argument is a string', function () {
84+
it('returns strings for keys requested', function (done) {
85+
client.hmget("hash key 2", "key 1", "key 2", function (err, reply) {
86+
assert.strictEqual(true, Array.isArray(reply));
87+
assert.strictEqual(2, reply.length);
88+
assert.strictEqual("val 1", reply[0]);
89+
assert.strictEqual("val 2", reply[1]);
90+
return done(err);
91+
});
92+
});
93+
94+
it('returns strings for keys requested in transaction', function (done) {
95+
client.multi().hmget("hash key 2", "key 1", "key 2").exec(function (err, reply) {
96+
assert.strictEqual(true, Array.isArray(reply));
97+
assert.strictEqual(1, reply.length);
98+
assert.strictEqual(2, reply[0].length);
99+
assert.strictEqual("val 1", reply[0][0]);
100+
assert.strictEqual("val 2", reply[0][1]);
101+
return done(err);
102+
});
103+
});
104+
105+
it('handles array of strings with undefined values (repro #344)', function (done) {
106+
client.hmget("hash key 2", "key 3", "key 4", function(err, reply) {
107+
assert.strictEqual(true, Array.isArray(reply));
108+
assert.strictEqual(2, reply.length);
109+
assert.equal(null, reply[0]);
110+
assert.equal(null, reply[1]);
111+
return done(err);
112+
});
113+
});
114+
115+
it('handles array of strings with undefined values in transaction (repro #344)', function (done) {
116+
client.multi().hmget("hash key 2", "key 3", "key 4").exec(function(err, reply) {
117+
assert.strictEqual(true, Array.isArray(reply));
118+
assert.strictEqual(1, reply.length);
119+
assert.strictEqual(2, reply[0].length);
120+
assert.equal(null, reply[0][0]);
121+
assert.equal(null, reply[0][1]);
122+
return done(err);
123+
});
124+
});
125+
});
126+
127+
describe('first argument is a buffer', function () {
128+
it('returns buffers for keys requested', function (done) {
129+
client.hmget(new Buffer("hash key 2"), "key 1", "key 2", function (err, reply) {
130+
assert.strictEqual(true, Array.isArray(reply));
131+
assert.strictEqual(2, reply.length);
132+
assert.strictEqual(true, Buffer.isBuffer(reply[0]));
133+
assert.strictEqual(true, Buffer.isBuffer(reply[1]));
134+
assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0].inspect());
135+
assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[1].inspect());
136+
return done(err);
137+
});
138+
});
139+
140+
it("returns buffers for keys requested in transaction", function (done) {
141+
client.multi().hmget(new Buffer("hash key 2"), "key 1", "key 2").exec(function (err, reply) {
142+
assert.strictEqual(true, Array.isArray(reply));
143+
assert.strictEqual(1, reply.length);
144+
assert.strictEqual(2, reply[0].length);
145+
assert.strictEqual(true, Buffer.isBuffer(reply[0][0]));
146+
assert.strictEqual(true, Buffer.isBuffer(reply[0][1]));
147+
assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0][0].inspect());
148+
assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[0][1].inspect());
149+
return done(err);
150+
});
151+
});
152+
});
153+
});
154+
155+
describe('hgetall', function (done) {
156+
describe('first argument is a string', function () {
157+
it('returns string values', function (done) {
158+
client.hgetall("hash key 2", function (err, reply) {
159+
assert.strictEqual("object", typeof reply);
160+
assert.strictEqual(2, Object.keys(reply).length);
161+
assert.strictEqual("val 1", reply["key 1"]);
162+
assert.strictEqual("val 2", reply["key 2"]);
163+
return done(err);
164+
});
165+
});
166+
167+
it('returns string values when executed in transaction', function (done) {
168+
client.multi().hgetall("hash key 2").exec(function (err, reply) {
169+
assert.strictEqual(1, reply.length);
170+
assert.strictEqual("object", typeof reply[0]);
171+
assert.strictEqual(2, Object.keys(reply[0]).length);
172+
assert.strictEqual("val 1", reply[0]["key 1"]);
173+
assert.strictEqual("val 2", reply[0]["key 2"]);
174+
return done(err);
175+
});
176+
});
177+
});
178+
179+
describe('first argument is a buffer', function () {
180+
it('returns buffer values', function (done) {
181+
client.hgetall(new Buffer("hash key 2"), function (err, reply) {
182+
assert.strictEqual(null, err);
183+
assert.strictEqual("object", typeof reply);
184+
assert.strictEqual(2, Object.keys(reply).length);
185+
assert.strictEqual(true, Buffer.isBuffer(reply["key 1"]));
186+
assert.strictEqual(true, Buffer.isBuffer(reply["key 2"]));
187+
assert.strictEqual("<Buffer 76 61 6c 20 31>", reply["key 1"].inspect());
188+
assert.strictEqual("<Buffer 76 61 6c 20 32>", reply["key 2"].inspect());
189+
return done(err);
190+
});
191+
});
192+
193+
it('returns buffer values when executed in transaction', function (done) {
194+
client.multi().hgetall(new Buffer("hash key 2")).exec(function (err, reply) {
195+
assert.strictEqual(1, reply.length);
196+
assert.strictEqual("object", typeof reply);
197+
assert.strictEqual(2, Object.keys(reply[0]).length);
198+
assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 1"]));
199+
assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 2"]));
200+
assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0]["key 1"].inspect());
201+
assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[0]["key 2"].inspect());
202+
return done(err);
203+
});
204+
});
205+
});
206+
});
207+
});
208+
});
209+
});

0 commit comments

Comments
 (0)