diff --git a/index.js b/index.js index 151b998220..ef53de50ce 100644 --- a/index.js +++ b/index.js @@ -29,14 +29,17 @@ try { require("./lib/parser/hiredis"); parsers.push(require("./lib/parser/hiredis")); } catch (err) { + /* istanbul ignore next: won't be reached with tests */ debug("hiredis parser not installed."); } parsers.push(require("./lib/parser/javascript")); function RedisClient(stream, options) { + options = options || {}; + this.stream = stream; - this.options = options = options || {}; + this.options = options; this.connection_id = ++connection_id; this.connected = false; @@ -51,26 +54,23 @@ function RedisClient(stream, options) { this.should_buffer = false; this.command_queue_high_water = this.options.command_queue_high_water || 1000; this.command_queue_low_water = this.options.command_queue_low_water || 0; - this.max_attempts = null; - if (options.max_attempts && !isNaN(options.max_attempts) && options.max_attempts > 0) { + if (options.max_attempts && options.max_attempts > 0) { this.max_attempts = +options.max_attempts; } this.command_queue = new Queue(); // holds sent commands to de-pipeline them this.offline_queue = new Queue(); // holds commands issued but not able to be sent this.commands_sent = 0; - this.connect_timeout = false; - if (options.connect_timeout && !isNaN(options.connect_timeout) && options.connect_timeout > 0) { + if (options.connect_timeout && options.connect_timeout > 0) { this.connect_timeout = +options.connect_timeout; } this.enable_offline_queue = true; - if (typeof this.options.enable_offline_queue === "boolean") { - this.enable_offline_queue = this.options.enable_offline_queue; + if (this.options.enable_offline_queue === false) { + this.enable_offline_queue = false; } this.retry_max_delay = null; - if (options.retry_max_delay !== undefined && !isNaN(options.retry_max_delay) && options.retry_max_delay > 0) { - this.retry_max_delay = options.retry_max_delay; + if (options.retry_max_delay && options.retry_max_delay > 0) { + this.retry_max_delay = +options.retry_max_delay; } - this.initialize_retry_vars(); this.pub_sub_mode = false; this.subscription_set = {}; @@ -131,12 +131,10 @@ RedisClient.prototype.initialize_retry_vars = function () { }; RedisClient.prototype.unref = function () { - debug("User requesting to unref the connection"); if (this.connected) { debug("unref'ing the socket connection"); this.stream.unref(); - } - else { + } else { debug("Not connected yet, will unref later"); this.once("connect", function () { this.unref(); @@ -210,23 +208,26 @@ RedisClient.prototype.do_auth = function () { self.send_anyway = true; self.send_command("auth", [this.auth_pass], function (err, res) { if (err) { + /* istanbul ignore if: this is almost impossible to test */ if (loading.test(err.message)) { // if redis is still loading the db, it will not authenticate and everything else will fail - console.log("Redis still loading, trying to authenticate later"); + debug("Redis still loading, trying to authenticate later"); setTimeout(function () { self.do_auth(); }, 2000); // TODO - magic number alert return; } else if (noPasswordIsSet.test(err.message)) { - console.log("Warning: Redis server does not require a password, but a password was supplied."); + debug("Warning: Redis server does not require a password, but a password was supplied."); err = null; res = "OK"; } else { return self.emit("error", new Error("Auth error: " + err.message)); } } - if (res.toString() !== "OK") { - return self.emit("error", new Error("Auth failed: " + res.toString())); + + res = res.toString(); + if (res !== "OK") { + return self.emit("error", new Error("Auth failed: " + res)); } debug("Auth succeeded " + self.address + " id " + self.connection_id); @@ -283,7 +284,7 @@ RedisClient.prototype.init_parser = function () { var self = this; if (this.options.parser) { - if (! parsers.some(function (parser) { + if (!parsers.some(function (parser) { if (parser.name === self.options.parser) { self.parser_module = parser; debug("Using parser module: " + self.parser_module.name); @@ -353,7 +354,9 @@ RedisClient.prototype.on_ready = function () { self.send_command(parts[0] + "scribe", [parts[1]], callback); }); return; - } else if (this.monitoring) { + } + + if (this.monitoring) { this.send_command("monitor", []); } else { this.send_offline_queue(); @@ -378,7 +381,7 @@ RedisClient.prototype.on_info_cmd = function (err, res) { }); obj.versions = []; - if( obj.redis_version ){ + if (obj.redis_version) { obj.redis_version.split('.').forEach(function (num) { obj.versions.push(+num); }); @@ -483,7 +486,7 @@ RedisClient.prototype.connection_gone = function (why) { this.retry_timer = null; // TODO - some people need a "Redis is Broken mode" for future commands that errors immediately, and others // want the program to exit. Right now, we just log, which doesn't really help in either case. - console.error("node_redis: Couldn't get Redis connection after " + this.max_attempts + " attempts."); + debug("node_redis: Couldn't get Redis connection after " + this.max_attempts + " attempts."); return; } @@ -500,7 +503,7 @@ RedisClient.prototype.connection_gone = function (why) { if (self.connect_timeout && self.retry_totaltime >= self.connect_timeout) { self.retry_timer = null; // TODO - engage Redis is Broken mode for future commands, or whatever - console.error("node_redis: Couldn't get Redis connection after " + self.retry_totaltime + "ms."); + debug("node_redis: Couldn't get Redis connection after " + self.retry_totaltime + "ms."); return; } @@ -525,7 +528,7 @@ RedisClient.prototype.on_data = function (data) { }; RedisClient.prototype.return_error = function (err) { - var command_obj = this.command_queue.shift(), queue_len = this.command_queue.getLength(); + var command_obj = this.command_queue.shift(), queue_len = this.command_queue.length; if (this.pub_sub_mode === false && queue_len === 0) { this.command_queue = new Queue(); @@ -536,20 +539,12 @@ RedisClient.prototype.return_error = function (err) { this.should_buffer = false; } - if (command_obj && typeof command_obj.callback === "function") { - try { - command_obj.callback(err); - } catch (callback_err) { - // if a callback throws an exception, re-throw it on a new stack so the parser can keep going - process.nextTick(function () { - throw callback_err; - }); - } - } else { - console.log("node_redis: no callback to send error: " + err.message); - // this will probably not make it anywhere useful, but we might as well throw + try { + command_obj.callback(err); + } catch (callback_err) { + // if a callback throws an exception, re-throw it on a new stack so the parser can keep going process.nextTick(function () { - throw err; + throw callback_err; }); } }; @@ -628,7 +623,7 @@ RedisClient.prototype.return_reply = function (reply) { command_obj = this.command_queue.shift(); } - queue_len = this.command_queue.getLength(); + queue_len = this.command_queue.length; if (this.pub_sub_mode === false && queue_len === 0) { this.command_queue = new Queue(); // explicitly reclaim storage from old Queue @@ -720,7 +715,7 @@ RedisClient.prototype.send_command = function (command, args, callback) { // probably the fastest way: // client.command([arg1, arg2], cb); (straight passthrough) // send_command(command, [arg1, arg2], cb); - } else if (! callback) { + } else if (!callback) { // 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]); @@ -739,7 +734,9 @@ RedisClient.prototype.send_command = function (command, args, callback) { throw new Error("send_command: second argument must be an array"); } - if (callback && process.domain) callback = process.domain.bind(callback); + if (callback && process.domain) { + callback = process.domain.bind(callback); + } // if the last argument is an array and command is sadd or srem, expand it out: // client.sadd(arg1, [arg2, arg3, arg4], cb); @@ -844,7 +841,7 @@ RedisClient.prototype.send_command = function (command, args, callback) { } } debug("send_command buffered_writes: " + buffered_writes, " should_buffer: " + this.should_buffer); - if (buffered_writes || this.command_queue.getLength() >= this.command_queue_high_water) { + if (buffered_writes || this.command_queue.length >= this.command_queue_high_water) { this.should_buffer = true; } return !this.should_buffer; @@ -1141,9 +1138,7 @@ Multi.prototype.EXEC = Multi.prototype.exec; RedisClient.prototype.multi = function (args) { return new Multi(this, args); }; -RedisClient.prototype.MULTI = function (args) { - return new Multi(this, args); -}; +RedisClient.prototype.MULTI = RedisClient.prototype.multi; // stash original eval method @@ -1177,26 +1172,26 @@ RedisClient.prototype.eval = RedisClient.prototype.EVAL = function () { }); }; -exports.createClient = function(arg0, arg1, options) { - if (typeof arg0 === 'object' || arg0 === undefined) { - options = arg0 || options; +exports.createClient = function(port_arg, host_arg, options) { + if (typeof port_arg === 'object' || port_arg === undefined) { + options = port_arg || options; return createClient_tcp(default_port, default_host, options); } - if (typeof arg0 === 'number' || typeof arg0 === 'string' && arg0.match(/^\d+$/)){ - return createClient_tcp(arg0, arg1, options); + if (typeof port_arg === 'number' || typeof port_arg === 'string' && /^\d+$/.test(port_arg)) { + return createClient_tcp(port_arg, host_arg, options); } - if (typeof arg0 === 'string') { - options = arg1 || {}; + if (typeof port_arg === 'string') { + options = host_arg || {}; - var parsed = URL.parse(arg0, true, true); + var parsed = URL.parse(port_arg, true, true); if (parsed.hostname) { if (parsed.auth) { options.auth_pass = parsed.auth.split(':')[1]; } - return createClient_tcp((parsed.port || default_port), parsed.hostname, options); + return createClient_tcp(parsed.port, parsed.hostname, options); } - return createClient_unix(arg0, options); + return createClient_unix(port_arg, options); } throw new Error('unknown type of connection in createClient()'); }; @@ -1206,7 +1201,7 @@ var createClient_unix = function(path, options){ path: path }; var net_client = net.createConnection(cnxOptions); - var redis_client = new RedisClient(net_client, options || {}); + var redis_client = new RedisClient(net_client, options); redis_client.connectionOption = cnxOptions; redis_client.address = path; @@ -1218,10 +1213,10 @@ var createClient_tcp = function (port_arg, host_arg, options) { var cnxOptions = { 'port' : port_arg || default_port, 'host' : host_arg || default_host, - 'family' : (options && options.family === 'IPv6') ? 6 : 4 + 'family' : options && options.family === 'IPv6' ? 6 : 4 }; var net_client = net.createConnection(cnxOptions); - var redis_client = new RedisClient(net_client, options || {}); + var redis_client = new RedisClient(net_client, options); redis_client.connectionOption = cnxOptions; redis_client.address = cnxOptions.host + ':' + cnxOptions.port; diff --git a/lib/parser/hiredis.js b/lib/parser/hiredis.js index d1fd0b7a9a..97ece3b08e 100644 --- a/lib/parser/hiredis.js +++ b/lib/parser/hiredis.js @@ -8,7 +8,7 @@ exports.name = "hiredis"; function HiredisReplyParser(options) { this.name = exports.name; - this.options = options || {}; + this.options = options; this.reset(); events.EventEmitter.call(this); } @@ -27,12 +27,7 @@ HiredisReplyParser.prototype.execute = function (data) { var reply; this.reader.feed(data); while (true) { - try { - reply = this.reader.get(); - } catch (err) { - this.emit("error", err); - break; - } + reply = this.reader.get(); if (reply === undefined) { break; diff --git a/lib/parser/javascript.js b/lib/parser/javascript.js index be5248d115..5772ad7990 100644 --- a/lib/parser/javascript.js +++ b/lib/parser/javascript.js @@ -12,7 +12,7 @@ exports.name = "javascript"; function ReplyParser(options) { this.name = exports.name; - this.options = options || { }; + this.options = options; this._buffer = null; this._offset = 0; diff --git a/test/commands/hmget.spec.js b/test/commands/hmget.spec.js index 3ad8fa9128..6ab175a914 100644 --- a/test/commands/hmget.spec.js +++ b/test/commands/hmget.spec.js @@ -37,6 +37,14 @@ describe("The 'hmget' method", function () { }); }); + it('allows keys to be specified by passing an array as first argument', function (done) { + client.HMGET([hash, "0123456789", "some manner of key"], function (err, reply) { + assert.strictEqual("abcdefghij", reply[0].toString()); + assert.strictEqual("a type of value", reply[1].toString()); + return done(err); + }); + }); + it('allows a single key to be specified in an array', function (done) { client.HMGET(hash, ["0123456789"], function (err, reply) { assert.strictEqual("abcdefghij", reply[0].toString()); diff --git a/test/commands/hmset.spec.js b/test/commands/hmset.spec.js index a4d77dfc01..88b9827286 100644 --- a/test/commands/hmset.spec.js +++ b/test/commands/hmset.spec.js @@ -20,7 +20,7 @@ describe("The 'hmset' method", function () { }); it('handles redis-style syntax', function (done) { - client.HMSET(hash, "0123456789", "abcdefghij", "some manner of key", "a type of value", helper.isString('OK')); + client.HMSET(hash, "0123456789", "abcdefghij", "some manner of key", "a type of value", "otherTypes", 555, helper.isString('OK')); client.HGETALL(hash, function (err, obj) { assert.equal(obj['0123456789'], 'abcdefghij'); assert.equal(obj['some manner of key'], 'a type of value'); @@ -29,7 +29,7 @@ describe("The 'hmset' method", function () { }); it('handles object-style syntax', function (done) { - client.HMSET(hash, {"0123456789": "abcdefghij", "some manner of key": "a type of value"}, helper.isString('OK')); + client.HMSET(hash, {"0123456789": "abcdefghij", "some manner of key": "a type of value", "otherTypes": 555}, helper.isString('OK')); client.HGETALL(hash, function (err, obj) { assert.equal(obj['0123456789'], 'abcdefghij'); assert.equal(obj['some manner of key'], 'a type of value'); @@ -37,6 +37,15 @@ describe("The 'hmset' method", function () { }) }); + it('handles object-style syntax and the key being a number', function (done) { + client.HMSET(231232, {"0123456789": "abcdefghij", "some manner of key": "a type of value", "otherTypes": 555}, helper.isString('OK')); + client.HGETALL(231232, function (err, obj) { + assert.equal(obj['0123456789'], 'abcdefghij'); + assert.equal(obj['some manner of key'], 'a type of value'); + return done(err); + }); + }); + it('allows a numeric key', function (done) { client.HMSET(hash, 99, 'banana', helper.isString('OK')); client.HGETALL(hash, function (err, obj) { diff --git a/test/commands/mset.spec.js b/test/commands/mset.spec.js index aacb969c69..45dab25462 100644 --- a/test/commands/mset.spec.js +++ b/test/commands/mset.spec.js @@ -7,12 +7,6 @@ var uuid = require('uuid'); describe("The 'mset' method", function () { - function removeMochaListener () { - var mochaListener = process.listeners('uncaughtException').pop(); - process.removeListener('uncaughtException', mochaListener); - return mochaListener; - } - helper.allTests(function(parser, ip, args) { describe("using " + parser + " and " + ip, function () { @@ -102,7 +96,7 @@ describe("The 'mset' method", function () { describe("with undefined 'key' and missing 'value' parameter", function () { // this behavior is different from the 'set' behavior. it("throws an error", function (done) { - var mochaListener = removeMochaListener(); + var mochaListener = helper.removeMochaListener(); process.once('uncaughtException', function (err) { process.on('uncaughtException', mochaListener); @@ -116,7 +110,7 @@ describe("The 'mset' method", function () { describe("with undefined 'key' and defined 'value' parameters", function () { it("throws an error", function () { - var mochaListener = removeMochaListener(); + var mochaListener = helper.removeMochaListener(); process.once('uncaughtException', function (err) { process.on('uncaughtException', mochaListener); diff --git a/test/commands/select.spec.js b/test/commands/select.spec.js index dac3c8b34c..f12eb07d97 100644 --- a/test/commands/select.spec.js +++ b/test/commands/select.spec.js @@ -6,12 +6,6 @@ var redis = config.redis; describe("The 'select' method", function () { - function removeMochaListener () { - var mochaListener = process.listeners('uncaughtException').pop(); - process.removeListener('uncaughtException', mochaListener); - return mochaListener; - } - helper.allTests(function(parser, ip, args) { describe("using " + parser + " and " + ip, function () { @@ -96,7 +90,7 @@ describe("The 'select' method", function () { describe("with an invalid db index", function () { it("throws an error when callback not provided", function (done) { - var mochaListener = removeMochaListener(); + var mochaListener = helper.removeMochaListener(); assert.strictEqual(client.selected_db, null, "default db should be null"); process.once('uncaughtException', function (err) { diff --git a/test/commands/set.spec.js b/test/commands/set.spec.js index 28455c9ebe..072342db4b 100644 --- a/test/commands/set.spec.js +++ b/test/commands/set.spec.js @@ -7,12 +7,6 @@ var uuid = require('uuid'); describe("The 'set' method", function () { - function removeMochaListener () { - var mochaListener = process.listeners('uncaughtException').pop(); - process.removeListener('uncaughtException', mochaListener); - return mochaListener; - } - helper.allTests(function(parser, ip, args) { describe("using " + parser + " and " + ip, function () { @@ -123,7 +117,7 @@ describe("The 'set' method", function () { it("does not throw an error", function (done) { this.timeout(200); - var mochaListener = removeMochaListener(); + var mochaListener = helper.removeMochaListener(); process.once('uncaughtException', function (err) { process.on('uncaughtException', mochaListener); @@ -140,7 +134,7 @@ describe("The 'set' method", function () { describe("with undefined 'key' and defined 'value' parameters", function () { it("throws an error", function () { - var mochaListener = removeMochaListener(); + var mochaListener = helper.removeMochaListener(); process.once('uncaughtException', function (err) { process.on('uncaughtException', mochaListener); diff --git a/test/conf/password.conf b/test/conf/password.conf index 8351cb455f..fae73a9415 100644 --- a/test/conf/password.conf +++ b/test/conf/password.conf @@ -1,5 +1,5 @@ requirepass porkchopsandwiches -port 6378 +port 6379 bind ::1 127.0.0.1 unixsocket /tmp/redis.sock unixsocketperm 755 diff --git a/test/conf/redis.conf b/test/conf/redis.conf index dd9a4b6572..16f1434200 100644 --- a/test/conf/redis.conf +++ b/test/conf/redis.conf @@ -1,4 +1,4 @@ -port 6378 +port 6379 bind ::1 127.0.0.1 unixsocket /tmp/redis.sock unixsocketperm 755 diff --git a/test/helper.js b/test/helper.js index 74404b2630..756ed1d139 100644 --- a/test/helper.js +++ b/test/helper.js @@ -92,12 +92,21 @@ module.exports = { return true; }, allTests: function (cb) { - ['javascript', 'hiredis'].forEach(function (parser) { - cb(parser, "/tmp/redis.sock", config.configureClient(parser, "/tmp/redis.sock")); - ['IPv4', 'IPv6'].forEach(function (ip) { - cb(parser, ip, config.configureClient(parser, ip)); + [undefined].forEach(function (options) { // add buffer option at some point + describe(options && options.return_buffers ? "returning buffers" : "returning strings", function () { + ['hiredis', 'javascript'].forEach(function (parser) { + cb(parser, "/tmp/redis.sock", config.configureClient(parser, "/tmp/redis.sock", options)); + ['IPv4', 'IPv6'].forEach(function (ip) { + cb(parser, ip, config.configureClient(parser, ip, options)); + }); + }); }); }); + }, + removeMochaListener: function () { + var mochaListener = process.listeners('uncaughtException').pop(); + process.removeListener('uncaughtException', mochaListener); + return mochaListener; } } diff --git a/test/lib/config.js b/test/lib/config.js index 54c6ed27bb..296fdc0bf8 100644 --- a/test/lib/config.js +++ b/test/lib/config.js @@ -4,7 +4,7 @@ var redis = require('../../index'); var config = { redis: redis, - PORT: 6378, + PORT: 6379, HOST: { IPv4: "127.0.0.1", IPv6: "::1" diff --git a/test/lib/redis-process.js b/test/lib/redis-process.js index 212cf359c0..32f2045e41 100644 --- a/test/lib/redis-process.js +++ b/test/lib/redis-process.js @@ -11,6 +11,17 @@ module.exports = { var confFile = conf || path.resolve(__dirname, '../conf/redis.conf'); var rp = cp.spawn("redis-server", [confFile], {}); + // capture a failure booting redis, and give + // the user running the test some directions. + rp.once("exit", function (code) { + if (code !== 0) { + console.error('failed to starting redis with exit code "' + code + '" ' + + 'stop any other redis processes currently running (' + + 'hint: lsof -i :6379)'); + process.exit(code) + } + }) + // wait for redis to become available, by // checking the port we bind on. waitForRedis(true, function () { diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index 580e3e127e..93b5531473 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -9,15 +9,36 @@ describe("The node_redis client", function () { helper.allTests(function(parser, ip, args) { + if (args[2]) { // skip if options are undefined + describe("testing parser existence", function () { + it('throws on non-existence', function (done) { + var mochaListener = helper.removeMochaListener(); + + process.once('uncaughtException', function (err) { + process.on('uncaughtException', mochaListener); + assert.equal(err.message, 'Couldn\'t find named parser nonExistingParser on this system'); + return done(); + }); + + // Don't pollute the args for the other connections + var tmp = JSON.parse(JSON.stringify(args)); + tmp[2].parser = 'nonExistingParser'; + redis.createClient.apply(redis.createClient, tmp); + }); + }); + } + describe("using " + parser + " and " + ip, function () { var client; describe("when not connected", function () { afterEach(function () { - client.end(); + if (client) { + client.end(); + } }); - it("connects correctly", function (done) { + it("connects correctly with args", function (done) { client = redis.createClient.apply(redis.createClient, args); client.on("error", done); @@ -28,6 +49,58 @@ describe("The node_redis client", function () { }); }); }); + + it("connects correctly with default values", function (done) { + client = redis.createClient(); + client.on("error", done); + + client.once("ready", function () { + client.removeListener("error", done); + client.get("recon 1", function (err, res) { + done(err); + }); + }); + }); + + it("connects correctly to localhost", function (done) { + client = redis.createClient(null, null); + client.on("error", done); + + client.once("ready", function () { + client.removeListener("error", done); + client.get("recon 1", function (err, res) { + done(err); + }); + }); + }); + + it("throws on strange connection info", function () { + try { + redis.createClient(true); + throw new Error('failed'); + } catch (err) { + assert.equal(err.message, 'unknown type of connection in createClient()'); + } + }); + + if (ip === 'IPv4') { + it('allows connecting with the redis url and the default port', function (done) { + client = redis.createClient('redis://foo:porkchopsandwiches@' + config.HOST[ip]); + client.on("ready", function () { + return done(); + }); + }); + + it('allows connecting with the redis url and no auth', function (done) { + client = redis.createClient('redis://' + config.HOST[ip] + ':' + config.PORT, { + detect_buffers: false + }); + client.on("ready", function () { + return done(); + }); + }); + } + }); describe("when connected", function () {