diff --git a/index.js b/index.js index 96cf624402..9cdc46dc37 100644 --- a/index.js +++ b/index.js @@ -568,6 +568,8 @@ RedisClient.prototype.connection_gone = function (why, error) { error.code = 'UNCERTAIN_STATE'; this.flush_and_error(error, ['command_queue']); error.message = 'Redis connection lost and commands aborted in uncertain state. They might have been processed.'; + // TODO: Reconsider emitting this always, as each running command is handled anyway + // This should likely be removed in v.3. This is different to the broken connection as we'll reconnect here this.emit('error', error); } diff --git a/lib/individualCommands.js b/lib/individualCommands.js index 2bfeb6eb42..4f4c8ab7a5 100644 --- a/lib/individualCommands.js +++ b/lib/individualCommands.js @@ -25,7 +25,6 @@ RedisClient.prototype.batch = RedisClient.prototype.BATCH = function batch (args // Store db in this.select_db to restore it on reconnect RedisClient.prototype.select = RedisClient.prototype.SELECT = function select (db, callback) { var self = this; - db = Array.isArray(db) && db.length === 1 ? db[0] : db; return this.internal_send_command('select', [db], function (err, res) { if (err === null) { self.selected_db = db; @@ -149,7 +148,6 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, c debug('Sending auth to ' + self.address + ' id ' + self.connection_id); // Stash auth for connect and reconnect. - pass = Array.isArray(pass) && pass.length === 1 ? pass[0] : pass; this.auth_pass = pass; this.ready = this.offline_queue.length === 0; // keep the execution order intakt var tmp = this.internal_send_command('auth', [pass], function (err, res) { @@ -163,7 +161,7 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, c debug('Redis still loading, trying to authenticate later'); setTimeout(function () { self.auth(pass, callback); - }, 200); + }, 100); return; } } diff --git a/test/auth.spec.js b/test/auth.spec.js index fd79b36845..47fa0964a2 100644 --- a/test/auth.spec.js +++ b/test/auth.spec.js @@ -54,8 +54,8 @@ describe('client authentication', function () { assert.strictEqual('retry worked', res); var now = Date.now(); // Hint: setTimeout sometimes triggers early and therefor the value can be like one or two ms to early - assert(now - time >= 198, 'Time should be above 200 ms (the reconnect time) and is ' + (now - time)); - assert(now - time < 300, 'Time should be below 300 ms (the reconnect should only take a bit above 200 ms) and is ' + (now - time)); + assert(now - time >= 98, 'Time should be above 100 ms (the reconnect time) and is ' + (now - time)); + assert(now - time < 225, 'Time should be below 255 ms (the reconnect should only take a bit above 100 ms) and is ' + (now - time)); done(); }); var tmp = client.command_queue.get(0).callback; diff --git a/test/batch.spec.js b/test/batch.spec.js index 2d02c371f1..5f25be65cb 100644 --- a/test/batch.spec.js +++ b/test/batch.spec.js @@ -12,6 +12,8 @@ describe("The 'batch' method", function () { describe('using ' + parser + ' and ' + ip, function () { describe('when not connected', function () { + // TODO: This is somewhat broken and should be fixed in v.3 + // The commands should return an error instead of returning an empty result var client; beforeEach(function (done) { diff --git a/test/commands/expire.spec.js b/test/commands/expire.spec.js index 4994caf86f..f9041b7109 100644 --- a/test/commands/expire.spec.js +++ b/test/commands/expire.spec.js @@ -23,7 +23,7 @@ describe("The 'expire' method", function () { client.EXPIRE('expiry key', '1', helper.isNumber(1)); setTimeout(function () { client.exists(['expiry key'], helper.isNumber(0, done)); - }, 1100); + }, 1050); }); it('expires key after timeout with array syntax', function (done) { @@ -31,7 +31,7 @@ describe("The 'expire' method", function () { client.EXPIRE(['expiry key', '1'], helper.isNumber(1)); setTimeout(function () { client.exists(['expiry key'], helper.isNumber(0, done)); - }, 1100); + }, 1050); }); afterEach(function () { diff --git a/test/commands/flushdb.spec.js b/test/commands/flushdb.spec.js index b96c4b1031..9081105349 100644 --- a/test/commands/flushdb.spec.js +++ b/test/commands/flushdb.spec.js @@ -90,7 +90,7 @@ describe("The 'flushdb' method", function () { it('results in a db size of zero without a callback', function (done) { client.flushdb(); setTimeout(function (err, res) { - client.dbsize([], function (err, res) { + client.dbsize(function (err, res) { helper.isNotError()(err, res); helper.isType.number()(err, res); assert.strictEqual(0, res, 'Flushing db should result in db size 0'); diff --git a/test/commands/get.spec.js b/test/commands/get.spec.js index dc4b92da25..83a330ad12 100644 --- a/test/commands/get.spec.js +++ b/test/commands/get.spec.js @@ -77,7 +77,7 @@ describe("The 'get' method", function () { client.on('error', function (err) { throw err; }); - setTimeout(done, 50); + setTimeout(done, 25); }); }); diff --git a/test/commands/info.spec.js b/test/commands/info.spec.js index ce4d486fb7..838dac21b8 100644 --- a/test/commands/info.spec.js +++ b/test/commands/info.spec.js @@ -34,7 +34,7 @@ describe("The 'info' method", function () { setTimeout(function () { assert.strictEqual(typeof client.server_info.db2, 'object'); done(); - }, 150); + }, 30); }); it('works with optional section provided with and without callback', function (done) { diff --git a/test/commands/select.spec.js b/test/commands/select.spec.js index 4e418767fb..d8878fe48c 100644 --- a/test/commands/select.spec.js +++ b/test/commands/select.spec.js @@ -62,7 +62,7 @@ describe("The 'select' method", function () { client.select(1, function (err) { assert.equal(err, null); assert.equal(client.selected_db, 1, 'we should have selected the new valid DB'); - return done(); + done(); }); }); }); @@ -73,7 +73,7 @@ describe("The 'select' method", function () { client.select(9999, function (err) { assert.equal(err.code, 'ERR'); assert.equal(err.message, 'ERR invalid DB index'); - return done(); + done(); }); }); }); @@ -86,8 +86,8 @@ describe("The 'select' method", function () { client.select(1); setTimeout(function () { assert.equal(client.selected_db, 1, 'we should have selected the new valid DB'); - return done(); - }, 100); + done(); + }, 25); }); }); diff --git a/test/commands/set.spec.js b/test/commands/set.spec.js index 94b6cd69df..7cb4b8314a 100644 --- a/test/commands/set.spec.js +++ b/test/commands/set.spec.js @@ -43,7 +43,7 @@ describe("The 'set' method", function () { beforeEach(function (done) { client = redis.createClient.apply(null, args); client.once('ready', function () { - done(); + client.flushdb(done); }); }); @@ -62,6 +62,35 @@ describe("The 'set' method", function () { }); }); }); + + it('set expire date in seconds', function (done) { + client.set('foo', 'bar', 'ex', 10, helper.isString('OK')); + client.pttl('foo', function (err, res) { + assert(res >= 10000 - 50); // Max 50 ms should have passed + assert(res <= 10000); // Max possible should be 10.000 + done(err); + }); + }); + + it('set expire date in milliseconds', function (done) { + client.set('foo', 'bar', 'px', 100, helper.isString('OK')); + client.pttl('foo', function (err, res) { + assert(res >= 50); // Max 50 ms should have passed + assert(res <= 100); // Max possible should be 100 + done(err); + }); + }); + + it('only set the key if (not) already set', function (done) { + client.set('foo', 'bar', 'NX', helper.isString('OK')); + client.set('foo', 'bar', 'nx', helper.isNull()); + client.set('foo', 'bar', 'EX', '10', 'XX', helper.isString('OK')); + client.ttl('foo', function (err, res) { + assert(res >= 9); // Min 9s should be left + assert(res <= 10); // Max 10s should be left + done(err); + }); + }); }); describe('reports an error with invalid parameters', function () { diff --git a/test/commands/ttl.spec.js b/test/commands/ttl.spec.js index 5309b70a1a..65212f710c 100644 --- a/test/commands/ttl.spec.js +++ b/test/commands/ttl.spec.js @@ -22,12 +22,11 @@ describe("The 'ttl' method", function () { it('returns the current ttl on a key', function (done) { client.set(['ttl key', 'ttl val'], helper.isString('OK')); client.expire(['ttl key', '100'], helper.isNumber(1)); - setTimeout(function () { - client.TTL(['ttl key'], function (err, ttl) { - assert.ok(ttl > 50 && ttl <= 100); - return done(err); - }); - }, 200); + client.TTL(['ttl key'], function (err, ttl) { + assert(ttl >= 99); + assert(ttl <= 100); + done(err); + }); }); afterEach(function () { diff --git a/test/conf/password.conf b/test/conf/password.conf index fae73a9415..c2b8feb447 100644 --- a/test/conf/password.conf +++ b/test/conf/password.conf @@ -2,4 +2,4 @@ requirepass porkchopsandwiches port 6379 bind ::1 127.0.0.1 unixsocket /tmp/redis.sock -unixsocketperm 755 +unixsocketperm 700 diff --git a/test/conf/redis.conf b/test/conf/redis.conf index 16f1434200..fad47a5fc6 100644 --- a/test/conf/redis.conf +++ b/test/conf/redis.conf @@ -1,4 +1,4 @@ port 6379 bind ::1 127.0.0.1 unixsocket /tmp/redis.sock -unixsocketperm 755 +unixsocketperm 700 diff --git a/test/conf/rename.conf b/test/conf/rename.conf index 4da225053f..ef83923655 100644 --- a/test/conf/rename.conf +++ b/test/conf/rename.conf @@ -1,7 +1,7 @@ port 6379 bind ::1 127.0.0.1 unixsocket /tmp/redis.sock -unixsocketperm 755 +unixsocketperm 700 rename-command SET 807081f5afa96845a02816a28b7258c3 rename-command GET f397808a43ceca3963e22b4e13deb672 rename-command GETRANGE 9e3102b15cf231c4e9e940f284744fe0 diff --git a/test/conf/slave.conf b/test/conf/slave.conf index 61ea50d959..31cd801c26 100644 --- a/test/conf/slave.conf +++ b/test/conf/slave.conf @@ -1,6 +1,6 @@ port 6381 bind ::1 127.0.0.1 unixsocket /tmp/redis6381.sock -unixsocketperm 755 +unixsocketperm 700 slaveof localhost 6379 masterauth porkchopsandwiches diff --git a/test/conf/stunnel.conf.template b/test/conf/stunnel.conf.template index 2f23951652..17ee3e879c 100644 --- a/test/conf/stunnel.conf.template +++ b/test/conf/stunnel.conf.template @@ -1,5 +1,5 @@ pid = __dirname/stunnel.pid -output = __dirname/stunnel.log +; output = __dirname/stunnel.log CAfile = __dirname/redis.js.org.cert cert = __dirname/redis.js.org.cert key = __dirname/redis.js.org.key diff --git a/test/connection.spec.js b/test/connection.spec.js index c87a15816d..1279ec4e37 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -89,6 +89,7 @@ describe('connection tests', function () { assert(called); done(); }); + // TODO: In v.3 the quit command would be fired right away, so bool should be true assert.strictEqual(bool, false); }); @@ -99,6 +100,18 @@ describe('connection tests', function () { assert.strictEqual(bool, false); }); + it('quit "succeeds" even if the client connection is closed while doing so', function (done) { + client = redis.createClient(); + client.set('foo', 'bar', function (err, res) { + assert.strictEqual(res, 'OK'); + client.quit(function (err, res) { + assert.strictEqual(res, 'OK'); + done(err); + }); + client.end(true); // Flushing the quit command should result in a success + }); + }); + it('quit right away if connection drops while quit command is on the fly', function (done) { client = redis.createClient(); client.once('ready', function () { @@ -119,7 +132,7 @@ describe('connection tests', function () { describe('on lost connection', function () { it('emit an error after max retry attempts and do not try to reconnect afterwards', function (done) { - var max_attempts = 4; + var max_attempts = 3; var options = { parser: parser, max_attempts: max_attempts @@ -138,10 +151,14 @@ describe('connection tests', function () { client.on('error', function (err) { if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) { - setTimeout(function () { + process.nextTick(function () { // End is called after the error got emitted assert.strictEqual(calls, max_attempts - 1); + assert.strictEqual(client.emitted_end, true); + assert.strictEqual(client.connected, false); + assert.strictEqual(client.ready, false); + assert.strictEqual(client.closing, true); done(); - }, 500); + }); } }); }); @@ -167,11 +184,15 @@ describe('connection tests', function () { client.on('error', function (err) { if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) { - setTimeout(function () { + process.nextTick(function () { // End is called after the error got emitted + assert.strictEqual(client.emitted_end, true); + assert.strictEqual(client.connected, false); + assert.strictEqual(client.ready, false); + assert.strictEqual(client.closing, true); assert.strictEqual(client.retry_totaltime, connect_timeout); assert.strictEqual(time, connect_timeout); done(); - }, 500); + }); } }); }); @@ -190,7 +211,7 @@ describe('connection tests', function () { client.on('reconnecting', function (params) { client.end(true); assert.strictEqual(params.times_connected, 1); - setTimeout(done, 100); + setTimeout(done, 5); }); }); @@ -291,7 +312,6 @@ describe('connection tests', function () { it('emit an error after the socket timeout exceeded the connect_timeout time', function (done) { var connect_timeout = 500; // in ms - var time = Date.now(); client = redis.createClient({ parser: parser, // Auto detect ipv4 and use non routable ip to trigger the timeout @@ -308,17 +328,18 @@ describe('connection tests', function () { throw new Error('No reconnect, since no connection was ever established'); }); + var time = Date.now(); client.on('error', function (err) { if (err.code === 'ENETUNREACH') { // The test is run without a internet connection. Pretent it works return done(); } assert(/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)); // The code execution on windows is very slow at times - var add = process.platform !== 'win32' ? 25 : 125; + var add = process.platform !== 'win32' ? 15 : 200; var now = Date.now(); assert(now - time < connect_timeout + add, 'The real timeout time should be below ' + (connect_timeout + add) + 'ms but is: ' + (now - time)); // Timers sometimes trigger early (e.g. 1ms to early) - assert(now - time >= connect_timeout - 3, 'The real timeout time should be above ' + connect_timeout + 'ms, but it is: ' + (now - time)); + assert(now - time >= connect_timeout - 5, 'The real timeout time should be above ' + connect_timeout + 'ms, but it is: ' + (now - time)); done(); }); }); @@ -546,7 +567,7 @@ describe('connection tests', function () { }); } - it('redis still loading <= 1000ms', function (done) { + it('redis still loading <= 500', function (done) { client = redis.createClient.apply(null, args); var tmp = client.info.bind(client); var end = helper.callFuncAfter(done, 3); @@ -568,9 +589,9 @@ describe('connection tests', function () { }; client.on('ready', function () { var rest = Date.now() - time; - assert(rest >= 498, 'Rest should be equal or above 500 ms but is: ' + rest); // setTimeout might trigger early + assert(rest >= 495, 'Rest should be equal or above 500 ms but is: ' + rest); // setTimeout might trigger early // Be on the safe side and accept 200ms above the original value - assert(rest - 200 < 500, 'Rest - 200 should be below 500 ms but is: ' + (rest - 200)); + assert(rest - 250 < 500, 'Rest - 250 should be below 500 ms but is: ' + (rest - 250)); assert(delayed); end(); }); @@ -601,7 +622,7 @@ describe('connection tests', function () { var rest = Date.now() - time; assert(rest >= 998, '`rest` should be equal or above 1000 ms but is: ' + rest); // setTimeout might trigger early // Be on the safe side and accept 200ms above the original value - assert(rest - 200 < 1000, '`rest` - 200 should be below 1000 ms but is: ' + (rest - 200)); + assert(rest - 250 < 1000, '`rest` - 250 should be below 1000 ms but is: ' + (rest - 250)); assert(delayed); end(); }); diff --git a/test/multi.spec.js b/test/multi.spec.js index 874d620134..817433db34 100644 --- a/test/multi.spec.js +++ b/test/multi.spec.js @@ -73,7 +73,7 @@ describe("The 'multi' method", function () { describe('pipeline limit', function () { it('do not exceed maximum string size', function (done) { - this.timeout(30000); // Windows tests are horribly slow + this.timeout(process.platform !== 'win32' ? 10000 : 35000); // Windows tests are horribly slow // Triggers a RangeError: Invalid string length if not handled properly client = redis.createClient(); var multi = client.multi(); diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index be4646fe5f..060c62926c 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -266,7 +266,7 @@ describe('The node_redis client', function () { bclient.blpop('blocking list 2', 5, function (err, value) { assert.strictEqual(value[0], 'blocking list 2'); assert.strictEqual(value[1], 'initial value'); - return done(err); + done(err); }); bclient.once('ready', function () { setTimeout(function () { @@ -297,10 +297,11 @@ describe('The node_redis client', function () { } client.set('foo', 'bar', cb); } + client.on('warning', function () {}); // Ignore deprecation message setTimeout(function () { finished = true; done(); - }, 250); + }, 25); }); it('used with flush set to true', function (done) { @@ -314,6 +315,7 @@ describe('The node_redis client', function () { for (var i = 0; i < 20; i++) { if (i === 10) { client.end(true); + client.stream.write('foo'); // Trigger an error on the closed stream that we ignore } client.set('foo', 'bar', cb); } @@ -583,6 +585,24 @@ describe('The node_redis client', function () { }); }); + it('monitor does not activate if the command could not be processed properly', function (done) { + client.MONITOR(function (err, res) { + assert.strictEqual(err.code, 'UNCERTAIN_STATE'); + }); + client.on('error', function (err) {}); // Ignore error here + client.stream.destroy(); + client.on('monitor', function (time, args, rawOutput) { + done(new Error('failed')); // Should not be activated + }); + client.on('reconnecting', function () { + client.get('foo', function (err, res) { + assert(!err); + assert.strictEqual(client.monitoring, false); + setTimeout(done, 10); // The monitor command might be returned a tiny bit later + }); + }); + }); + it('monitors works in combination with the pub sub mode and the offline queue', function (done) { var responses = []; var pub = redis.createClient(); @@ -657,7 +677,7 @@ describe('The node_redis client', function () { client.set(['utf8test', utf8_sample], helper.isString('OK')); client.get(['utf8test'], function (err, obj) { assert.strictEqual(utf8_sample, obj); - return done(err); + done(err); }); }); }); @@ -671,13 +691,13 @@ describe('The node_redis client', function () { var id = setTimeout(function () { external.kill(); - return done(Error('unref subprocess timed out')); + done(Error('unref subprocess timed out')); }, 8000); external.on('close', function (code) { clearTimeout(id); assert.strictEqual(code, 0); - return done(); + done(); }); }); }); @@ -727,11 +747,7 @@ describe('The node_redis client', function () { client = redis.createClient.apply(null, args); client.on('ready', function () { assert.strictEqual(true, client.options.socket_nodelay); - client.quit(); - - client.once('end', function () { - return done(); - }); + client.quit(done); }); }); @@ -743,11 +759,7 @@ describe('The node_redis client', function () { client.set(['set key 2', 'set val'], helper.isString('OK')); client.get(['set key 1'], helper.isString('set val')); client.get(['set key 2'], helper.isString('set val')); - client.quit(); - - client.once('end', function () { - return done(); - }); + client.quit(done); }); }); }); @@ -761,11 +773,7 @@ describe('The node_redis client', function () { client = redis.createClient.apply(null, args); client.on('ready', function () { assert.strictEqual(false, client.options.socket_nodelay); - client.quit(); - - client.once('end', function () { - return done(); - }); + client.quit(done); }); }); @@ -777,11 +785,7 @@ describe('The node_redis client', function () { client.set(['set key 2', 'set val'], helper.isString('OK')); client.get(['set key 1'], helper.isString('set val')); client.get(['set key 2'], helper.isString('set val')); - client.quit(); - - client.once('end', function () { - return done(); - }); + client.quit(done); }); }); }); @@ -792,11 +796,7 @@ describe('The node_redis client', function () { client = redis.createClient.apply(null, args); client.on('ready', function () { assert.strictEqual(true, client.options.socket_nodelay); - client.quit(); - - client.once('end', function () { - return done(); - }); + client.quit(done); }); }); @@ -808,11 +808,7 @@ describe('The node_redis client', function () { client.set(['set key 2', 'set val'], helper.isString('OK')); client.get(['set key 1'], helper.isString('set val')); client.get(['set key 2'], helper.isString('set val')); - client.quit(); - - client.once('end', function () { - return done(); - }); + client.quit(done); }); }); }); @@ -902,19 +898,16 @@ describe('The node_redis client', function () { // ignore, b/c expecting a "can't connect" error }); - return setTimeout(function () { + setTimeout(function () { client.set('foo', 'bar', function (err, result) { - if (!finished) { - // This should never be called - return done(err); - } + if (!finished) done(err); assert.strictEqual(err.message, "The command can't be processed. The connection has already been closed."); }); - return setTimeout(function () { + setTimeout(function () { assert.strictEqual(client.offline_queue.length, 1); finished = true; - return done(); + done(); }, 25); }, 50); }); diff --git a/test/pubsub.spec.js b/test/pubsub.spec.js index 1651746a54..b24c1899dc 100644 --- a/test/pubsub.spec.js +++ b/test/pubsub.spec.js @@ -57,7 +57,8 @@ describe('publish/subscribe', function () { sub.on('reconnecting', function () { a = true; sub.on('ready', function () { - setTimeout(done, 250); + assert.strictEqual(sub.command_queue.length, 0); + done(); }); }); diff --git a/test/tls.spec.js b/test/tls.spec.js index 68cc02d95f..40a424c27c 100644 --- a/test/tls.spec.js +++ b/test/tls.spec.js @@ -71,10 +71,15 @@ describe('TLS connection tests', function () { client.on('error', function (err) { if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) { - setTimeout(function () { - assert(time === connect_timeout); + process.nextTick(function () { + assert.strictEqual(time, connect_timeout); + assert.strictEqual(client.emitted_end, true); + assert.strictEqual(client.connected, false); + assert.strictEqual(client.ready, false); + assert.strictEqual(client.closing, true); + assert.strictEqual(time, connect_timeout); done(); - }, 100); + }); } }); });