googleapis / googleapis/google-cloud-node
[gaxios] totalTimeout does not stop retries when timeout is exceeded
- Dominant language
- TypeScript
- Stars
- 3.2k
- Forks
- 712
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 99
Description
## 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`:
```typescript
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`:
```typescript
const maxAllowableDelay =
config.totalTimeout! - (Date.now() - config.timeOfFirstRequest!);
return Math.min(calculatedDelay, maxAllowableDelay, config.maxRetryDelay!);
```
## Suggested Fix
Add a timeout check to `shouldRetryRequest`:
```typescript
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.
Contributor guide
Assessment
This issue has not been assessed yet.