From 63aeab048bd6290a1511bf772a3e97b67de21ae0 Mon Sep 17 00:00:00 2001 From: Sean McIntyre Date: Thu, 28 Apr 2016 12:42:40 -0400 Subject: [PATCH] Using new node-redis reconnection stuff properly now. Adding in Ravel error logging when redis dies. --- .eslintrc.json | 2 +- lib/ravel.js | 10 +++++-- lib/util/kvstore.js | 61 ++++++++++++++++++++------------------- package.json | 7 ++--- test/util/test-kvstore.js | 50 +++++++++++++++++--------------- 5 files changed, 69 insertions(+), 61 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 04e297fc..4b068585 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -56,7 +56,7 @@ "single" ], "semi": [2, "always"], - "strict": [2, "global"], + "strict": [0, "global"], "valid-typeof": 2, "wrap-iife": [ 2, diff --git a/lib/ravel.js b/lib/ravel.js index 1883316f..55847a62 100644 --- a/lib/ravel.js +++ b/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'); diff --git a/lib/util/kvstore.js b/lib/util/kvstore.js index 8e2f25ea..88c9bdc9 100644 --- a/lib/util/kvstore.js +++ b/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; diff --git a/package.json b/package.json index 41cc320a..2c2a15b8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ravel", - "version": "0.11.8", + "version": "0.11.9", "author": "Sean McIntyre ", "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" } } diff --git a/test/util/test-kvstore.js b/test/util/test-kvstore.js index 367922ba..ed6d57ae 100644 --- a/test/util/test-kvstore.js +++ b/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(); }); });