Skip to content

fix(patch): cherry-pick 9e6914d to release/v0.22.0-preview.2-pr-15288 to patch version v0.22.0-preview.2 and create version 0.22.0-preview.3 #15294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sehoon38 merged 1 commit into from
Dec 19, 2025
Merged

fix(patch): cherry-pick 9e6914d to release/v0.22.0-preview.2-pr-15288 to patch version v0.22.0-preview.2 and create version 0.22.0-preview.3 #15294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 62 additions & 2 deletions packages/core/src/utils/googleQuotaErrors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ describe('classifyGoogleError', () => {
expect(result).toBeInstanceOf(TerminalQuotaError);
});

it('should return original error for 429 without specific details', () => {
it('should return RetryableQuotaError for any 429', () => {
const apiError: GoogleApiError = {
code: 429,
message: 'Too many requests',
Expand All @@ -340,7 +340,10 @@ describe('classifyGoogleError', () => {
vi.spyOn(errorParser, 'parseGoogleApiError').mockReturnValue(apiError);
const originalError = new Error();
const result = classifyGoogleError(originalError);
expect(result).toBe(originalError);
expect(result).toBeInstanceOf(RetryableQuotaError);
if (result instanceof RetryableQuotaError) {
expect(result.retryDelayMs).toBe(5000);
}
});

it('should classify nested JSON string 404 error as ModelNotFoundError', () => {
Expand Down Expand Up @@ -389,4 +392,61 @@ describe('classifyGoogleError', () => {
});
}
});

it('should return RetryableQuotaError with 5s fallback for generic 429 without specific message', () => {
const generic429 = {
status: 429,
message: 'Resource exhausted. No specific retry info.',
};

const result = classifyGoogleError(generic429);

expect(result).toBeInstanceOf(RetryableQuotaError);
if (result instanceof RetryableQuotaError) {
expect(result.retryDelayMs).toBe(5000);
}
});

it('should return RetryableQuotaError with 5s fallback for 429 with empty details and no regex match', () => {
const errorWithEmptyDetails = {
error: {
code: 429,
message: 'A generic 429 error with no retry message.',
details: [],
},
};

const result = classifyGoogleError(errorWithEmptyDetails);

expect(result).toBeInstanceOf(RetryableQuotaError);
if (result instanceof RetryableQuotaError) {
expect(result.retryDelayMs).toBe(5000);
}
});

it('should return RetryableQuotaError with 5s fallback for 429 with some detail', () => {
const errorWithEmptyDetails = {
error: {
code: 429,
message: 'A generic 429 error with no retry message.',
details: [
{
'@type': 'type.googleapis.com/google.rpc.ErrorInfo',
reason: 'QUOTA_EXCEEDED',
domain: 'googleapis.com',
metadata: {
quota_limit: '',
},
},
],
},
};

const result = classifyGoogleError(errorWithEmptyDetails);

expect(result).toBeInstanceOf(RetryableQuotaError);
if (result instanceof RetryableQuotaError) {
expect(result.retryDelayMs).toBe(5000);
}
});
Comment on lines +427 to +451
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In this test case, the variable errorWithEmptyDetails is used, but the error object it holds actually contains details (lines 432-441). This name is misleading and was likely copied from the previous test. Using a more accurate name like errorWithSomeDetails would improve the test's readability and prevent future confusion.

  it('should return RetryableQuotaError with 5s fallback for 429 with some detail', () => {
    const errorWithSomeDetails = {
      error: {
        code: 429,
        message: 'A generic 429 error with no retry message.',
        details: [
          {
            '@type': 'type.googleapis.com/google.rpc.ErrorInfo',
            reason: 'QUOTA_EXCEEDED',
            domain: 'googleapis.com',
            metadata: {
              quota_limit: '',
            },
          },
        ],
      },
    };

    const result = classifyGoogleError(errorWithSomeDetails);

    expect(result).toBeInstanceOf(RetryableQuotaError);
    if (result instanceof RetryableQuotaError) {
      expect(result.retryDelayMs).toBe(5000);
    }
  });

});
30 changes: 30 additions & 0 deletions packages/core/src/utils/googleQuotaErrors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import type {
import { parseGoogleApiError } from './googleErrors.js';
import { getErrorStatus, ModelNotFoundError } from './httpErrors.js';

const DEFAULT_RETRYABLE_DELAY_SECOND = 5;

/**
* A non-retryable error indicating a hard quota limit has been reached (e.g., daily limit).
*/
Expand Down Expand Up @@ -112,6 +114,18 @@ export function classifyGoogleError(error: unknown): unknown {
retryDelaySeconds,
);
}
} else if (status === 429) {
// Fallback: If it is a 429 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,
message: errorMessage,
details: [],
},
DEFAULT_RETRYABLE_DELAY_SECOND,
);
}

return error; // Not a 429 error we can handle with structured details or a parsable retry message.
Expand Down Expand Up @@ -232,5 +246,21 @@ export function classifyGoogleError(error: unknown): unknown {
);
}
}

// If we reached this point and the status is still 429, we return retryable.
if (status === 429) {
const errorMessage =
googleApiError?.message ||
(error instanceof Error ? error.message : String(error));
return new RetryableQuotaError(
errorMessage,
googleApiError ?? {
code: 429,
message: errorMessage,
details: [],
},
DEFAULT_RETRYABLE_DELAY_SECOND,
);
}
Comment on lines +251 to +264
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This block of code, which creates a fallback RetryableQuotaError, is nearly identical to the logic in lines 117-129. The definition of errorMessage is also duplicated from line 100. This redundancy violates the DRY (Don't Repeat Yourself) principle and can make the code harder to maintain, as any future changes would need to be applied in two places.

To improve maintainability, I recommend extracting this logic into a single, private helper function. This function would be responsible for creating the default RetryableQuotaError and could be called from both locations.

For example:

function createDefaultRetryableError(error: unknown, googleApiError: GoogleApiError | null): RetryableQuotaError {
  const errorMessage =
    googleApiError?.message ||
    (error instanceof Error ? error.message : String(error));
  return new RetryableQuotaError(
    errorMessage,
    googleApiError ?? {
      code: 429,
      message: errorMessage,
      details: [],
    },
    DEFAULT_RETRYABLE_DELAY_SECOND,
  );
}

This would centralize the fallback logic, making the code cleaner and easier to manage.

return error; // Fallback to original error if no specific classification fits.
}