fix(runtime-fallback): detect prettified quota errors without HTTP status codes (fixes #2747)#2752
Conversation
|
I have read the CLA Document and I hereby sign the CLA |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62d2704009
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const errorObj2 = error as Record<string, unknown> | ||
| const name = errorObj2.name | ||
| if (typeof name === "string" && name.length > 0) { | ||
| const nameColonMatch = name.match(/:\s*(.+)/) | ||
| if (nameColonMatch) return nameColonMatch[1].trim().toLowerCase() |
There was a problem hiding this comment.
If a wrapped provider error uses a generic name like AI_APICallError: Request failed and keeps the retryable detail only in nested fields such as cause.message, this new early return prevents the old JSON.stringify(error) fallback from ever seeing that nested text. extractStatusCode() only reads numeric cause.statusCode, so shapes like { cause: { message: 'status code 503' } } now become non-retryable and the runtime-fallback hook will stop switching models for those wrapped 5xx/quota failures.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 3/5
- This lands at moderate merge risk because there is a concrete behavior gap plus a high-confidence test-validity issue, so outcomes around retry classification may be less reliable.
- The most severe issue is in
src/hooks/runtime-fallback/error-classifier.test.ts: the new test can pass via thequota_exceededpath beforeRETRYABLE_ERROR_PATTERNSis evaluated, creating a false positive for what it claims to validate. - In
src/hooks/runtime-fallback/error-classifier.ts,quota_exceededdetection ignoreserrorName, which can miss quota-related custom error classes when message text lacks quota keywords. - Pay close attention to
src/hooks/runtime-fallback/error-classifier.test.ts,src/hooks/runtime-fallback/error-classifier.ts- verify the test truly exercises pattern-list logic and that quota classification accounts for custom error class names.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/runtime-fallback/error-classifier.test.ts">
<violation number="1" location="src/hooks/runtime-fallback/error-classifier.test.ts:233">
P2: The new test is a false positive for pattern-list validation because `classifyErrorType` marks the same message as `quota_exceeded`, and `isRetryableError` returns true before `RETRYABLE_ERROR_PATTERNS` is evaluated.</violation>
</file>
<file name="src/hooks/runtime-fallback/error-classifier.ts">
<violation number="1" location="src/hooks/runtime-fallback/error-classifier.ts:123">
P2: `quota_exceeded` classification ignores `errorName`, so quota-related custom error classes can be missed when message text lacks quota keywords.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| test("matches subscription quota pattern in RETRYABLE_ERROR_PATTERNS", () => { | ||
| //#given | ||
| const error = { message: "Subscription quota exceeded. You can continue using free models." } |
There was a problem hiding this comment.
P2: The new test is a false positive for pattern-list validation because classifyErrorType marks the same message as quota_exceeded, and isRetryableError returns true before RETRYABLE_ERROR_PATTERNS is evaluated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/hooks/runtime-fallback/error-classifier.test.ts, line 233:
<comment>The new test is a false positive for pattern-list validation because `classifyErrorType` marks the same message as `quota_exceeded`, and `isRetryableError` returns true before `RETRYABLE_ERROR_PATTERNS` is evaluated.</comment>
<file context>
@@ -166,3 +166,76 @@ describe("extractStatusCode", () => {
+
+ test("matches subscription quota pattern in RETRYABLE_ERROR_PATTERNS", () => {
+ //#given
+ const error = { message: "Subscription quota exceeded. You can continue using free models." }
+
+ //#when
</file context>
|
Re: issues identified by cubic. P2 (test false positive): The test validates that P2 (errorName not checked in quota classification): Valid enhancement suggestion. Could add errorName-based detection for custom quota error classes (e.g., |
|
Fixed both issues from cubic review:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f16d55ad95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /billing.?(?:hard.?)?limit/i.test(message) || | ||
| /exhausted\s+your\s+capacity/i.test(message) || | ||
| /out\s+of\s+credits?/i.test(message) || | ||
| /payment.?required/i.test(message) |
There was a problem hiding this comment.
message-update-handler.ts turns any assistant parts entry with type: "error" into a MessageContentError before it reaches classifyErrorType(). With this new payment required match, a downstream tool/MCP error like “payment required to use premium search” will now be labeled quota_exceeded, so isRetryableError() will switch models even though the failure is not provider- or model-specific and cannot be fixed by fallback. That creates unnecessary retry churn whenever assistant error parts contain billing text from other integrations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/hooks/runtime-fallback/error-classifier.test.ts">
<violation number="1" location="src/hooks/runtime-fallback/error-classifier.test.ts:257">
P3: The test comment/intent contradicts actual behavior: `classifyErrorType` does match `payment required`, so this does not validate the fallback pattern path it claims.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Re: P3 identified by cubic on latest commit — test comment/intent contradiction. Acknowledged. The test name says "matches payment required pattern directly via RETRYABLE_ERROR_PATTERNS" but This is a documentation/naming issue, not a behavioral one. Happy to rename the test to something like "detects payment required errors as retryable" to remove the misleading claim about which internal path is exercised. |
The test name claimed it exercised RETRYABLE_ERROR_PATTERNS directly, but classifyErrorType actually matches 'payment required' via the quota_exceeded path first. Rename to 'detects payment required errors as retryable' to accurately describe end-to-end behavior.
|
Fixed P3 from cubic review: renamed misleading test.
The old name and inline comments claimed the test exercised the RETRYABLE_ERROR_PATTERNS fallback path, but |
2 participants
Summary
quota_exceedederror type classification for pattern-based detectionProblem
When providers return quota-exhaustion errors (e.g.,
"Subscription quota exceeded. You can continue using free models."), OpenCode prettifies the raw HTTP error into a friendly message. This strips the HTTP status code (402 Payment Required), so the runtime-fallback hook's status-code-based detection fails. The hook never triggers the fallback chain, and the session errors out instead of switching to a fallback model.Fix
Three-layer detection approach that works regardless of error prettification:
retry_on_errorsdefault list alongside 429/500/502/503/504"quota_exceeded"toclassifyErrorType()that matches quota/billing/subscription patterns by message content. This type is always treated as retryable (likemissing_api_keyandmodel_not_found)subscription.*quota,billing.*limit,payment.*required,out of creditspatterns toRETRYABLE_ERROR_PATTERNSChanges
src/hooks/runtime-fallback/constants.tsretry_on_errors; added 4 new patterns toRETRYABLE_ERROR_PATTERNSsrc/hooks/runtime-fallback/error-classifier.ts"quota_exceeded"classification inclassifyErrorType(); made it always retryable inisRetryableError(); enhancedgetErrorMessage()fallback extractionsrc/hooks/runtime-fallback/error-classifier.test.tsFixes #2747
Summary by cubic
Detects provider quota/billing errors even when status codes are missing or prettified, so runtime fallback switches to a fallback model instead of failing. Fixes #2747.
quota_exceedederror type based on message patterns anderror.name; always retry on it.RETRYABLE_ERROR_PATTERNS(subscription quota, billing limit, payment required, out of credits, textual "402").quota_exceededpath.Written for commit 44fb114. Summary will update on new commits.