fix: non-zero delay between first attempt and first retry for linear and exp strategy

package.json

@@ -8,7 +8,7 @@
   "unpkg": "dist/index.umd.js",
   "types": "dist/src/index.d.ts",
   "engines": {
-    "node": ">=10.0.0"
+    "node": ">=10.7.0"
   },
   "repository": {
     "type": "git",

src/index.ts

@@ -223,19 +223,42 @@ function onError(err: AxiosError) {
         return reject(err);
       }
     }
-    // Else calculate delay according to chosen strategy
+
+    // Now it's certain that a retry is supposed to happen. Incremenent the
+    // counter, critical for linear and exp backoff delay calc. Note that
+    // `config.currentRetryAttempt` is local to this function whereas
+    // `(err.config as RaxConfig).raxConfig` is state that is tranferred across
+    // retries. That is, we want to mutate `(err.config as
+    // RaxConfig).raxConfig`. Another important note is about the definition of
+    // `currentRetryAttempt`: When we are here becasue the first and actual
+    // HTTP request attempt failed then `currentRetryAttempt` is still zero. We
+    // have found that a retry is indeed required. Since that is (will be)
+    // indeed the first retry it makes sense to now increase
+    // `currentRetryAttempt` by 1. So that it is in fact 1 for the first retry
+    // (as opposed to 0 or 2); an intuitive convention to use for the math
+    // below.
+    (err.config as RaxConfig).raxConfig!.currentRetryAttempt! += 1;
+
+    // store with shorter and more expressive variable name.
+    const retrycount = (err.config as RaxConfig).raxConfig!
+      .currentRetryAttempt!;
+
+    // Calculate delay according to chosen strategy
     // Default to exponential backoff - formula: ((2^c - 1) / 2) * 1000
-    else {
+    if (delay === 0) {
+      // was not set by Retry-After logic
       if (config.backoffType === 'linear') {
-        delay = config.currentRetryAttempt! * 1000;
+        // The delay between the first (actual) attempt and the first retry
+        // should be non-zero. Rely on the convention that `retrycount` is
+        // equal to 1 for the first retry when we are in here (was once 0,
+        // which was a bug -- see #122).
+        delay = retrycount * 1000;
       } else if (config.backoffType === 'static') {
         delay = config.retryDelay!;
       } else {
-        delay = ((Math.pow(2, config.currentRetryAttempt!) - 1) / 2) * 1000;
+        delay = ((Math.pow(2, retrycount) - 1) / 2) * 1000;
       }
     }
-    // We're going to retry!  Incremenent the counter.
-    (err.config as RaxConfig).raxConfig!.currentRetryAttempt! += 1;
     setTimeout(resolve, delay);
   });
 

test/index.ts

@@ -109,6 +109,126 @@ describe('retry-axios', () => {
     assert.fail('Expected to throw');
   });
 
+  it('should have non-zero delay between first and second attempt, static backoff', async () => {
+    const requesttimes: bigint[] = [];
+    const scopes = [
+      nock(url)
+        .get('/')
+        .reply((_, __) => {
+          requesttimes.push(process.hrtime.bigint());
+          return [500, 'foo'];
+        }),
+      nock(url)
+        .get('/')
+        .reply((_, __) => {
+          requesttimes.push(process.hrtime.bigint());
+          return [200, 'bar'];
+        }),
+    ];
+
+    interceptorId = rax.attach();
+    const res = await axios({
+      url,
+      raxConfig: {
+        backoffType: 'static',
+      },
+    });
+
+    // Confirm that first retry did yield 200 OK with expected body
+    assert.strictEqual(res.data, 'bar');
+    scopes.forEach(s => s.done());
+
+    assert.strictEqual(requesttimes.length, 2);
+    const delayInSeconds = Number(requesttimes[1] - requesttimes[0]) / 10 ** 9;
+
+    // The default delay between attempts using the
+    // static backoff strategy is 100 ms. Test with tolerance.
+    assert.strict(
+      0.16 > delayInSeconds && delayInSeconds > 0.1,
+      `unexpected delay: ${delayInSeconds.toFixed(3)} s`
+    );
+  });
+
+  it('should have non-zero delay between first and second attempt, linear backoff', async () => {
+    const requesttimes: bigint[] = [];
+    const scopes = [
+      nock(url)
+        .get('/')
+        .reply((_, __) => {
+          requesttimes.push(process.hrtime.bigint());
+          return [500, 'foo'];
+        }),
+      nock(url)
+        .get('/')
+        .reply((_, __) => {
+          requesttimes.push(process.hrtime.bigint());
+          return [200, 'bar'];
+        }),
+    ];
+
+    interceptorId = rax.attach();
+    const res = await axios({
+      url,
+      raxConfig: {
+        backoffType: 'linear',
+      },
+    });
+
+    // Confirm that first retry did yield 200 OK with expected body
+    assert.strictEqual(res.data, 'bar');
+    scopes.forEach(s => s.done());
+
+    assert.strictEqual(requesttimes.length, 2);
+    const delayInSeconds = Number(requesttimes[1] - requesttimes[0]) / 10 ** 9;
+
+    // The default delay between the first two attempts using the
+    // linear backoff strategy is 1000 ms. Test with tolerance.
+    assert.strict(
+      1.1 > delayInSeconds && delayInSeconds > 1.0,
+      `unexpected delay: ${delayInSeconds.toFixed(3)} s`
+    );
+  });
+
+  it('should have non-zero delay between first and second attempt, exp backoff', async () => {
+    const requesttimes: bigint[] = [];
+    const scopes = [
+      nock(url)
+        .get('/')
+        .reply((_, __) => {
+          requesttimes.push(process.hrtime.bigint());
+          return [500, 'foo'];
+        }),
+      nock(url)
+        .get('/')
+        .reply((_, __) => {
+          requesttimes.push(process.hrtime.bigint());
+          return [200, 'bar'];
+        }),
+    ];
+
+    interceptorId = rax.attach();
+    const res = await axios({
+      url,
+      raxConfig: {
+        backoffType: 'exponential',
+      },
+    });
+
+    // Confirm that first retry did yield 200 OK with expected body
+    assert.strictEqual(res.data, 'bar');
+    scopes.forEach(s => s.done());
+
+    assert.strictEqual(requesttimes.length, 2);
+    const delayInSeconds = Number(requesttimes[1] - requesttimes[0]) / 10 ** 9;
+
+    // The default delay between attempts using the
+    // exp backoff strategy is 500 ms. Test with tolerance.
+    assert.strict(
+      0.55 > delayInSeconds && delayInSeconds > 0.5,
+      `unexpected delay: ${delayInSeconds.toFixed(3)} s`
+    );
+  });
+
   it('should accept a new axios instance', async () => {
     const scopes = [
       nock(url).get('/').times(2).reply(500),