[gaxios] totalTimeout does not stop retries when timeout is exceeded

[matan-shavit-cyera]

Description

The totalTimeout option in RetryConfig does not work as documented. According to the type definition:

"The total time starting from when the initial request is sent, after which an error will be returned, regardless of the retrying attempts made meanwhile."

However, the actual implementation only uses totalTimeout to shorten the delay between retries, but never stops retries when the timeout is exceeded.

Expected Behavior

When Date.now() - timeOfFirstRequest >= totalTimeout, retries should stop and an error should be returned.

Actual Behavior

Retries continue indefinitely (up to retry count) even after totalTimeout is exceeded. The only effect is that maxAllowableDelay becomes negative, causing immediate retries (since setTimeout treats negative values as 0).

Root Cause

In packages/gaxios/src/retry.ts, the shouldRetryRequest function has no check for totalTimeout:

function shouldRetryRequest(err: GaxiosError) {
  const config = getConfig(err);

  // Checks for: abort, config.retry === 0, noResponseRetries,
  // httpMethodsToRetry, statusCodesToRetry, currentRetryAttempt
  
  // ❌ Missing check for totalTimeout!
  
  return true;
}

The totalTimeout is only used in getNextRetryDelay:

const maxAllowableDelay =
  config.totalTimeout! - (Date.now() - config.timeOfFirstRequest!);

return Math.min(calculatedDelay, maxAllowableDelay, config.maxRetryDelay!);

Suggested Fix

Add a timeout check to shouldRetryRequest:

function shouldRetryRequest(err: GaxiosError) {
  const config = getConfig(err);

  // ... existing checks ...

  // Check if total timeout exceeded
  if (config.totalTimeout && config.timeOfFirstRequest) {
    const elapsed = Date.now() - config.timeOfFirstRequest;
    if (elapsed >= config.totalTimeout) {
      return false;
    }
  }

  return true;
}

Environment

• gaxios version: 7.1.3 (also affects earlier versions)
• Node.js version: 18+

Impact

This affects any user relying on totalTimeout to bound the total retry duration. Without this fix, retries can continue much longer than expected.

[sofisl]

Can confirm this is an issue because this test should be updated (it's not testing timeout correctly) to something like this, and it's failing:

 it.only('should respect totalTimeout if configured', async () => {
    const scope = nock(url).get('/').delay(1000).reply(500);

    const start = Date.now();
    await assert.rejects(
      request({
        url,
        retryConfig: {
          retryDelayMultiplier: 100,
          totalTimeout: 500,
        },
      }),
      (err: GaxiosError) => {
        assert.strictEqual(err.status, 500);
        return true;
      },
    );
    const delay = Date.now() - start;
    console.log(`ACTUAL DELAY: ${delay}`);
    assert.ok(delay <= 1000);
    scope.done();
  });

However the solution you suggested unfortunately doesn't do the trick, so we'll need to file this away for later.