Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom retry interval #1161

Merged
merged 4 commits into from
May 21, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions lib/retry.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,20 @@ export default function retry(times, task, callback) {
var DEFAULT_TIMES = 5;
var DEFAULT_INTERVAL = 0;


var opts = {
times: DEFAULT_TIMES,
interval: DEFAULT_INTERVAL
intervalFunc: constantIntervalFunc(DEFAULT_INTERVAL)
};

function parseTimes(acc, t) {
if (typeof t === 'object') {
acc.times = +t.times || DEFAULT_TIMES;
acc.interval = +t.interval || DEFAULT_INTERVAL;

if (typeof t.interval === 'function') {
acc.intervalFunc = t.interval;
} else {
acc.intervalFunc = constantIntervalFunc(+t.interval || DEFAULT_INTERVAL);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ternary this guy

} else if (typeof t === 'number' || typeof t === 'string') {
acc.times = +t || DEFAULT_TIMES;
} else {
Expand All @@ -95,13 +99,13 @@ export default function retry(times, task, callback) {
throw new Error("Invalid arguments for async.retry");
}


var attempts = [];
while (opts.times) {
var isFinalAttempt = !(opts.times -= 1);
for (var i = 1; i < opts.times + 1; i++) {
var isFinalAttempt = (i == opts.times);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change to this loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to pass the retryCount to the interval function but the loop was counting backwards and destroying opts.times.

attempts.push(retryAttempt(isFinalAttempt));
if (!isFinalAttempt && opts.interval > 0) {
attempts.push(retryInterval(opts.interval));
var interval = opts.intervalFunc(i);
if (!isFinalAttempt && interval > 0) {
attempts.push(retryInterval(interval));
}
}

Expand Down Expand Up @@ -129,4 +133,10 @@ export default function retry(times, task, callback) {
}, interval);
};
}

function constantIntervalFunc(interval) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you import constant from 'lodash/constant' instead

return function() {
return interval;
};
}
}
26 changes: 24 additions & 2 deletions mocha_test/retry.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe("retry", function () {
});
});

it('retry when all attempts succeeds',function(done) {
it('retry when all attempts fail',function(done) {
var times = 3;
var callCount = 0;
var error = 'ERROR';
Expand Down Expand Up @@ -53,7 +53,7 @@ describe("retry", function () {
done();
});

it('retry with interval when all attempts succeeds',function(done) {
it('retry with interval when all attempts fail',function(done) {
var times = 3;
var interval = 50;
var callCount = 0;
Expand All @@ -75,6 +75,28 @@ describe("retry", function () {
});
});

it('retry with custom interval when all attempts fail',function(done) {
var times = 3;
var intervalFunc = function(retryCount) { return retryCount * 100; };
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: intervalFunc}, fn, function(err, result){
var now = new Date().getTime();
var duration = now - start;
assert(duration >= 300, 'did not include custom interval');
assert.equal(callCount, 3, "did not retry the correct number of times");
assert.equal(err, error + times, "Incorrect error was returned");
assert.equal(result, erroredResult + times, "Incorrect result was returned");
done();
});
});

it("should not require a callback", function (done) {
var called = false;
async.retry(3, function(cb) {
Expand Down