Skip to content

Commit 005e869

Browse files
author
Ruben Bridgewater
committed
Remove send_command safety checks
This checks are only important for users who use send_command directly instead of using the convience method. As the readme clearly stats how send_command should work and any user would have run into errors if misused, these checks can be removed. If any user might misuse the function anyway, it is very likely that another error will be thrown because of that Fix #629 and insert tests
1 parent e24f056 commit 005e869

File tree

2 files changed

+56
-29
lines changed

2 files changed

+56
-29
lines changed

index.js

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -661,37 +661,26 @@ function Command(command, args, sub_command, buffer_args, callback) {
661661
}
662662

663663
RedisClient.prototype.send_command = function (command, args, callback) {
664-
var arg, command_obj, i, elem_count, buffer_args, stream = this.stream, command_str = "", buffered_writes = 0, last_arg_type;
665-
666-
if (typeof command !== "string") {
667-
throw new Error("First argument to send_command must be the command name string, not " + typeof command);
668-
}
669-
670-
if (Array.isArray(args)) {
671-
if (typeof callback === "function") {
672-
// probably the fastest way:
673-
// client.command([arg1, arg2], cb); (straight passthrough)
674-
// send_command(command, [arg1, arg2], cb);
675-
} else if (!callback) {
676-
// most people find this variable argument length form more convenient, but it uses arguments, which is slower
677-
// client.command(arg1, arg2, cb); (wraps up arguments into an array)
678-
// send_command(command, [arg1, arg2, cb]);
679-
// client.command(arg1, arg2); (callback is optional)
680-
// send_command(command, [arg1, arg2]);
681-
// client.command(arg1, arg2, undefined); (callback is undefined)
682-
// send_command(command, [arg1, arg2, undefined]);
683-
last_arg_type = typeof args[args.length - 1];
684-
if (last_arg_type === "function" || last_arg_type === "undefined") {
685-
callback = args.pop();
686-
}
687-
} else {
688-
throw new Error("send_command: last argument must be a callback or undefined");
689-
}
690-
} else {
691-
throw new Error("send_command: second argument must be an array");
664+
var arg, command_obj, i, elem_count, buffer_args, stream = this.stream, command_str = "", buffered_writes = 0;
665+
666+
// if (typeof callback === "function") {}
667+
// probably the fastest way:
668+
// client.command([arg1, arg2], cb); (straight passthrough)
669+
// send_command(command, [arg1, arg2], cb);
670+
if (args === undefined) {
671+
args = [];
672+
} else if (!callback && typeof args[args.length - 1] === "function") {
673+
// most people find this variable argument length form more convenient, but it uses arguments, which is slower
674+
// client.command(arg1, arg2, cb); (wraps up arguments into an array)
675+
// send_command(command, [arg1, arg2, cb]);
676+
// client.command(arg1, arg2); (callback is optional)
677+
// send_command(command, [arg1, arg2]);
678+
// client.command(arg1, arg2, undefined); (callback is undefined)
679+
// send_command(command, [arg1, arg2, undefined]);
680+
callback = args.pop();
692681
}
693682

694-
if (callback && process.domain) {
683+
if (process.domain && callback) {
695684
callback = process.domain.bind(callback);
696685
}
697686

test/node_redis.spec.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,44 @@ describe("The node_redis client", function () {
117117
client.end();
118118
});
119119

120+
describe("send_command", function () {
121+
122+
it("omitting args should be fine in some cases", function (done) {
123+
client.send_command("info", undefined, function(err, res) {
124+
assert(/redis_version/.test(res));
125+
done();
126+
});
127+
});
128+
129+
it("using another type as cb should just work as if there were no callback parameter", function (done) {
130+
client.send_command('set', ['test', 'bla'], [true]);
131+
client.get('test', function(err, res) {
132+
assert.equal(res, 'bla');
133+
done();
134+
});
135+
});
136+
137+
it("misusing the function should eventually throw (no command)", function (done) {
138+
var mochaListener = helper.removeMochaListener();
139+
140+
process.once('uncaughtException', function (err) {
141+
process.on('uncaughtException', mochaListener);
142+
assert(/is not a function|toUpperCase/.test(err));
143+
done();
144+
});
145+
146+
client.send_command(true, 'info');
147+
});
148+
149+
it("misusing the function should eventually throw (wrong args)", function (done) {
150+
client.send_command('info', false, function(err, res) {
151+
assert.equal(err.message, 'ERR Protocol error: invalid multibulk length');
152+
done();
153+
});
154+
});
155+
156+
});
157+
120158
describe("when redis closes unexpectedly", function () {
121159
it("reconnects and can retrieve the pre-existing data", function (done) {
122160
client.on("reconnecting", function on_recon(params) {

0 commit comments

Comments
 (0)