Merge pull request #33 from Joneser/master

Add retry logic to requests

README.md

@@ -109,6 +109,45 @@ const catWatch = new ServiceClient({
 });
 ```
 
+## Retry Logic
+
+For application critical requests it can be a good idea to retry failed requests to the responsible services.
+
+Occasionaly target server can have high latency for a short period of time, or in the case of a stack of servers, one server can be having issues
+and retrying the request will allow perron to attempt to access one of the other servers that currently aren't facing issues.
+
+By default `perron` has retry logic implemented, but configured to perform 0 retries. Internally `perron` uses [node-retry](https://github.com/tim-kos/node-retry) to handle the retry logic
+and configuration. All of the existing options provided by `node-retry` can be passed via configuration options through `perron`.
+
+There is a shouldRetry function which can be defined in any way by the consumer and is used in the try logic to determine whether to attempt the retries or not depending on the type of error and the original request object. 
+If the function returns true and the number of retries hasn't been exceeded, the request can be retried.
+
+There is also an onRetry function which can be defined by the user of `perron`. This function is called every time a retry request will be triggered.
+It is provided the currentAttempt, as well as the error that is causing the retry.
+
+The first time onRetry gets called, the value of currentAttempt will be 2. This is because the first initial request is counted as the first attempt, and the first retry attempted will then be the second request.
+
+```js
+const ServiceClient = require('perron');
+
+const catWatch = new ServiceClient({
+    hostname: 'catwatch.opensource.zalan.do',
+    retryOptions: {
+        retries: 1,
+        factor: 2,
+        minTimeout: 200,
+        maxTimeout: 400,
+        randomize: true,
+        shouldRetry(err, req) {
+            return (err && err.response && err.response.statusCode >= 500);
+        },
+        onRetry(currentAttempt, err) {
+            console.log('Retry attempt #' + currentAttempt + 'due to ' + err);
+        }
+    }
+});
+```
+
 ## Filters
 
 It's quite often necessary to do some pre- or post-processing of the request. For this purpose `perron` implements a concept of filters, that are just an object with 2 optional methods: `request` and `response`.

lib/client.js

@@ -3,6 +3,7 @@
 const request = require('./request');
 const url = require('url');
 const CircuitBreaker = require('circuit-breaker-js');
+const retry = require('retry');
 
 /**
  * @typedef {Object.<string, (string|Array.<string>)>} ServiceClientHeaders
@@ -21,6 +22,15 @@ const CircuitBreaker = require('circuit-breaker-js');
  *  service: string,
  *  filters?: Array.<ServiceClient~requestFilter>,
  *  timing?: boolean,
+ *  retryOptions?: {
+ *      retries?: number,
+ *      factor?: number,
+ *      minTimeout?: number,
+ *      maxTimeout?: number,
+ *      randomize?: boolean,
+ *      shouldRetry?: Function,
+ *      onRetry?: Function
+ *  },
  *  circuitBreaker?: (false|{
  *      windowDuration?: number,
  *      numBuckets?: number,
@@ -216,6 +226,24 @@ class ServiceClient {
             timeout: DEFAULT_REQUEST_TIMEOUT
         }, this.options.defaultRequestOptions);
 
+        this.options.retryOptions = Object.assign({
+            retries: 0,
+            factor: 2,
+            minTimeout: 200,
+            maxTimeout: 400,
+            randomize: true,
+            // eslint-disable-next-line
+            shouldRetry(err, req) {
+                return true;
+            },
+            // eslint-disable-next-line
+            onRetry(currentAttempt, err) {}
+        }, this.options.retryOptions);
+
+        if (this.options.retryOptions.minTimeout > this.options.retryOptions.maxTimeout) {
+            throw new Error('The `maxTimeout` must be equal to or greater than the `minTimeout`');
+        }
+
         if (this.options.circuitBreaker) {
             const breakerOptions = Object.assign({
                 windowDuration: 10000,
@@ -246,20 +274,50 @@ class ServiceClient {
             accept: 'application/json'
         }, params.headers);
 
-        return new Promise((resolve, reject) => this.breaker.run(
-            (success, failure) => {
+        const {
+            retries,
+            factor,
+            minTimeout,
+            maxTimeout,
+            randomize,
+            shouldRetry,
+            onRetry
+        } = this.options.retryOptions;
+
+        const opts = {
+            retries,
+            factor,
+            minTimeout,
+            maxTimeout,
+            randomize
+        };
+
+        const operation = retry.operation(opts);
+
+        return new Promise((resolve, reject) => operation.attempt((currentAttempt) => {
+            this.breaker.run((success, failure) => {
                 return requestWithFilters(params, this.options.filters)
                     .then(result => {
-                        success(); resolve(result);
+                        success();
+                        resolve(result);
                     })
                     .catch(error => {
-                        failure(); reject(error);
+                        failure();
+                        if (!shouldRetry(error, params)) {
+                            reject(error);
+                            return;
+                        }
+                        if (!operation.retry(error)) {
+                            reject(error);
+                            return;
+                        }
+                        onRetry(currentAttempt + 1, error);
                     });
             },
             () => {
                 reject(new ServiceClient.Error({}, ServiceClient.CIRCUIT_OPEN));
-            }
-        ));
+            });
+        }));
     }
 }
 

package.json

@@ -25,6 +25,7 @@
     "sinon": "3.3.0"
   },
   "dependencies": {
-    "circuit-breaker-js": "0.0.1"
+    "circuit-breaker-js": "0.0.1",
+    "retry": "0.10.1"
   }
 }

test/client.test.js

@@ -396,4 +396,92 @@ describe('ServiceClient', () => {
             });
         });
     });
+
+    it('should perform the desired number of retries based on the configuration', (done) => {
+        let numberOfRetries = 0;
+        clientOptions.retryOptions = {
+            retries: 3,
+            onRetry() {
+                numberOfRetries += 1;
+            }
+        };
+        const client = new ServiceClient(clientOptions);
+        requestStub.returns(Promise.resolve({
+            statusCode: 501,
+            headers: {},
+            body: '{}'
+        }));
+        client.request().catch(err => {
+            assert.equal(numberOfRetries, 3);
+            assert.equal(err instanceof ServiceClient.Error, true);
+            assert.equal(err.type, 'Response filter marked request as failed');
+            done();
+        });
+    });
+
+    it('should open the circuit after 50% from 11 requests failed and correct number of retries were performed', (done) => {
+        const httpErrorResponse = Promise.resolve({
+            statusCode: 500,
+            headers: {},
+            body: '{}'
+        });
+        let numberOfRetries = 0;
+        clientOptions.retryOptions = {
+            retries: 1,
+            onRetry() {
+                numberOfRetries += 1;
+            }
+        };
+        const errorResponse = Promise.resolve(Promise.reject(new Error('timeout')));
+        const requests = Array.from({ length: 11 });
+
+        [   emptySuccessResponse, emptySuccessResponse, errorResponse, emptySuccessResponse, httpErrorResponse, errorResponse,
+            httpErrorResponse, emptySuccessResponse, errorResponse, httpErrorResponse, emptySuccessResponse
+        ].forEach((response, index) => {
+            requestStub.onCall(index).returns(response);
+        });
+
+        const client = new ServiceClient(clientOptions);
+        requests.reduce((promise) => {
+            const tick = () => {
+                return client.request();
+            };
+            return promise.then(tick, tick);
+        }, Promise.resolve()).then(() => {
+            return client.request();
+        }).catch((err) => {
+            assert.equal(numberOfRetries, 4);
+            assert.equal(err instanceof ServiceClient.Error, true);
+            assert.equal(err.type, ServiceClient.CIRCUIT_OPEN);
+            done();
+        });
+    });
+
+    it('should not retry if the shouldRetry function returns false', (done) => {
+        let numberOfRetries = 0;
+        clientOptions.retryOptions = {
+            retries: 1,
+            shouldRetry(err) {
+                if (err.response.statusCode === 501) {
+                    return false;
+                }
+                return true;
+            },
+            onRetry() {
+                numberOfRetries += 1;
+            }
+        };
+        const client = new ServiceClient(clientOptions);
+        requestStub.returns(Promise.resolve({
+            statusCode: 501,
+            headers: {},
+            body: '{}'
+        }));
+        client.request().catch(err => {
+            assert.equal(numberOfRetries, 0);
+            assert.equal(err instanceof ServiceClient.Error, true);
+            assert.equal(err.type, 'Response filter marked request as failed');
+            done();
+        });
+    });
 });