Merge pull request #793 from ahum/feature/retry-add-optional-interval

Feature/retry add optional interval

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() {
+  //...
 });
 ```
 
@@ -1459,7 +1459,7 @@ new tasks much easier (and the code more readable).
 ---------------------------------------
 
 <a name="retry" />
-### retry([times = 5], task, [callback])
+### 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,7 +1468,8 @@ 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
@@ -1485,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:
 
@@ -1646,7 +1653,7 @@ async.times(5, function(n, next){
 <a name="timesSeries" />
 ### 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.
 
@@ -1727,9 +1734,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);
 

lib/async.js

@@ -601,17 +601,55 @@
         });
     };
 
-    async.retry = function(times, task, callback) {
+
+
+    async.retry = function(/*[times,] task [, callback]*/) {
         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 opts = {
+          times: DEFAULT_TIMES,
+          interval: DEFAULT_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 is a number
-        times = parseInt(times, 10) || DEFAULT_TIMES;
+
+        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) {
@@ -622,17 +660,31 @@
                 };
             }
 
-            while (times) {
-                attempts.push(retryAttempt(task, !(times-=1)));
+            function retryInterval(interval){
+              return function(seriesCallback){
+                setTimeout(function(){
+                  seriesCallback(null);
+                }, interval);
+              };
             }
+
+            while (opts.times) {
+
+                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) {

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: 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');
+        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();
+    });
+};
+
 exports['retry as an embedded task'] = function(test) {
     var retryResult = 'RETRY';
     var fooResults;
@@ -749,6 +771,25 @@ 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;
+        var expectedMinimumDuration = (opts.times -1) * opts.interval;
+        test.ok(duration >= expectedMinimumDuration, "The duration should have been greater than " + expectedMinimumDuration + ", but was " + duration);
+        test.done();
+    });
+};
+
 exports['waterfall'] = {
 
 'basic': function(test){