Skip to content

Commit a31a4e2

Browse files
bcoebcoe
committed
Merge pull request #819 from NodeRedis/815-tweaks
minor tweaks to how we spawn tests in #815
2 parents 75b3fdb + d976bbc commit a31a4e2

File tree

14 files changed

+180
-98
lines changed

14 files changed

+180
-98
lines changed

‎index.js

Lines changed: 51 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,17 @@ try {
2929
require("./lib/parser/hiredis");
3030
parsers.push(require("./lib/parser/hiredis"));
3131
} catch (err) {
32+
/* istanbul ignore next: won't be reached with tests */
3233
debug("hiredis parser not installed.");
3334
}
3435

3536
parsers.push(require("./lib/parser/javascript"));
3637

3738
function RedisClient(stream, options) {
39+
options = options || {};
40+
3841
this.stream = stream;
39-
this.options = options = options || {};
42+
this.options = options;
4043

4144
this.connection_id = ++connection_id;
4245
this.connected = false;
@@ -51,26 +54,23 @@ function RedisClient(stream, options) {
5154
this.should_buffer = false;
5255
this.command_queue_high_water = this.options.command_queue_high_water || 1000;
5356
this.command_queue_low_water = this.options.command_queue_low_water || 0;
54-
this.max_attempts = null;
55-
if (options.max_attempts && !isNaN(options.max_attempts) && options.max_attempts > 0) {
57+
if (options.max_attempts && options.max_attempts > 0) {
5658
this.max_attempts = +options.max_attempts;
5759
}
5860
this.command_queue = new Queue(); // holds sent commands to de-pipeline them
5961
this.offline_queue = new Queue(); // holds commands issued but not able to be sent
6062
this.commands_sent = 0;
61-
this.connect_timeout = false;
62-
if (options.connect_timeout && !isNaN(options.connect_timeout) && options.connect_timeout > 0) {
63+
if (options.connect_timeout && options.connect_timeout > 0) {
6364
this.connect_timeout = +options.connect_timeout;
6465
}
6566
this.enable_offline_queue = true;
66-
if (typeof this.options.enable_offline_queue === "boolean") {
67-
this.enable_offline_queue = this.options.enable_offline_queue;
67+
if (this.options.enable_offline_queue === false) {
68+
this.enable_offline_queue = false;
6869
}
6970
this.retry_max_delay = null;
70-
if (options.retry_max_delay !== undefined && !isNaN(options.retry_max_delay) && options.retry_max_delay > 0) {
71-
this.retry_max_delay = options.retry_max_delay;
71+
if (options.retry_max_delay && options.retry_max_delay > 0) {
72+
this.retry_max_delay = +options.retry_max_delay;
7273
}
73-
7474
this.initialize_retry_vars();
7575
this.pub_sub_mode = false;
7676
this.subscription_set = {};
@@ -131,12 +131,10 @@ RedisClient.prototype.initialize_retry_vars = function () {
131131
};
132132

133133
RedisClient.prototype.unref = function () {
134-
debug("User requesting to unref the connection");
135134
if (this.connected) {
136135
debug("unref'ing the socket connection");
137136
this.stream.unref();
138-
}
139-
else {
137+
} else {
140138
debug("Not connected yet, will unref later");
141139
this.once("connect", function () {
142140
this.unref();
@@ -210,23 +208,26 @@ RedisClient.prototype.do_auth = function () {
210208
self.send_anyway = true;
211209
self.send_command("auth", [this.auth_pass], function (err, res) {
212210
if (err) {
211+
/* istanbul ignore if: this is almost impossible to test */
213212
if (loading.test(err.message)) {
214213
// if redis is still loading the db, it will not authenticate and everything else will fail
215-
console.log("Redis still loading, trying to authenticate later");
214+
debug("Redis still loading, trying to authenticate later");
216215
setTimeout(function () {
217216
self.do_auth();
218217
}, 2000); // TODO - magic number alert
219218
return;
220219
} else if (noPasswordIsSet.test(err.message)) {
221-
console.log("Warning: Redis server does not require a password, but a password was supplied.");
220+
debug("Warning: Redis server does not require a password, but a password was supplied.");
222221
err = null;
223222
res = "OK";
224223
} else {
225224
return self.emit("error", new Error("Auth error: " + err.message));
226225
}
227226
}
228-
if (res.toString() !== "OK") {
229-
return self.emit("error", new Error("Auth failed: " + res.toString()));
227+
228+
res = res.toString();
229+
if (res !== "OK") {
230+
return self.emit("error", new Error("Auth failed: " + res));
230231
}
231232

232233
debug("Auth succeeded " + self.address + " id " + self.connection_id);
@@ -283,7 +284,7 @@ RedisClient.prototype.init_parser = function () {
283284
var self = this;
284285

285286
if (this.options.parser) {
286-
if (! parsers.some(function (parser) {
287+
if (!parsers.some(function (parser) {
287288
if (parser.name === self.options.parser) {
288289
self.parser_module = parser;
289290
debug("Using parser module: " + self.parser_module.name);
@@ -353,7 +354,9 @@ RedisClient.prototype.on_ready = function () {
353354
self.send_command(parts[0] + "scribe", [parts[1]], callback);
354355
});
355356
return;
356-
} else if (this.monitoring) {
357+
}
358+
359+
if (this.monitoring) {
357360
this.send_command("monitor", []);
358361
} else {
359362
this.send_offline_queue();
@@ -378,7 +381,7 @@ RedisClient.prototype.on_info_cmd = function (err, res) {
378381
});
379382

380383
obj.versions = [];
381-
if( obj.redis_version ){
384+
if (obj.redis_version) {
382385
obj.redis_version.split('.').forEach(function (num) {
383386
obj.versions.push(+num);
384387
});
@@ -483,7 +486,7 @@ RedisClient.prototype.connection_gone = function (why) {
483486
this.retry_timer = null;
484487
// TODO - some people need a "Redis is Broken mode" for future commands that errors immediately, and others
485488
// want the program to exit. Right now, we just log, which doesn't really help in either case.
486-
console.error("node_redis: Couldn't get Redis connection after " + this.max_attempts + " attempts.");
489+
debug("node_redis: Couldn't get Redis connection after " + this.max_attempts + " attempts.");
487490
return;
488491
}
489492

@@ -500,7 +503,7 @@ RedisClient.prototype.connection_gone = function (why) {
500503
if (self.connect_timeout && self.retry_totaltime >= self.connect_timeout) {
501504
self.retry_timer = null;
502505
// TODO - engage Redis is Broken mode for future commands, or whatever
503-
console.error("node_redis: Couldn't get Redis connection after " + self.retry_totaltime + "ms.");
506+
debug("node_redis: Couldn't get Redis connection after " + self.retry_totaltime + "ms.");
504507
return;
505508
}
506509

@@ -525,7 +528,7 @@ RedisClient.prototype.on_data = function (data) {
525528
};
526529

527530
RedisClient.prototype.return_error = function (err) {
528-
var command_obj = this.command_queue.shift(), queue_len = this.command_queue.getLength();
531+
var command_obj = this.command_queue.shift(), queue_len = this.command_queue.length;
529532

530533
if (this.pub_sub_mode === false && queue_len === 0) {
531534
this.command_queue = new Queue();
@@ -536,20 +539,12 @@ RedisClient.prototype.return_error = function (err) {
536539
this.should_buffer = false;
537540
}
538541

539-
if (command_obj && typeof command_obj.callback === "function") {
540-
try {
541-
command_obj.callback(err);
542-
} catch (callback_err) {
543-
// if a callback throws an exception, re-throw it on a new stack so the parser can keep going
544-
process.nextTick(function () {
545-
throw callback_err;
546-
});
547-
}
548-
} else {
549-
console.log("node_redis: no callback to send error: " + err.message);
550-
// this will probably not make it anywhere useful, but we might as well throw
542+
try {
543+
command_obj.callback(err);
544+
} catch (callback_err) {
545+
// if a callback throws an exception, re-throw it on a new stack so the parser can keep going
551546
process.nextTick(function () {
552-
throw err;
547+
throw callback_err;
553548
});
554549
}
555550
};
@@ -628,7 +623,7 @@ RedisClient.prototype.return_reply = function (reply) {
628623
command_obj = this.command_queue.shift();
629624
}
630625

631-
queue_len = this.command_queue.getLength();
626+
queue_len = this.command_queue.length;
632627

633628
if (this.pub_sub_mode === false && queue_len === 0) {
634629
this.command_queue = new Queue(); // explicitly reclaim storage from old Queue
@@ -720,7 +715,7 @@ RedisClient.prototype.send_command = function (command, args, callback) {
720715
// probably the fastest way:
721716
// client.command([arg1, arg2], cb); (straight passthrough)
722717
// send_command(command, [arg1, arg2], cb);
723-
} else if (! callback) {
718+
} else if (!callback) {
724719
// most people find this variable argument length form more convenient, but it uses arguments, which is slower
725720
// client.command(arg1, arg2, cb); (wraps up arguments into an array)
726721
// send_command(command, [arg1, arg2, cb]);
@@ -739,7 +734,9 @@ RedisClient.prototype.send_command = function (command, args, callback) {
739734
throw new Error("send_command: second argument must be an array");
740735
}
741736

742-
if (callback && process.domain) callback = process.domain.bind(callback);
737+
if (callback && process.domain) {
738+
callback = process.domain.bind(callback);
739+
}
743740

744741
// if the last argument is an array and command is sadd or srem, expand it out:
745742
// client.sadd(arg1, [arg2, arg3, arg4], cb);
@@ -844,7 +841,7 @@ RedisClient.prototype.send_command = function (command, args, callback) {
844841
}
845842
}
846843
debug("send_command buffered_writes: " + buffered_writes, " should_buffer: " + this.should_buffer);
847-
if (buffered_writes || this.command_queue.getLength() >= this.command_queue_high_water) {
844+
if (buffered_writes || this.command_queue.length >= this.command_queue_high_water) {
848845
this.should_buffer = true;
849846
}
850847
return !this.should_buffer;
@@ -1141,9 +1138,7 @@ Multi.prototype.EXEC = Multi.prototype.exec;
11411138
RedisClient.prototype.multi = function (args) {
11421139
return new Multi(this, args);
11431140
};
1144-
RedisClient.prototype.MULTI = function (args) {
1145-
return new Multi(this, args);
1146-
};
1141+
RedisClient.prototype.MULTI = RedisClient.prototype.multi;
11471142

11481143

11491144
// stash original eval method
@@ -1177,26 +1172,26 @@ RedisClient.prototype.eval = RedisClient.prototype.EVAL = function () {
11771172
});
11781173
};
11791174

1180-
exports.createClient = function(arg0, arg1, options) {
1181-
if (typeof arg0 === 'object' || arg0 === undefined) {
1182-
options = arg0 || options;
1175+
exports.createClient = function(port_arg, host_arg, options) {
1176+
if (typeof port_arg === 'object' || port_arg === undefined) {
1177+
options = port_arg || options;
11831178
return createClient_tcp(default_port, default_host, options);
11841179
}
1185-
if (typeof arg0 === 'number' || typeof arg0 === 'string' && arg0.match(/^\d+$/)){
1186-
return createClient_tcp(arg0, arg1, options);
1180+
if (typeof port_arg === 'number' || typeof port_arg === 'string' && /^\d+$/.test(port_arg)) {
1181+
return createClient_tcp(port_arg, host_arg, options);
11871182
}
1188-
if (typeof arg0 === 'string') {
1189-
options = arg1 || {};
1183+
if (typeof port_arg === 'string') {
1184+
options = host_arg || {};
11901185

1191-
var parsed = URL.parse(arg0, true, true);
1186+
var parsed = URL.parse(port_arg, true, true);
11921187
if (parsed.hostname) {
11931188
if (parsed.auth) {
11941189
options.auth_pass = parsed.auth.split(':')[1];
11951190
}
1196-
return createClient_tcp((parsed.port || default_port), parsed.hostname, options);
1191+
return createClient_tcp(parsed.port, parsed.hostname, options);
11971192
}
11981193

1199-
return createClient_unix(arg0, options);
1194+
return createClient_unix(port_arg, options);
12001195
}
12011196
throw new Error('unknown type of connection in createClient()');
12021197
};
@@ -1206,7 +1201,7 @@ var createClient_unix = function(path, options){
12061201
path: path
12071202
};
12081203
var net_client = net.createConnection(cnxOptions);
1209-
var redis_client = new RedisClient(net_client, options || {});
1204+
var redis_client = new RedisClient(net_client, options);
12101205

12111206
redis_client.connectionOption = cnxOptions;
12121207
redis_client.address = path;
@@ -1218,10 +1213,10 @@ var createClient_tcp = function (port_arg, host_arg, options) {
12181213
var cnxOptions = {
12191214
'port' : port_arg || default_port,
12201215
'host' : host_arg || default_host,
1221-
'family' : (options && options.family === 'IPv6') ? 6 : 4
1216+
'family' : options && options.family === 'IPv6' ? 6 : 4
12221217
};
12231218
var net_client = net.createConnection(cnxOptions);
1224-
var redis_client = new RedisClient(net_client, options || {});
1219+
var redis_client = new RedisClient(net_client, options);
12251220

12261221
redis_client.connectionOption = cnxOptions;
12271222
redis_client.address = cnxOptions.host + ':' + cnxOptions.port;

‎lib/parser/hiredis.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ exports.name = "hiredis";
88

99
function HiredisReplyParser(options) {
1010
this.name = exports.name;
11-
this.options = options || {};
11+
this.options = options;
1212
this.reset();
1313
events.EventEmitter.call(this);
1414
}
@@ -27,12 +27,7 @@ HiredisReplyParser.prototype.execute = function (data) {
2727
var reply;
2828
this.reader.feed(data);
2929
while (true) {
30-
try {
31-
reply = this.reader.get();
32-
} catch (err) {
33-
this.emit("error", err);
34-
break;
35-
}
30+
reply = this.reader.get();
3631

3732
if (reply === undefined) {
3833
break;

‎lib/parser/javascript.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ exports.name = "javascript";
1212

1313
function ReplyParser(options) {
1414
this.name = exports.name;
15-
this.options = options || { };
15+
this.options = options;
1616

1717
this._buffer = null;
1818
this._offset = 0;

‎test/commands/hmget.spec.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ describe("The 'hmget' method", function () {
3737
});
3838
});
3939

40+
it('allows keys to be specified by passing an array as first argument', function (done) {
41+
client.HMGET([hash, "0123456789", "some manner of key"], function (err, reply) {
42+
assert.strictEqual("abcdefghij", reply[0].toString());
43+
assert.strictEqual("a type of value", reply[1].toString());
44+
return done(err);
45+
});
46+
});
47+
4048
it('allows a single key to be specified in an array', function (done) {
4149
client.HMGET(hash, ["0123456789"], function (err, reply) {
4250
assert.strictEqual("abcdefghij", reply[0].toString());

‎test/commands/hmset.spec.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe("The 'hmset' method", function () {
2020
});
2121

2222
it('handles redis-style syntax', function (done) {
23-
client.HMSET(hash, "0123456789", "abcdefghij", "some manner of key", "a type of value", helper.isString('OK'));
23+
client.HMSET(hash, "0123456789", "abcdefghij", "some manner of key", "a type of value", "otherTypes", 555, helper.isString('OK'));
2424
client.HGETALL(hash, function (err, obj) {
2525
assert.equal(obj['0123456789'], 'abcdefghij');
2626
assert.equal(obj['some manner of key'], 'a type of value');
@@ -29,14 +29,23 @@ describe("The 'hmset' method", function () {
2929
});
3030

3131
it('handles object-style syntax', function (done) {
32-
client.HMSET(hash, {"0123456789": "abcdefghij", "some manner of key": "a type of value"}, helper.isString('OK'));
32+
client.HMSET(hash, {"0123456789": "abcdefghij", "some manner of key": "a type of value", "otherTypes": 555}, helper.isString('OK'));
3333
client.HGETALL(hash, function (err, obj) {
3434
assert.equal(obj['0123456789'], 'abcdefghij');
3535
assert.equal(obj['some manner of key'], 'a type of value');
3636
return done(err);
3737
})
3838
});
3939

40+
it('handles object-style syntax and the key being a number', function (done) {
41+
client.HMSET(231232, {"0123456789": "abcdefghij", "some manner of key": "a type of value", "otherTypes": 555}, helper.isString('OK'));
42+
client.HGETALL(231232, function (err, obj) {
43+
assert.equal(obj['0123456789'], 'abcdefghij');
44+
assert.equal(obj['some manner of key'], 'a type of value');
45+
return done(err);
46+
});
47+
});
48+
4049
it('allows a numeric key', function (done) {
4150
client.HMSET(hash, 99, 'banana', helper.isString('OK'));
4251
client.HGETALL(hash, function (err, obj) {

‎test/commands/mset.spec.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,6 @@ var uuid = require('uuid');
77

88
describe("The 'mset' method", function () {
99

10-
function removeMochaListener () {
11-
var mochaListener = process.listeners('uncaughtException').pop();
12-
process.removeListener('uncaughtException', mochaListener);
13-
return mochaListener;
14-
}
15-
1610
helper.allTests(function(parser, ip, args) {
1711

1812
describe("using " + parser + " and " + ip, function () {
@@ -102,7 +96,7 @@ describe("The 'mset' method", function () {
10296
describe("with undefined 'key' and missing 'value' parameter", function () {
10397
// this behavior is different from the 'set' behavior.
10498
it("throws an error", function (done) {
105-
var mochaListener = removeMochaListener();
99+
var mochaListener = helper.removeMochaListener();
106100

107101
process.once('uncaughtException', function (err) {
108102
process.on('uncaughtException', mochaListener);
@@ -116,7 +110,7 @@ describe("The 'mset' method", function () {
116110

117111
describe("with undefined 'key' and defined 'value' parameters", function () {
118112
it("throws an error", function () {
119-
var mochaListener = removeMochaListener();
113+
var mochaListener = helper.removeMochaListener();
120114

121115
process.once('uncaughtException', function (err) {
122116
process.on('uncaughtException', mochaListener);

0 commit comments

Comments
 (0)