fix(core): improve API response error handling and retry logic (#14563)

This commit is contained in:
matt korwel
2025-12-05 09:49:08 -08:00
committed by GitHub
parent 738354ff65
commit dd3fd73ffe
8 changed files with 375 additions and 45 deletions

View File

@@ -22,8 +22,9 @@ export class FetchError extends Error {
constructor(
message: string,
public code?: string,
options?: ErrorOptions,
) {
super(message);
super(message, options);
this.name = 'FetchError';
}
}
@@ -51,7 +52,7 @@ export async function fetchWithTimeout(
if (isNodeError(error) && error.code === 'ABORT_ERR') {
throw new FetchError(`Request timed out after ${timeout}ms`, 'ETIMEDOUT');
}
throw new FetchError(getErrorMessage(error));
throw new FetchError(getErrorMessage(error), undefined, { cause: error });
} finally {
clearTimeout(timeoutId);
}

View File

@@ -307,7 +307,7 @@ describe('retryWithBackoff', () => {
});
describe('Fetch error retries', () => {
it('should retry on specific fetch error when retryFetchErrors is true', async () => {
it("should retry on 'fetch failed' when retryFetchErrors is true", async () => {
const mockFn = vi.fn();
mockFn.mockRejectedValueOnce(new TypeError('fetch failed'));
mockFn.mockResolvedValueOnce('success');
@@ -365,19 +365,48 @@ describe('retryWithBackoff', () => {
expect(mockFn).toHaveBeenCalledTimes(2);
});
it.each([false, undefined])(
'should not retry on specific fetch error when retryFetchErrors is %s',
async (retryFetchErrors) => {
const mockFn = vi.fn().mockRejectedValue(new TypeError('fetch failed'));
it("should retry on 'fetch failed' when retryFetchErrors is true (short delays)", async () => {
const mockFn = vi
.fn()
.mockRejectedValueOnce(new TypeError('fetch failed'))
.mockResolvedValue('success');
const promise = retryWithBackoff(mockFn, {
retryFetchErrors,
});
const promise = retryWithBackoff(mockFn, {
retryFetchErrors: true,
initialDelayMs: 1,
maxDelayMs: 1,
});
await vi.runAllTimersAsync();
await expect(promise).resolves.toBe('success');
});
await expect(promise).rejects.toThrow('fetch failed');
expect(mockFn).toHaveBeenCalledTimes(1);
},
);
it("should not retry on 'fetch failed' when retryFetchErrors is false", async () => {
const mockFn = vi.fn().mockRejectedValue(new TypeError('fetch failed'));
const promise = retryWithBackoff(mockFn, {
retryFetchErrors: false,
initialDelayMs: 1,
maxDelayMs: 1,
});
await expect(promise).rejects.toThrow('fetch failed');
expect(mockFn).toHaveBeenCalledTimes(1);
});
it('should retry on network error code (ETIMEDOUT) even when retryFetchErrors is false', async () => {
const error = new Error('connect ETIMEDOUT');
(error as any).code = 'ETIMEDOUT';
const mockFn = vi
.fn()
.mockRejectedValueOnce(error)
.mockResolvedValue('success');
const promise = retryWithBackoff(mockFn, {
retryFetchErrors: false,
initialDelayMs: 1,
maxDelayMs: 1,
});
await vi.runAllTimersAsync();
await expect(promise).resolves.toBe('success');
});
});
describe('Flash model fallback for OAuth users', () => {

View File

@@ -35,7 +35,7 @@ const DEFAULT_RETRY_OPTIONS: RetryOptions = {
maxAttempts: 3,
initialDelayMs: 5000,
maxDelayMs: 30000, // 30 seconds
shouldRetryOnError: defaultShouldRetry,
shouldRetryOnError: isRetryableError,
};
const RETRYABLE_NETWORK_CODES = [
@@ -79,21 +79,21 @@ const FETCH_FAILED_MESSAGE = 'fetch failed';
* @param retryFetchErrors Whether to retry on specific fetch errors.
* @returns True if the error is a transient error, false otherwise.
*/
function defaultShouldRetry(
export function isRetryableError(
error: Error | unknown,
retryFetchErrors?: boolean,
): boolean {
// Check for common network error codes
const errorCode = getNetworkErrorCode(error);
if (errorCode && RETRYABLE_NETWORK_CODES.includes(errorCode)) {
return true;
}
if (retryFetchErrors && error instanceof Error) {
// Check for generic fetch failed message (case-insensitive)
if (error.message.toLowerCase().includes(FETCH_FAILED_MESSAGE)) {
return true;
}
// Check for common network error codes
const errorCode = getNetworkErrorCode(error);
if (errorCode && RETRYABLE_NETWORK_CODES.includes(errorCode)) {
return true;
}
}
// Priority check for ApiError
@@ -147,6 +147,7 @@ export async function retryWithBackoff<T>(
signal,
} = {
...DEFAULT_RETRY_OPTIONS,
shouldRetryOnError: isRetryableError,
...cleanOptions,
};