From 31fafd8b7c4323ea9259c4fc8247dbe2e0d4b13a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 30 May 2017 05:29:51 +0200 Subject: [PATCH] fixup --- index.js | 39 +++++++++++++++++++++++---------------- lib/connect.js | 2 +- lib/reconnect.js | 6 +++--- lib/replyHandler.js | 3 ++- lib/utils.js | 2 +- test/auth.spec.js | 2 +- 6 files changed, 31 insertions(+), 23 deletions(-) diff --git a/index.js b/index.js index 771c7d33ac..54df396863 100644 --- a/index.js +++ b/index.js @@ -74,8 +74,6 @@ class RedisClient extends EventEmitter { } options.enableOfflineQueue = true } - // Only used as timeout until redis has to be connected to redis until throwing an connection error - options.connectTimeout = +options.connectTimeout || 60000 // 60 * 1000 ms // Override the detectBuffers setting if returnBuffers is active and print a warning if (options.returnBuffers && options.detectBuffers) { process.nextTick( @@ -85,12 +83,6 @@ class RedisClient extends EventEmitter { ) options.detectBuffers = false } - this._pipelineQueue = new Queue() // Holds all pipelined commands - this._pubSubMode = 0 - this._subscriptionSet = {} - this._monitoring = false - this.messageBuffers = false - this._closing = false if (options.authPass) { if (options.password) { throw new TypeError('The "password" and "authPass" option may not both be set at the same time.') @@ -108,23 +100,38 @@ class RedisClient extends EventEmitter { this.serverInfo = {} this.connectionId = connectionId++ this.selectedDb = options.db // Save the selected db here, used when reconnecting - this._strCache = '' + + // Private Variables + // Pipelining this._pipeline = false + this._strCache = '' + this._pipelineQueue = new Queue() + // Pub sub mode this._subCommandsLeft = 0 - this.timesConnected = 0 - this.buffers = options.returnBuffers || options.detectBuffers - this._options = options + this._pubSubMode = 0 + this._subscriptionSet = {} + this._subscribeChannels = [] + this._messageBuffers = false + // Others this._multi = false - this._reply = 'ON' // Returning replies is the default - this.retryStrategy = options.retryStrategy || function (options) { + this._monitoring = false + this._parserReturningBuffers = options.returnBuffers || options.detectBuffers + this._options = options + this._reply = 'ON' + this._retryStrategyProvided = !!options.retryStrategy + this._closing = false + this._timesConnected = 0 + this._connectionOptions = cnxOptions + // Only used as timeout until redis has to be connected to redis until throwing an connection error + this._connectTimeout = +options.connectTimeout || 60 * 1000 // ms + this._retryStrategy = options.retryStrategy || function (options) { + // TODO: Find better defaults if (options.attempt > 100) { return } // reconnect after return Math.min(options.attempt * 100, 3000) } - this._retryStrategyProvided = !!options.retryStrategy - this._subscribeChannels = [] utils.setReconnectDefaults(this) // Init parser and connect connect(this) diff --git a/lib/connect.js b/lib/connect.js index 465de5993f..7169887474 100644 --- a/lib/connect.js +++ b/lib/connect.js @@ -60,7 +60,7 @@ function createParser (client) { connect(client) setImmediate(() => client.emit('error', err)) }, - returnBuffers: client.buffers || client.messageBuffers, + returnBuffers: client._parserReturningBuffers, stringNumbers: client._options.stringNumbers || false }) } diff --git a/lib/reconnect.js b/lib/reconnect.js index a634bd67d6..193c472154 100644 --- a/lib/reconnect.js +++ b/lib/reconnect.js @@ -19,7 +19,7 @@ function retryConnection (client, error) { attempt: client.attempts, error, totalRetryTime: client.retryTotaltime, - timesConnected: client.timesConnected + timesConnected: client._timesConnected } client.emit('reconnecting', reconnectParams) @@ -71,11 +71,11 @@ function reconnect (client, why, error) { return } - client.retryDelay = client.retryStrategy({ + client.retryDelay = client._retryStrategy({ attempt: client.attempts, error, totalRetryTime: client.retryTotaltime, - timesConnected: client.timesConnected + timesConnected: client._timesConnected }) if (typeof client.retryDelay !== 'number') { var err diff --git a/lib/replyHandler.js b/lib/replyHandler.js index f29b2a8d1a..fd50eb76d3 100644 --- a/lib/replyHandler.js +++ b/lib/replyHandler.js @@ -42,7 +42,8 @@ function onResult (client, reply) { // the average performance of all other commands in case of no monitor mode if (client._monitoring === true) { var replyStr - if (client.buffers && Buffer.isBuffer(reply)) { + // TODO: This could be further improved performance wise + if (client._parserReturningBuffers && Buffer.isBuffer(reply)) { replyStr = reply.toString() } else { replyStr = reply diff --git a/lib/utils.js b/lib/utils.js index f18271778d..341fdc52a4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -141,7 +141,7 @@ function warn (client, msg) { * @returns {string|number|null|Buffer|any[]|object} */ function handleReply (client, reply, command) { - if (client._options.detectBuffers === true && command.bufferArgs === false || client.messageBuffers === true) { + if (client._options.detectBuffers === true && command.bufferArgs === false || client._messageBuffers === true) { reply = replyToStrings(reply) } diff --git a/test/auth.spec.js b/test/auth.spec.js index 43c40b056a..eabe575da0 100644 --- a/test/auth.spec.js +++ b/test/auth.spec.js @@ -125,7 +125,7 @@ if (process.platform !== 'win32') { client = redis.createClient.apply(null, args) client.auth(auth).catch(done) client.on('ready', function () { - if (this.timesConnected < 3) { + if (this._timesConnected < 3) { let interval = setInterval(() => { if (client.commandQueue.length !== 0) { return