Merge pull request #85 from raveljs/feature/84

Using new node-redis reconnection stuff properly now. Better logging.

.eslintrc.json

@@ -56,7 +56,7 @@
       "single"
     ],
     "semi": [2, "always"],
-    "strict": [2, "global"],
+    "strict": [0, "global"],
     "valid-typeof": 2,
     "wrap-iife": [
       2,

lib/ravel.js

@@ -44,7 +44,8 @@ class Ravel extends EventEmitter {
     this.ApplicationError = require('./util/application_error');
 
     // init logging system
-    this.Log = new (require('./util/log'))(this);
+    this.log = new (require('./util/log'))(this);
+    this.Log = this.log;
 
     // init dependency injection utility, which is used by most everything else
     this[coreSymbols.injector] = new (require('./core/injector'))(this, module.parent);
@@ -60,6 +61,7 @@ class Ravel extends EventEmitter {
     this.registerSimpleParameter('redis host', true, '0.0.0.0');
     this.registerSimpleParameter('redis port', true, 6379);
     this.registerSimpleParameter('redis password');
+    this.registerSimpleParameter('redis max retries', true, 10);
     // Node/koa parameters
     this.registerSimpleParameter('port', true, 8080);
     this.registerSimpleParameter('koa public directory');
@@ -130,8 +132,10 @@ class Ravel extends EventEmitter {
     const sessionStoreArgs = {
       host:this.get('redis host'),
       port:this.get('redis port'),
-      db:0, //we stick to 0 because clustered redis doesn't support multiple dbs
-      'no_ready_check': true
+      options: {
+        'no_ready_check': true,
+        'retry_strategy': require('./util/kvstore').retryStrategy(this)
+      }
     };
     if (this.get('redis password')) {
       sessionStoreArgs.pass = this.get('redis password');

lib/util/kvstore.js

@@ -1,42 +1,45 @@
 'use strict';
 
+const ApplicationError = require('./application_error');
 const redis = require('redis');
-const events = require('events');
+
+function retryStrategy(ravelInstance) {
+  return function(options) {
+    if (options.error.code === 'ECONNREFUSED') {
+      // End reconnecting on a specific error and flush all commands with a individual error
+      ravelInstance.log.error(`Lost connection to redis: ${options.error.code}.`);
+      return new ApplicationError.General(`Lost connection to redis: ${options.error.code}.`);
+    } else if (options.attempt > ravelInstance.get('redis max retries')) {
+      ravelInstance.log.error(`Lost connection to redis: ${options.error.code}. Max retry attempts exceeded.`);
+      // End reconnecting with built in error
+      return new ApplicationError.General(
+        `Lost connection to redis: ${options.error.code}. Max retry attempts reached.`);
+    } else  {
+      const time = Math.pow(options.attempt, 2)*100;
+      ravelInstance.log.error(`Lost connection to redis: ${options.error.code}. Reconnecting in ${time} milliseconds.`);
+      // reconnect after
+      return time;
+    }
+  };
+};
 
 /**
  * Abstraction for redis-like data store
- * Handles reconnections gracefully - something that node-redis claims to do but doesn't, actually.
- *
- * With help from https://www.exratione.com/2013/01/nodejs-connections-will-end-close-and-otherwise-blow-up/
  */
 module.exports = function(ravelInstance) {
 
-  const client = {};
-
-  const kvstore = new events.EventEmitter();
-
-  function replace(first) {
-    if (first !== true) {
-      kvstore.emit('replace');
-    }
-    if (client.end) {
-      client.closing = true;
-      client.end();
-    }
-    const newClient = redis.createClient(ravelInstance.get('redis port'), ravelInstance.get('redis host'), {
-      'no_ready_check': true
+  const client = redis.createClient(
+    ravelInstance.get('redis port'),
+    ravelInstance.get('redis host'),
+    {
+      'no_ready_check': true,
+      'retry_strategy': retryStrategy(ravelInstance)
     });
-    if (ravelInstance.get('redis password')) {
-      newClient.auth(ravelInstance.get('redis password'));
-    }
-    newClient.once('end', replace);
-    newClient.once('error', replace);
-    //copy over redis methods to client reference
-    //TODO add key prefixes
-    Object.assign(client, newClient);
-    //TODO handle pub/sub
-  }
-  replace(true);
+  if (ravelInstance.get('redis password')) {
+    client.auth(ravelInstance.get('redis password'));
+  };
 
   return client;
 };
+
+module.exports.retryStrategy = retryStrategy;

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ravel",
-  "version": "0.11.8",
+  "version": "0.11.9",
   "author": "Sean McIntyre <[email protected]>",
   "description": "Ravel Rapid Application Development Framework",
   "keywords": [
@@ -70,14 +70,13 @@
     "sinon":"1.17.3",
     "sinon-chai":"2.8.0",
     "supertest":"1.2.0",
-    "eslint": "1.10.3",
+    "eslint": "2.8.0",
 
     "codeclimate-test-reporter": "0.3.1",
     "del": "2.2.0",
     "gulp": "3.9.1",
     "gulp-env": "0.4.0",
     "istanbul": "0.4.3",
-    "isparta": "4.0.0",
     "gulp-istanbul": "0.10.4",
     "gulp-eslint": "2.0.0",
     "gulp-load-plugins": "1.2.2",
@@ -88,7 +87,7 @@
     "gulp-sourcemaps": "1.6.0",
     "gulp-babel":"6.1.2",
     "babel":"6.5.2",
-    "babel-eslint": "5.0.1",
+    "babel-eslint": "6.0.4",
     "babel-plugin-transform-decorators-legacy": "1.3.4"
   }
 }

test/util/test-kvstore.js

@@ -4,9 +4,8 @@ const chai = require('chai');
 const expect = chai.expect;
 chai.use(require('chai-things'));
 const mockery = require('mockery');
-const sinon = require('sinon');
 
-let Ravel, kvstore, redisClientStub, redisMock;
+let Ravel, redisClientStub, redisMock;
 
 describe('Ravel', function() {
 
@@ -41,32 +40,35 @@ describe('Ravel', function() {
   });
 
   describe('util/kvstore', function() {
-
-    describe('reconnection', function() {
-      it('should seamlessly create a new redis client when an \'end\' event is received from the original', function(done) {
-        kvstore = require('../../lib/util/kvstore')(Ravel);
-        expect(kvstore).to.have.a.property('auth').that.is.a('function');
-
-        const origKvstoreAuth = kvstore.auth;
-        const spy = sinon.spy(redisMock, 'createClient');
-        //fake disconnection
-        redisClientStub.emit('end');
-        expect(spy).to.have.been.calledOnce;
-        expect(kvstore.auth).to.not.equal(origKvstoreAuth);
+    describe('retryStrategy', function() {
+      it('should return an error when redis refuses the connection', function(done) {
+        const retryStrategy = require('../../lib/util/kvstore').retryStrategy(Ravel);
+        expect(retryStrategy).to.be.a('function');
+        expect(retryStrategy({error:{code:'ECONNREFUSED'}})).to.be.an.instanceof(Ravel.ApplicationError.General);
         done();
       });
 
-      it('should support auth', function(done) {
-        Ravel.set('redis password', 'password');
-        kvstore = require('../../lib/util/kvstore')(Ravel);
-        expect(kvstore).to.have.a.property('auth').that.is.a('function');
+      it('should return an error when the maximum number of retries is exceeded', function(done) {
+        const retryStrategy = require('../../lib/util/kvstore').retryStrategy(Ravel);
+        expect(retryStrategy).to.be.a('function');
+        Ravel.set('redis max retries', 10);
+        const options = {
+          error:{code:'something'},
+          attempt: Ravel.get('redis max retries') + 1
+        };
+        expect(retryStrategy(options)).to.be.an.instanceof(Ravel.ApplicationError.General);
+        done();
+      });
 
-        const origKvstoreAuth = kvstore.auth;
-        const spy = sinon.spy(redisMock, 'createClient');
-        //fake disconnection
-        redisClientStub.emit('end');
-        expect(spy).to.have.been.calledOnce;
-        expect(kvstore.auth).to.not.equal(origKvstoreAuth);
+      it('should return the time to the next reconnect if the number of retries does not exceed the maximum', function(done) {
+        const retryStrategy = require('../../lib/util/kvstore').retryStrategy(Ravel);
+        expect(retryStrategy).to.be.a('function');
+        Ravel.set('redis max retries', 10);
+        const options = {
+          error:{code:'something'},
+          attempt: 1
+        };
+        expect(retryStrategy(options)).to.equal(100);
         done();
       });
     });