From 66fca2eaba5cf834e4b3b20b9b0028d52523e73c Mon Sep 17 00:00:00 2001 From: edeustace Date: Tue, 16 Jun 2015 23:06:37 +0100 Subject: [PATCH 1/4] add interval option to retry --- README.md | 3 ++- lib/async.js | 62 ++++++++++++++++++++++++++++++++++++++++------ test/test-async.js | 22 ++++++++++++++++ 3 files changed, 78 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 2fa8f46fb..4253f8c8f 100644 --- a/README.md +++ b/README.md @@ -1459,7 +1459,7 @@ new tasks much easier (and the code more readable). --------------------------------------- -### retry([times = 5], task, [callback]) +### retry([times = 5], task, [callback, interval = 0]) Attempts to get a successful response from `task` no more than `times` times before returning an error. If the task is successful, the `callback` will be passed the result @@ -1475,6 +1475,7 @@ __Arguments__ the previously executed functions (if nested inside another control flow). * `callback(err, results)` - An optional callback which is called when the task has succeeded, or after the final failed attempt. It receives the `err` and `result` arguments of the last attempt at completing the `task`. +* `interval` - How long to wait before making another attempt. Defaults to 0 (aka don't wait). The [`retry`](#retry) function can be used as a stand-alone control flow by passing a callback, as shown below: diff --git a/lib/async.js b/lib/async.js index 6a1edb663..6df72d04b 100644 --- a/lib/async.js +++ b/lib/async.js @@ -601,17 +601,49 @@ }); }; - async.retry = function(times, task, callback) { + + async.retry = function(/*[times,] task [, callback, interval]*/) { var DEFAULT_TIMES = 5; + var DEFAULT_INTERVAL = 0; + var attempts = []; - // Use defaults if times not passed - if (typeof times === 'function') { - callback = task; - task = times; - times = DEFAULT_TIMES; + + var times = DEFAULT_TIMES; + var interval = DEFAULT_INTERVAL; + var task; + var callback; + + switch(arguments.length){ + case 1: { + task = arguments[0]; + break; + } + case 2 : { + task = arguments[0]; + callback = arguments[1]; + break; + } + case 3: { + times = arguments[0]; + task = arguments[1]; + callback = arguments[2]; + break; + } + case 4: { + times = arguments[0]; + task = arguments[1]; + callback = arguments[2]; + interval = arguments[3]; + break; + } + default: { + throw new Error('Invalid arguments - must be either (task), (task, callback), (times, task, callback) or (times, task, callback, interval)'); + } } - // Make sure times is a number + + // Make sure times and interval are numbers times = parseInt(times, 10) || DEFAULT_TIMES; + interval = parseInt(interval, 10) || DEFAULT_INTERVAL; function wrappedTask(wrappedCallback, wrappedResults) { function retryAttempt(task, finalAttempt) { @@ -622,8 +654,22 @@ }; } + function retryInterval(interval){ + return function(seriesCallback){ + setTimeout(function(){ + seriesCallback(null); + }, interval); + }; + } + while (times) { - attempts.push(retryAttempt(task, !(times-=1))); + + var finalAttempt = !(times-=1); + attempts.push(retryAttempt(task, finalAttempt)); + if(!finalAttempt && interval > 0){ + attempts.push(retryInterval(interval)); + } + } async.series(attempts, function(done, data){ data = data[data.length - 1]; diff --git a/test/test-async.js b/test/test-async.js index b388b9cd6..b74e84f8e 100755 --- a/test/test-async.js +++ b/test/test-async.js @@ -728,6 +728,28 @@ exports['retry when all attempts succeeds'] = function(test) { }); }; +exports['retry with interval when all attempts succeeds'] = function(test) { + var times = 3; + var interval = 500; + var callCount = 0; + var error = 'ERROR'; + var erroredResult = 'RESULT'; + function fn(callback) { + callCount++; + callback(error + callCount, erroredResult + callCount); // respond with indexed values + } + var start = new Date().getTime(); + async.retry(times, fn, function(err, result){ + var now = new Date().getTime(); + var duration = now - start; + test.ok(duration > (interval * (times -1)), 'did not include interval'); + test.equal(callCount, 3, "did not retry the correct number of times"); + test.equal(err, error + times, "Incorrect error was returned"); + test.equal(result, erroredResult + times, "Incorrect result was returned"); + test.done(); + }, interval); +}; + exports['retry as an embedded task'] = function(test) { var retryResult = 'RETRY'; var fooResults; From 41ff5496cac6e10bbaabf67856fa40111429d2b8 Mon Sep 17 00:00:00 2001 From: edeustace Date: Tue, 16 Jun 2015 23:13:25 +0100 Subject: [PATCH 2/4] add milliseconds --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 4253f8c8f..947d30ff6 100644 --- a/README.md +++ b/README.md @@ -62,12 +62,12 @@ This can also arise by accident if you callback early in certain cases: ```js async.eachSeries(hugeArray, function iterator(item, callback) { if (inCache(item)) { - callback(null, cache[item]); // if many items are cached, you'll overflow + callback(null, cache[item]); // if many items are cached, you'll overflow } else { doSomeIO(item, callback); } -}, function done() { - //... +}, function done() { + //... }); ``` @@ -1475,7 +1475,7 @@ __Arguments__ the previously executed functions (if nested inside another control flow). * `callback(err, results)` - An optional callback which is called when the task has succeeded, or after the final failed attempt. It receives the `err` and `result` arguments of the last attempt at completing the `task`. -* `interval` - How long to wait before making another attempt. Defaults to 0 (aka don't wait). +* `interval` - How long to wait in milliseconds before making another attempt. Defaults to 0 (aka don't wait). The [`retry`](#retry) function can be used as a stand-alone control flow by passing a callback, as shown below: @@ -1647,7 +1647,7 @@ async.times(5, function(n, next){ ### timesSeries(n, iterator, [callback]) -The same as [`times`](#times), only the iterator is applied in series. +The same as [`times`](#times), only the iterator is applied in series. The next `iterator` is only called once the current one has completed. The results array will be in the same order as the original. @@ -1728,9 +1728,9 @@ function sometimesAsync(arg, callback) { } // this has a risk of stack overflows if many results are cached in a row -async.mapSeries(args, sometimesAsync, done); +async.mapSeries(args, sometimesAsync, done); -// this will defer sometimesAsync's callback if necessary, +// this will defer sometimesAsync's callback if necessary, // preventing stack overflows async.mapSeries(args, async.ensureAsync(sometimesAsync), done); From dac52c832159179967cc4252dd2bd7db37d65682 Mon Sep 17 00:00:00 2001 From: edeustace Date: Wed, 17 Jun 2015 11:46:38 +0100 Subject: [PATCH 3/4] move interval to options object that may be passed in as the 1st parameter --- README.md | 12 +++++-- lib/async.js | 88 +++++++++++++++++++++++++--------------------- test/test-async.js | 22 ++++++++++-- 3 files changed, 76 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 947d30ff6..392f06dde 100644 --- a/README.md +++ b/README.md @@ -1459,7 +1459,7 @@ new tasks much easier (and the code more readable). --------------------------------------- -### retry([times = 5], task, [callback, interval = 0]) +### retry([opts = {times: 5, interval: 0}| 5], task, [callback]) Attempts to get a successful response from `task` no more than `times` times before returning an error. If the task is successful, the `callback` will be passed the result @@ -1468,14 +1468,14 @@ result (if any) of the final attempt. __Arguments__ -* `times` - An integer indicating how many times to attempt the `task` before giving up. Defaults to 5. +* `opts` - Can be either an object with `times` and `interval` or a number. `times` is how many attempts should be made before giving up. `interval` is how long to wait inbetween attempts. Defaults to {times: 5, interval: 0} + * if a number is passed in it sets `times` only (with `interval` defaulting to 0). * `task(callback, results)` - A function which receives two arguments: (1) a `callback(err, result)` which must be called when finished, passing `err` (which can be `null`) and the `result` of the function's execution, and (2) a `results` object, containing the results of the previously executed functions (if nested inside another control flow). * `callback(err, results)` - An optional callback which is called when the task has succeeded, or after the final failed attempt. It receives the `err` and `result` arguments of the last attempt at completing the `task`. -* `interval` - How long to wait in milliseconds before making another attempt. Defaults to 0 (aka don't wait). The [`retry`](#retry) function can be used as a stand-alone control flow by passing a callback, as shown below: @@ -1486,6 +1486,12 @@ async.retry(3, apiMethod, function(err, result) { }); ``` +```js +async.retry({times: 3, interval: 200}, apiMethod, function(err, result) { + // do something with the result +}); +``` + It can also be embeded within other control flow functions to retry individual methods that are not as reliable, like this: diff --git a/lib/async.js b/lib/async.js index 6df72d04b..de50c7676 100644 --- a/lib/async.js +++ b/lib/async.js @@ -602,48 +602,54 @@ }; - async.retry = function(/*[times,] task [, callback, interval]*/) { + + async.retry = function(/*[times,] task [, callback]*/) { var DEFAULT_TIMES = 5; var DEFAULT_INTERVAL = 0; var attempts = []; - var times = DEFAULT_TIMES; - var interval = DEFAULT_INTERVAL; - var task; - var callback; + var opts = { + times: DEFAULT_TIMES, + interval: DEFAULT_INTERVAL + }; - switch(arguments.length){ - case 1: { - task = arguments[0]; - break; - } - case 2 : { - task = arguments[0]; - callback = arguments[1]; - break; - } - case 3: { - times = arguments[0]; - task = arguments[1]; - callback = arguments[2]; - break; - } - case 4: { - times = arguments[0]; - task = arguments[1]; - callback = arguments[2]; - interval = arguments[3]; - break; - } - default: { - throw new Error('Invalid arguments - must be either (task), (task, callback), (times, task, callback) or (times, task, callback, interval)'); + function parseTimes(acc, t){ + if(typeof t === 'number'){ + acc.times = parseInt(t, 10) || DEFAULT_TIMES; + } else if(typeof t === 'object'){ + acc.times = parseInt(t.times, 10) || DEFAULT_TIMES; + acc.interval = parseInt(t.interval, 10) || DEFAULT_INTERVAL; + } else { + throw new Error('Unsupported argument type for \'times\': ' + typeof(t)); } } - // Make sure times and interval are numbers - times = parseInt(times, 10) || DEFAULT_TIMES; - interval = parseInt(interval, 10) || DEFAULT_INTERVAL; + switch(arguments.length){ + case 1: { + opts.task = arguments[0]; + break; + } + case 2 : { + if(typeof arguments[0] === 'number' || typeof arguments[0] === 'object'){ + parseTimes(opts, arguments[0]); + opts.task = arguments[1]; + } else { + opts.task = arguments[0]; + opts.callback = arguments[1]; + } + break; + } + case 3: { + parseTimes(opts, arguments[0]); + opts.task = arguments[1]; + opts.callback = arguments[2]; + break; + } + default: { + throw new Error('Invalid arguments - must be either (task), (task, callback), (times, task) or (times, task, callback)'); + } + } function wrappedTask(wrappedCallback, wrappedResults) { function retryAttempt(task, finalAttempt) { @@ -662,23 +668,23 @@ }; } - while (times) { + while (opts.times) { - var finalAttempt = !(times-=1); - attempts.push(retryAttempt(task, finalAttempt)); - if(!finalAttempt && interval > 0){ - attempts.push(retryInterval(interval)); + var finalAttempt = !(opts.times-=1); + attempts.push(retryAttempt(opts.task, finalAttempt)); + if(!finalAttempt && opts.interval > 0){ + attempts.push(retryInterval(opts.interval)); } - } + async.series(attempts, function(done, data){ data = data[data.length - 1]; - (wrappedCallback || callback)(data.err, data.result); + (wrappedCallback || opts.callback)(data.err, data.result); }); } // If a callback is passed, run this as a controll flow - return callback ? wrappedTask() : wrappedTask; + return opts.callback ? wrappedTask() : wrappedTask; }; async.waterfall = function (tasks, callback) { diff --git a/test/test-async.js b/test/test-async.js index b74e84f8e..5b6f80324 100755 --- a/test/test-async.js +++ b/test/test-async.js @@ -739,7 +739,7 @@ exports['retry with interval when all attempts succeeds'] = function(test) { callback(error + callCount, erroredResult + callCount); // respond with indexed values } var start = new Date().getTime(); - async.retry(times, fn, function(err, result){ + async.retry({ times: times, interval: interval}, fn, function(err, result){ var now = new Date().getTime(); var duration = now - start; test.ok(duration > (interval * (times -1)), 'did not include interval'); @@ -747,7 +747,7 @@ exports['retry with interval when all attempts succeeds'] = function(test) { test.equal(err, error + times, "Incorrect error was returned"); test.equal(result, erroredResult + times, "Incorrect result was returned"); test.done(); - }, interval); + }); }; exports['retry as an embedded task'] = function(test) { @@ -771,6 +771,24 @@ exports['retry as an embedded task'] = function(test) { }); }; +exports['retry as an embedded task with interval'] = function(test) { + var start = new Date().getTime(); + var opts = {times: 5, interval: 100}; + + async.auto({ + foo: function(callback){ + callback(null, 'FOO'); + }, + retry: async.retry(opts, function(callback) { + callback('err'); + }) + }, function(){ + var duration = new Date().getTime() - start; + test.ok(duration > ((opts.times -1) * opts.interval), "The duration should have been greater than ((times -1) * interval)"); + test.done(); + }); +}; + exports['waterfall'] = { 'basic': function(test){ From 33b06ac7418eafed4c15c7202a7c93a65325dd31 Mon Sep 17 00:00:00 2001 From: Ed Eustace Date: Wed, 17 Jun 2015 14:09:00 +0100 Subject: [PATCH 4/4] add durations to failure message --- test/test-async.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test-async.js b/test/test-async.js index 5b6f80324..8b2f87fa1 100755 --- a/test/test-async.js +++ b/test/test-async.js @@ -784,7 +784,8 @@ exports['retry as an embedded task with interval'] = function(test) { }) }, function(){ var duration = new Date().getTime() - start; - test.ok(duration > ((opts.times -1) * opts.interval), "The duration should have been greater than ((times -1) * interval)"); + var expectedMinimumDuration = (opts.times -1) * opts.interval; + test.ok(duration >= expectedMinimumDuration, "The duration should have been greater than " + expectedMinimumDuration + ", but was " + duration); test.done(); }); };