feat(core): add HTTP 499 to retryable errors and map to RetryableQuotaError (#20432)

This commit is contained in:
Bryan Morgan
2026-02-26 10:42:34 -05:00
committed by GitHub
parent 3db35812b7
commit 9c2fd5a7c6
8 changed files with 92 additions and 19 deletions

View File

@@ -55,8 +55,8 @@ describe('file-system', () => {
});
});
it('should be able to write a file', async () => {
await rig.setup('should be able to write a file', {
it('should be able to write a hello world message to a file', async () => {
await rig.setup('should be able to write a hello world message to a file', {
settings: { tools: { core: ['write_file', 'replace', 'read_file'] } },
});
rig.createFile('test.txt', '');

View File

@@ -22,8 +22,8 @@ describe('write_file', () => {
afterEach(async () => await rig.cleanup());
it('should be able to write a file', async () => {
await rig.setup('should be able to write a file', {
it('should be able to write a joke to a file', async () => {
await rig.setup('should be able to write a joke to a file', {
settings: { tools: { core: ['write_file', 'read_file'] } },
});
const prompt = `show me an example of using the write tool. put a dad joke in dad.txt`;

View File

@@ -19,6 +19,7 @@ import { AuthType } from '../core/contentGenerator.js';
// Import the new types (Assuming this test file is in packages/core/src/utils/)
import type { FallbackModelHandler } from '../fallback/types.js';
import type { GoogleApiError } from './googleErrors.js';
import { type HttpError } from './httpErrors.js';
import { TerminalQuotaError } from './googleQuotaErrors.js';
vi.mock('node:fs');
@@ -106,6 +107,34 @@ describe('Retry Utility Fallback Integration', () => {
expect(mockApiCall).toHaveBeenCalledTimes(3);
});
it('should trigger onPersistent429 when HTTP 499 persists through all retry attempts', async () => {
let fallbackCalled = false;
const mockError: HttpError = new Error('Simulated 499 error');
mockError.status = 499;
const mockApiCall = vi.fn().mockRejectedValue(mockError); // Always fail with 499
const mockPersistent429Callback = vi.fn(async (_authType?: string) => {
fallbackCalled = true;
// In a real scenario, this would change the model being called by mockApiCall
// or similar, but for the test we just need to see if it's called.
// We return null to stop retrying after the fallback attempt in this test.
return null;
});
const promise = retryWithBackoff(mockApiCall, {
maxAttempts: 2,
initialDelayMs: 1,
maxDelayMs: 10,
onPersistent429: mockPersistent429Callback,
authType: AuthType.LOGIN_WITH_GOOGLE,
});
await expect(promise).rejects.toThrow('Simulated 499 error');
expect(fallbackCalled).toBe(true);
expect(mockPersistent429Callback).toHaveBeenCalledTimes(1);
});
it('should not trigger onPersistent429 for API key users', async () => {
const fallbackCallback = vi.fn();

View File

@@ -81,7 +81,7 @@ describe('classifyGoogleError', () => {
}
});
it('should return original error if code is not 429 or 503', () => {
it('should return original error if code is not 429, 499 or 503', () => {
const apiError: GoogleApiError = {
code: 500,
message: 'Server error',
@@ -95,6 +95,22 @@ describe('classifyGoogleError', () => {
expect(result).not.toBeInstanceOf(RetryableQuotaError);
});
it('should return RetryableQuotaError for 499 Client Closed Request', () => {
const apiError: GoogleApiError = {
code: 499,
message: 'Client Closed Request',
details: [],
};
vi.spyOn(errorParser, 'parseGoogleApiError').mockReturnValue(apiError);
const originalError = new Error('Client Closed Request');
const result = classifyGoogleError(originalError);
expect(result).toBeInstanceOf(RetryableQuotaError);
if (result instanceof RetryableQuotaError) {
expect(result.cause).toBe(apiError);
expect(result.message).toBe('Client Closed Request');
}
});
it('should return TerminalQuotaError for daily quota violations in QuotaFailure', () => {
const apiError: GoogleApiError = {
code: 429,

View File

@@ -219,7 +219,7 @@ export function classifyGoogleError(error: unknown): unknown {
if (
!googleApiError ||
googleApiError.code !== 429 ||
(googleApiError.code !== 429 && googleApiError.code !== 499) ||
googleApiError.details.length === 0
) {
// Fallback: try to parse the error message for a retry delay
@@ -233,27 +233,27 @@ export function classifyGoogleError(error: unknown): unknown {
return new RetryableQuotaError(
errorMessage,
googleApiError ?? {
code: 429,
code: status ?? 429,
message: errorMessage,
details: [],
},
retryDelaySeconds,
);
}
} else if (status === 429) {
// Fallback: If it is a 429 but doesn't have a specific "retry in" message,
} else if (status === 429 || status === 499) {
// Fallback: If it is a 429 or 499 but doesn't have a specific "retry in" message,
// assume it is a temporary rate limit and retry after 5 sec (same as DEFAULT_RETRY_OPTIONS).
return new RetryableQuotaError(
errorMessage,
googleApiError ?? {
code: 429,
code: status,
message: errorMessage,
details: [],
},
);
}
return error; // Not a 429 error we can handle with structured details or a parsable retry message.
return error; // Not a retryable error we can handle with structured details or a parsable retry message.
}
const quotaFailure = googleApiError.details.find(
@@ -353,15 +353,15 @@ export function classifyGoogleError(error: unknown): unknown {
}
}
// If we reached this point and the status is still 429, we return retryable.
if (status === 429) {
// If we reached this point and the status is still 429 or 499, we return retryable.
if (status === 429 || status === 499) {
const errorMessage =
googleApiError?.message ||
(error instanceof Error ? error.message : String(error));
return new RetryableQuotaError(
errorMessage,
googleApiError ?? {
code: 429,
code: status,
message: errorMessage,
details: [],
},

View File

@@ -158,6 +158,30 @@ describe('retryWithBackoff', () => {
expect(mockFn).not.toHaveBeenCalled();
});
it('should retry on HTTP 499 (Client Closed Request) error', async () => {
let attempts = 0;
const mockFn = vi.fn(async () => {
attempts++;
if (attempts === 1) {
const error: HttpError = new Error('Simulated 499 error');
error.status = 499;
throw error;
}
return 'success';
});
const promise = retryWithBackoff(mockFn, {
maxAttempts: 2,
initialDelayMs: 10,
});
await vi.runAllTimersAsync();
const result = await promise;
expect(result).toBe('success');
expect(mockFn).toHaveBeenCalledTimes(2);
});
it('should use default shouldRetry if not provided, retrying on ApiError 429', async () => {
const mockFn = vi.fn(async () => {
throw new ApiError({ message: 'Too Many Requests', status: 429 });

View File

@@ -130,13 +130,17 @@ export function isRetryableError(
if (error instanceof ApiError) {
// Explicitly do not retry 400 (Bad Request)
if (error.status === 400) return false;
return error.status === 429 || (error.status >= 500 && error.status < 600);
return (
error.status === 429 ||
error.status === 499 ||
(error.status >= 500 && error.status < 600)
);
}
// Check for status using helper (handles other error shapes)
const status = getErrorStatus(error);
if (status !== undefined) {
return status === 429 || (status >= 500 && status < 600);
return status === 429 || status === 499 || (status >= 500 && status < 600);
}
return false;

View File

@@ -144,14 +144,14 @@ describe('GeminiCliAgent Integration', () => {
});
it('propagates errors from dynamic instructions', async () => {
const goldenFile = getGoldenPath('agent-static-instructions');
const agent = new GeminiCliAgent({
instructions: () => {
throw new Error('Dynamic instruction failure');
},
model: 'gemini-2.0-flash',
fakeResponses: RECORD_MODE
? undefined
: getGoldenPath('agent-dynamic-instructions'),
recordResponses: RECORD_MODE ? goldenFile : undefined,
fakeResponses: RECORD_MODE ? undefined : goldenFile,
});
const session = agent.session();