From 67c4759627dea46a9938e20881ec7fbd09ca0d99 Mon Sep 17 00:00:00 2001 From: Baptiste Mathus Date: Fri, 14 Sep 2018 13:16:10 +0200 Subject: [PATCH 1/5] Add simplistic exponential backoff retry Introducing options.delay and options.factor. Now, by default even without specifying options.delay, there's a default delay of 100ms between retries in case of at least one failure. I think it makes sense as a default behavior, and anyway users can override this if needed. --- index.js | 11 ++++++++++- test/index.test.js | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 72356bb..b8b87ba 100644 --- a/index.js +++ b/index.js @@ -22,7 +22,16 @@ class rpRetry { logger.info(`Encountered error ${err.message} for ${options.method} request to ${options.uri}, retry count ${tryCount}`); tryCount -= 1; if (tryCount) { - return fetchDataWithRetry(tryCount); + let delay = options.delay || 100; // default delay between retries + if (options.factor) { + delay *= options.factor; + } + return new Promise((resolve, reject) => { + setTimeout(() => { + logger.debug(`waiting for ${delay} ms before next retry for ${options.uri}`); + resolve(fetchDataWithRetry(tryCount)); + }, delay); + }); } return Promise.reject(err); }); diff --git a/test/index.test.js b/test/index.test.js index 4e14a55..0730bbf 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -102,4 +102,18 @@ describe('request-promise-retry', function () { expect(data.error).equal(undefined); }); }); + it('should pass, retry with exponential backoff', () => { + // failure should take a bit of time to happen + const startTime = new Date(); + return rp({ + uri: 'http://adadadadad.com/', + method: 'GET', + retry: 5, + delay: 200, + factor: 1.1 // + }) + .catch(error => { + expect(new Date() - startTime).to.be.above((5 - 1) * 200); + }); + }); }); From 48ba75251226dca955adec0cf0a436d667f371d5 Mon Sep 17 00:00:00 2001 From: Baptiste Mathus Date: Fri, 14 Sep 2018 17:21:34 +0200 Subject: [PATCH 2/5] Clarify unit --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index b8b87ba..e11a11e 100644 --- a/index.js +++ b/index.js @@ -22,7 +22,7 @@ class rpRetry { logger.info(`Encountered error ${err.message} for ${options.method} request to ${options.uri}, retry count ${tryCount}`); tryCount -= 1; if (tryCount) { - let delay = options.delay || 100; // default delay between retries + let delay = options.delay || 100; // default ms delay between retries if (options.factor) { delay *= options.factor; } From dbaddc668cc24aa53a41e655a0d39038b995f597 Mon Sep 17 00:00:00 2001 From: Baptiste Mathus Date: Sat, 15 Sep 2018 22:52:41 +0200 Subject: [PATCH 3/5] Make algo *actually* exponential --- index.js | 27 ++++++++++++++++----------- test/index.test.js | 8 ++++---- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index e11a11e..25dbc76 100644 --- a/index.js +++ b/index.js @@ -5,12 +5,21 @@ const logger = require('./modules/logger')('request-promise-retry'); class rpRetry { static _rpRetry(options) { - if(options.verbose_logging) { - logger.info(`calling ${options.uri} with retry ${options.retry}`); - } + const tries = options.retry || 1; delete options.retry; - const fetchDataWithRetry = tryCount => { + + const delay = options.delay || 100; // default ms delay between retries + delete options.delay; + + const factor = options.factor || 1; // If absent, delay will always be the same. + delete options.factor; + + if(options.verbose_logging) { + logger.info(`calling ${options.uri} with retry ${tries}, initial delay=${delay}, factor=${factor}`); + } + + const fetchDataWithRetry = (tryCount, delay) => { return requestPromise(options) .then(result => { if(options.verbose_logging) { @@ -22,21 +31,17 @@ class rpRetry { logger.info(`Encountered error ${err.message} for ${options.method} request to ${options.uri}, retry count ${tryCount}`); tryCount -= 1; if (tryCount) { - let delay = options.delay || 100; // default ms delay between retries - if (options.factor) { - delay *= options.factor; - } return new Promise((resolve, reject) => { setTimeout(() => { - logger.debug(`waiting for ${delay} ms before next retry for ${options.uri}`); - resolve(fetchDataWithRetry(tryCount)); + logger.debug(`waiting for ${delay} ms before next retry for ${options.uri}. Next wait ${delay*factor}`); + resolve(fetchDataWithRetry(tryCount, delay*factor)); }, delay); }); } return Promise.reject(err); }); }; - return fetchDataWithRetry(tries); + return fetchDataWithRetry(tries, delay); } static _rp(options) { diff --git a/test/index.test.js b/test/index.test.js index 0730bbf..bb5da37 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -108,12 +108,12 @@ describe('request-promise-retry', function () { return rp({ uri: 'http://adadadadad.com/', method: 'GET', - retry: 5, - delay: 200, - factor: 1.1 // + retry: 4, + delay: 30, + factor: 10 // 0 + 30 + 300 + 3000 }) .catch(error => { - expect(new Date() - startTime).to.be.above((5 - 1) * 200); + expect(new Date() - startTime).to.be.above(0 + 30 + 300 + 3000); }); }); }); From 91aa4ac34372a5f3a2930877aa1ae33848220b79 Mon Sep 17 00:00:00 2001 From: Baptiste Mathus Date: Mon, 17 Sep 2018 14:41:40 +0200 Subject: [PATCH 4/5] Fix formatting issues --- index.js | 20 ++++++++++---------- package.json | 5 ++++- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index 25dbc76..430fb00 100644 --- a/index.js +++ b/index.js @@ -15,15 +15,15 @@ class rpRetry { const factor = options.factor || 1; // If absent, delay will always be the same. delete options.factor; - if(options.verbose_logging) { - logger.info(`calling ${options.uri} with retry ${tries}, initial delay=${delay}, factor=${factor}`); + if (options.verbose_logging) { + logger.info(`calling ${options.uri} with retry ${tries}, initial delay=${delay}, factor=${factor}`); } const fetchDataWithRetry = (tryCount, delay) => { return requestPromise(options) .then(result => { - if(options.verbose_logging) { - logger.info(`Result obtained for ${options.method} request to ${options.uri}`); + if (options.verbose_logging) { + logger.info(`Result obtained for ${options.method} request to ${options.uri}`); } return Promise.resolve(result); }) @@ -33,8 +33,8 @@ class rpRetry { if (tryCount) { return new Promise((resolve, reject) => { setTimeout(() => { - logger.debug(`waiting for ${delay} ms before next retry for ${options.uri}. Next wait ${delay*factor}`); - resolve(fetchDataWithRetry(tryCount, delay*factor)); + logger.debug(`waiting for ${delay} ms before next retry for ${options.uri}. Next wait ${delay * factor}`); + resolve(fetchDataWithRetry(tryCount, delay * factor)); }, delay); }); } @@ -45,13 +45,13 @@ class rpRetry { } static _rp(options) { - if(options.verbose_logging) { - logger.info(`calling ${options.uri} without retries`); + if (options.verbose_logging) { + logger.info(`calling ${options.uri} without retries`); } return requestPromise(options) .then(result => { - if(options.verbose_logging) { - logger.info(`Result obtained for ${options.method} request to ${options.uri}`); + if (options.verbose_logging) { + logger.info(`Result obtained for ${options.method} request to ${options.uri}`); } return Promise.resolve(result); }) diff --git a/package.json b/package.json index d224838..2b77abb 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,9 @@ "main": "index.js", "scripts": { "test": "node_modules/gulp/bin/gulp.js test", - "coveralls": "cat ./coverage/lcov.info | ./node_modules/.bin/coveralls" + "coveralls": "cat ./coverage/lcov.info | ./node_modules/.bin/coveralls", + "eslint": "eslint test/. index.js", + "eslint-fix": "eslint test/. index.js --fix" }, "repository": { "type": "git", @@ -45,6 +47,7 @@ "chai": "^4.1.2", "chai-subset": "^1.6.0", "coveralls": "^3.0.0", + "eslint": "^5.5.0", "gulp": "^3.9.1", "gulp-eslint": "^4.0.0", "gulp-if": "^2.0.2", From f27c79f8ca86052fbfcc337237afbd4f094abb64 Mon Sep 17 00:00:00 2001 From: Sushim Dutta Date: Thu, 31 Oct 2019 12:25:30 +0530 Subject: [PATCH 5/5] - updated readme with delay and factor parameters --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index ebc8beb..773443c 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,8 @@ var options = { retry : 2, // will retry the call twice, in case of error. verbose_logging : false, // will log errors only, if set to be true, will log all actions accepted: [ 400, 404 ] // Accepted HTTP Status codes (will not retry if request response has any of these HTTP Status Code) + delay: 2000 // will delay retries by 2000 ms. The default is 100. + factor: 2 // will multiple the delay by the factor each time a retry is attempted. }; rp(options)