Skip to content

Commit

Permalink
fix: correctly calculate retry attempt (#33)
Browse files Browse the repository at this point in the history
  • Loading branch information
stephenplusplus committed Jul 31, 2020
1 parent 6f69c95 commit 4c852e2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 14 deletions.
16 changes: 5 additions & 11 deletions .circleci/config.yml
@@ -1,4 +1,4 @@
version: 3
version: 2.1

install_and_test: &install_and_test
steps:
Expand All @@ -11,23 +11,17 @@ install_and_test: &install_and_test
command: npm test

jobs:
test_node8:
docker:
- image: node:8
<<: *install_and_test
test_node10:
docker:
- image: node:10
- image: circleci/node:10
<<: *install_and_test
test_node11:
test_node12:
docker:
- image: node:11
- image: circleci/node:12
<<: *install_and_test

workflows:
version: 3
test:
jobs:
- test_node8
- test_node10
- test_node11
- test_node12
12 changes: 10 additions & 2 deletions index.js
Expand Up @@ -57,9 +57,12 @@ function retryRequest(requestOpts, opts, callback) {
if (typeof opts.retries !== 'number') {
opts.retries = DEFAULTS.retries;
}
if (typeof opts.currentRetryAttempt !== 'number') {

var manualCurrentRetryAttemptWasSet = typeof opts.currentRetryAttempt === 'number';
if (!manualCurrentRetryAttemptWasSet) {
opts.currentRetryAttempt = DEFAULTS.currentRetryAttempt;
}

if (typeof opts.noResponseRetries !== 'number') {
opts.noResponseRetries = DEFAULTS.noResponseRetries;
}
Expand Down Expand Up @@ -190,7 +193,12 @@ function retryRequest(requestOpts, opts, callback) {
}

// Send the response to see if we should try again.
if (currentRetryAttempt <= opts.retries && opts.shouldRetryFn(response)) {
// NOTE: "currentRetryAttempt" isn't accurate by default, as it counts
// the very first request sent as the first "retry". It is only accurate
// when a user provides their own "currentRetryAttempt" option at
// instantiation.
var adjustedCurrentRetryAttempt = manualCurrentRetryAttemptWasSet ? currentRetryAttempt : currentRetryAttempt - 1;
if (adjustedCurrentRetryAttempt < opts.retries && opts.shouldRetryFn(response)) {
retryAfterDelay(currentRetryAttempt);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion test.js
Expand Up @@ -272,7 +272,7 @@ describe('retry-request', function () {
};

retryRequest(URI_404, opts, function (err) {
assert.strictEqual(numAttempts, 2);
assert.strictEqual(numAttempts, 1);
done();
});
});
Expand Down

0 comments on commit 4c852e2

Please sign in to comment.