refactor: better code style for handling sentinel errors

redis · Apr 16, 2017 · 626deb4 · 626deb4
1 parent 8f3d3f7
commit 626deb4
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 25 deletions.
diff --git a/lib/connectors/sentinel_connector.js b/lib/connectors/sentinel_connector.js
@@ -48,24 +48,27 @@ SentinelConnector.prototype.connect = function (callback, eventEmitter) {
     if (_this.currentPoint === _this.sentinels.length) {
       _this.currentPoint = -1;
 
-      var error;
-      var retryDelay;
-      if (typeof _this.options.sentinelRetryStrategy === 'function') {
-        retryDelay = _this.options.sentinelRetryStrategy(++_this.retryAttempts);
+      var retryDelay = typeof _this.options.sentinelRetryStrategy === 'function'
+        ? _this.options.sentinelRetryStrategy(++_this.retryAttempts)
+        : null;
+
+      var errorMsg = typeof retryDelay !== 'number'
+        ? 'All sentinels are unreachable and retry is disabled.'
+        : 'All sentinels are unreachable. Retrying from scratch after ' + retryDelay + 'ms';
+
+      if (lastError) {
+        errorMsg += ' Last error: ' + lastError.message;
       }
 
-      if (typeof retryDelay !== 'number') {
-        debug('All sentinels are unreachable and retry is disabled, emitting error...');
-        error = 'All sentinels are unreachable.';
-        if (lastError) {
-          error += ' Last error: ' + lastError.message;
-        }
-        return callback(new Error(error));
+      debug(errorMsg);
+
+      var error = new Error(errorMsg);
+      if (typeof retryDelay === 'number') {
+        setTimeout(connectToNext, retryDelay);
+        eventEmitter('error', error);
+      } else {
+        callback(error);
       }
-      debug('All sentinels are unreachable. Retrying from scratch after %d', retryDelay);
-      error = 'All sentinels are unreachable, retrying...';
-      setTimeout(connectToNext, retryDelay);
-      eventEmitter('error', new Error(error));
       return;
     }
 
@@ -79,15 +82,19 @@ SentinelConnector.prototype.connect = function (callback, eventEmitter) {
         debug('resolved: %s:%s', resolved.host, resolved.port);
         _this.stream = net.createConnection(resolved);
         callback(null, _this.stream);
-      } else if (err) {
-        debug('failed to connect to sentinel %s:%s because %s', endpoint.host, endpoint.port, err);
-        eventEmitter('sentinelError', new Error('failed to connect to sentinel '+endpoint.host+':'+endpoint.port+' because '+err));
-        lastError = err;
-        connectToNext();
       } else {
-        debug('connected to sentinel %s:%s successfully, but got a invalid reply: %s',
-          endpoint.host, endpoint.port, resolved);
-        eventEmitter('sentinelError', new Error('connected to sentinel '+endpoint.host+':'+endpoint.port+' successfully, but got a invalid reply: '+resolved));
+        var endpointAddress = endpoint.host + ':' + endpoint.port;
+        var errorMsg = err
+          ? 'failed to connect to sentinel ' + endpointAddress + ' because ' + err.message
+          : 'connected to sentinel ' + endpointAddress + ' successfully, but got an invalid reply: ' + resolved;
+
+        debug(errorMsg);
+
+        eventEmitter('sentinelError', new Error(errorMsg));
+
+        if (err) {
+          lastError = err;
+        }
         connectToNext();
       }
     });

diff --git a/lib/redis.js b/lib/redis.js
@@ -311,7 +311,7 @@ Redis.prototype.connect = function (callback) {
       };
       _this.once(CONNECT_EVENT, connectionConnectHandler);
       _this.once('close', connectionCloseHandler);
-    }, function(type, err) {
+    }, function (type, err) {
       _this.silentEmit(type, err);
     });
   }.bind(this)).nodeify(callback);

diff --git a/test/functional/sentinel.js b/test/functional/sentinel.js
@@ -168,7 +168,7 @@ describe('sentinel', function () {
       redis.connect().then(function () {
         throw new Error('Expect `connect` to be thrown');
       }).catch(function (err) {
-        expect(err.message).to.eql('All sentinels are unreachable. Last error: just rejected');
+        expect(err.message).to.eql('All sentinels are unreachable and retry is disabled. Last error: just rejected');
         redis.disconnect();
         sentinel.disconnect(done);
       });