From 146d88154c6f080a45265f2663ab3d61cdc79d83 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Mon, 5 Oct 2015 11:16:24 +0200 Subject: [PATCH] Fix send_command always returning should_buffer boolean Fix .auth, .select and .exec to return the should_buffer boolean --- changelog.md | 3 ++- index.js | 25 +++++++++++++------------ test/commands/multi.spec.js | 3 ++- test/commands/select.spec.js | 8 +++++--- test/commands/setex.spec.js | 3 ++- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/changelog.md b/changelog.md index 9cdca6501e..be6f7a2b98 100644 --- a/changelog.md +++ b/changelog.md @@ -14,7 +14,8 @@ Features Bugfixes -- Fix a javascript parser regression introduced in 2.0 that could result in timeouts on high load. (@BridgeAR) +- Fix a javascript parser regression introduced in 2.0 that could result in timeouts on high load. (@BridgeAR) +- Fixed should_buffer boolean for .exec, .select and .auth commands not being returned (@BridgeAR) ## v2.1.0 - Oct 02, 2015 diff --git a/index.js b/index.js index b8300577de..23fd75fc1e 100644 --- a/index.js +++ b/index.js @@ -677,10 +677,11 @@ RedisClient.prototype.send_command = function (command, args, callback) { err = new Error('send_command: ' + command + ' value must not be undefined or null'); err.command = command; if (callback) { - return callback && callback(err); + callback(err); + } else { + this.emit('error', err); } - this.emit('error', err); - return; + return false; } } @@ -715,7 +716,8 @@ RedisClient.prototype.send_command = function (command, args, callback) { this.offline_queue.push(command_obj); this.should_buffer = true; } - return; + // Return false to signal no buffering + return false; } if (command === 'subscribe' || command === 'psubscribe' || command === 'unsubscribe' || command === 'punsubscribe') { @@ -728,7 +730,7 @@ RedisClient.prototype.send_command = function (command, args, callback) { err = new Error('Connection in subscriber mode, only subscriber commands may be used'); err.command = command.toUpperCase(); this.emit('error', err); - return; + return false; } this.command_queue.push(command_obj); this.commands_sent += 1; @@ -916,8 +918,7 @@ commands.forEach(function (fullCommand) { // store db in this.select_db to restore it on reconnect RedisClient.prototype.select = RedisClient.prototype.SELECT = function (db, callback) { var self = this; - - this.send_command('select', [db], function (err, res) { + return this.send_command('select', [db], function (err, res) { if (err === null) { self.selected_db = db; } @@ -939,16 +940,16 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function (pass, callba } else { this.emit('error', err); } - return; + return false; } this.auth_pass = pass; debug('Saving auth as ' + this.auth_pass); // Only run the callback once. So do not safe it if already connected if (this.connected) { - this.send_command('auth', [this.auth_pass], callback); - } else { - this.auth_callback = callback; + return this.send_command('auth', [this.auth_pass], callback); } + this.auth_callback = callback; + return false; }; RedisClient.prototype.hmset = RedisClient.prototype.HMSET = function (key, args, callback) { @@ -1048,7 +1049,7 @@ Multi.prototype.exec = Multi.prototype.EXEC = function (callback) { this.send_command(command, args, index, cb); } - this._client.send_command('exec', [], function(err, replies) { + return this._client.send_command('exec', [], function(err, replies) { self.execute_callback(err, replies); }); }; diff --git a/test/commands/multi.spec.js b/test/commands/multi.spec.js index fa099a2856..89868133b0 100644 --- a/test/commands/multi.spec.js +++ b/test/commands/multi.spec.js @@ -203,11 +203,12 @@ describe("The 'multi' method", function () { }); it('runs a multi without any further commands', function(done) { - client.multi().exec(function(err, res) { + var buffering = client.multi().exec(function(err, res) { assert.strictEqual(err, null); assert.strictEqual(res.length, 0); done(); }); + assert(typeof buffering === 'boolean'); }); it('allows multiple operations to be performed using a chaining API', function (done) { diff --git a/test/commands/select.spec.js b/test/commands/select.spec.js index dc121b781a..3a90703332 100644 --- a/test/commands/select.spec.js +++ b/test/commands/select.spec.js @@ -24,10 +24,11 @@ describe("The 'select' method", function () { }); it("returns an error if redis is not connected", function (done) { - client.select(1, function (err, res) { + var buffering = client.select(1, function (err, res) { assert(err.message.match(/The connection has already been closed/)); done(); }); + assert(typeof buffering === 'boolean'); }); }); @@ -36,7 +37,7 @@ describe("The 'select' method", function () { beforeEach(function (done) { client = redis.createClient.apply(redis.createClient, args); - client.once("connect", function () { done(); }); + client.once("ready", function () { done(); }); }); afterEach(function () { @@ -46,11 +47,12 @@ describe("The 'select' method", function () { it("changes the database and calls the callback", function (done) { // default value of null means database 0 will be used. assert.strictEqual(client.selected_db, null, "default db should be null"); - client.SELECT(1, function (err, res) { + var buffering = client.SELECT(1, function (err, res) { helper.isNotError()(err, res); assert.strictEqual(client.selected_db, 1, "db should be 1 after select"); done(); }); + assert(typeof buffering === 'boolean'); }); describe("and a callback is specified", function () { diff --git a/test/commands/setex.spec.js b/test/commands/setex.spec.js index e8de49bd57..7d0b152ae0 100644 --- a/test/commands/setex.spec.js +++ b/test/commands/setex.spec.js @@ -29,7 +29,8 @@ describe("The 'setex' method", function () { }); it('returns an error if no value is provided', function (done) { - client.SETEX(["setex key", "100", undefined], helper.isError(done)); + var buffering = client.SETEX(["setex key", "100", undefined], helper.isError(done)); + assert(typeof buffering === 'boolean'); }); afterEach(function () {