Fix should_buffer return values and empty .batch and .auth return value being sync

Fix test

Author: Ruben Bridgewater

README.md

@@ -152,8 +152,13 @@ So please attach the error listener to node_redis.
 ### "drain"
 
 `client` will emit `drain` when the TCP connection to the Redis server has been buffering, but is now
-writable. This event can be used to stream commands in to Redis and adapt to backpressure. Right now,
-you need to check `client.command_queue.length` to decide when to reduce your send rate and resume sending commands when you get `drain`.
+writable. This event can be used to stream commands in to Redis and adapt to backpressure.
+
+All commands return a boolean if the stream had to buffer or not. If false is returned the stream had to buffer.
+That way you can decide when to reduce your send rate and resume sending commands when you get `drain`.
+
+You can manually control the low water and high water marks by passing ommand_queue_high_water` and `command_queue_low_water` to the client options.
+Check the [Node.js streams API](https://nodejs.org/api/stream.html) for further info.
 
 ### "idle"
 
@@ -689,7 +694,7 @@ Client count: 5, node version: 4.1.2, server version: 3.0.3, parser: hiredis
 End of tests. Total time elapsed: 44952 ms
 
 The hiredis and js parser should most of the time be on the same level. The js parser lacks speed for large responses though.
-Therefor the hiredis parser is the default used in node_redis. To use `hiredis`, do:
+Therefor the hiredis parser is the default used in node_redis and we recommend using the hiredis parser. To use `hiredis`, do:
 
     npm install hiredis redis
 

changelog.md

@@ -19,7 +19,7 @@ Features
 Bugfixes
 
 -  Fix a javascript parser regression introduced in 2.0 that could result in timeouts on high load. ([@BridgeAR](https://github.com/BridgeAR))
--  Fixed should_buffer boolean for .exec, .select and .auth commands not being returned ([@BridgeAR](https://github.com/BridgeAR))
+-  Fixed should_buffer boolean for .exec, .select and .auth commands not being returned and fix a couple special conditions ([@BridgeAR](https://github.com/BridgeAR))
 
 If you do not rely on transactions but want to reduce the RTT you can use .batch from now on. It'll behave just the same as .multi but it does not have any transaction and therefor won't roll back any failed commands.<br>
 Both .multi and .batch are from now on going to cache the commands and release them while calling .exec.

index.js

@@ -690,7 +690,8 @@ RedisClient.prototype.send_command = function (command, args, callback) {
             } else {
                 this.emit('error', err);
             }
-            return false;
+            // Singal no buffering
+            return true;
         }
     }
 
@@ -725,7 +726,7 @@ RedisClient.prototype.send_command = function (command, args, callback) {
             this.offline_queue.push(command_obj);
             this.should_buffer = true;
         }
-        // Return false to signal no buffering
+        // Return false to signal buffering
         return false;
     }
 
@@ -739,7 +740,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 false;
+        return true;
     }
     this.command_queue.push(command_obj);
     this.commands_sent += 1;
@@ -800,7 +801,7 @@ RedisClient.prototype.writeStream = function (data) {
 
     // Do not use a pipeline
     if (this.pipeline === 0) {
-        return !this.stream.__write(data);
+        return this.stream.__write(data);
     }
     this.pipeline--;
     this.pipeline_queue.push(data);
@@ -975,11 +976,13 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function (pass, callba
         var err = new Error('The password has to be of type "string"');
         err.command = 'AUTH';
         if (callback) {
-            callback(err);
+            setImmediate(function () {
+                callback(err);
+            });
         } else {
             this.emit('error', err);
         }
-        return false;
+        return true;
     }
     this.auth_pass = pass;
     debug('Saving auth as ' + this.auth_pass);
@@ -988,7 +991,7 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function (pass, callba
         return this.send_command('auth', [this.auth_pass], callback);
     }
     this.auth_callback = callback;
-    return false;
+    return true;
 };
 
 RedisClient.prototype.hmset = RedisClient.prototype.HMSET = function (key, args, callback) {
@@ -1180,9 +1183,11 @@ Multi.prototype.exec = Multi.prototype.EXEC = function (callback) {
     var args;
     if (len === 0) {
         if (callback) {
-            callback(null, []);
+            setImmediate(function () {
+                callback(null, []);
+            });
         }
-        return false;
+        return true;
     }
     this.results = new Array(len);
     this._client.pipeline = len;

test/auth.spec.js

@@ -128,12 +128,15 @@ describe("client authentication", function () {
                 if (helper.redisProcess().spawnFailed()) this.skip();
 
                 client = redis.createClient.apply(redis.createClient, args);
+                var async = true;
                 client.auth(undefined, function(err, res) {
                     assert.strictEqual(err.message, 'The password has to be of type "string"');
                     assert.strictEqual(err.command, 'AUTH');
                     assert.strictEqual(res, undefined);
+                    async = false;
                     done();
                 });
+                assert(async);
             });
 
             it('should emit an error if the password is not of type string and no callback has been provided', function (done) {

test/commands/batch.spec.js test/batch.spec.jstest/commands/batch.spec.js renamed to test/batch.spec.js

@@ -1,8 +1,8 @@
 'use strict';
 
 var assert = require('assert');
-var config = require("../lib/config");
-var helper = require('../helper');
+var config = require("./lib/config");
+var helper = require('./helper');
 var redis = config.redis;
 var uuid = require('uuid');
 
@@ -52,7 +52,7 @@ describe("The 'batch' method", function () {
 
                 beforeEach(function (done) {
                     client = redis.createClient.apply(redis.createClient, args);
-                    client.once("connect", function () {
+                    client.once("ready", function () {
                         client.flushdb(function (err) {
                             return done(err);
                         });
@@ -63,6 +63,19 @@ describe("The 'batch' method", function () {
                     client.end();
                 });
 
+                it("returns an empty result array", function (done) {
+                    var batch = client.batch();
+                    var async = true;
+                    var notBuffering = batch.exec(function (err, res) {
+                        assert.strictEqual(err, null);
+                        assert.strictEqual(res.length, 0);
+                        async = false;
+                        done();
+                    });
+                    assert(async);
+                    assert.strictEqual(notBuffering, true);
+                });
+
                 it('fail individually when one command fails using chaining notation', function (done) {
                     var batch1, batch2;
                     batch1 = client.batch();
@@ -216,8 +229,7 @@ describe("The 'batch' method", function () {
 
                 it('runs a batch without any further commands and without callback', function() {
                     var buffering = client.batch().exec();
-                    assert(typeof buffering === 'boolean');
-                    assert(!buffering);
+                    assert.strictEqual(buffering, true);
                 });
 
                 it('allows batchple operations to be performed using a chaining API', function (done) {

test/commands/multi.spec.js

@@ -32,11 +32,12 @@ describe("The 'multi' method", function () {
                 });
 
                 it("reports an error", function (done) {
-                    client.multi();
-                    client.exec(function (err, res) {
+                    var multi = client.multi();
+                    var notBuffering = multi.exec(function (err, res) {
                         assert(err.message.match(/The connection has already been closed/));
                         done();
                     });
+                    assert.strictEqual(notBuffering, false);
                 });
 
                 it("reports an error if promisified", function () {
@@ -51,7 +52,7 @@ describe("The 'multi' method", function () {
 
                 beforeEach(function (done) {
                     client = redis.createClient.apply(redis.createClient, args);
-                    client.once("connect", function () {
+                    client.once("ready", function () {
                         client.flushdb(function (err) {
                             return done(err);
                         });
@@ -62,6 +63,16 @@ describe("The 'multi' method", function () {
                     client.end();
                 });
 
+                it("returns an empty result array", function (done) {
+                    var multi = client.multi();
+                    var notBuffering = multi.exec(function (err, res) {
+                        assert.strictEqual(err, null);
+                        assert.strictEqual(res.length, 0);
+                        done();
+                    });
+                    assert.strictEqual(notBuffering, true);
+                });
+
                 it('roles back a transaction when one command in a sequence of commands fails', function (done) {
                     var multi1, multi2;
                     var expected = helper.serverVersionAtLeast(client, [2, 6, 5]) ? helper.isError() : function () {};