-
Notifications
You must be signed in to change notification settings - Fork 269
fix(task): bound the auto-approval retry loop in attemptApiRequest #1324
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
7da7408
d6d4f6a
2fe084b
2312414
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,6 +171,22 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio | |
|
|
||
| const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors | ||
| const MAX_CONTEXT_WINDOW_RETRIES = 3 // Maximum retries for context window errors | ||
| // Bounds the auto-approval retry loop for persistent API errors (e.g. HTTP 429 fair usage). | ||
| // Applied at both boundaries: first-chunk failures inside attemptApiRequest, and the | ||
| // streaming_failed re-push loop in recursivelyMakeClineRequests. Does not govern mid-stream | ||
| // retries when auto-approval is disabled (those re-push without backoff, upstream behavior). | ||
| const MAX_AUTO_APPROVAL_RETRIES = 3 | ||
|
|
||
| // Terminal signal that the auto-approval retry cap is spent. Thrown by attemptApiRequest on | ||
| // first-chunk failures; the streaming_failed handler in recursivelyMakeClineRequests must | ||
| // honor it instead of re-pushing — attemptApiRequest only checks the cap after an error | ||
| // occurs, so a re-push would issue another full API request and loop forever. | ||
| export class ApiRetryCapExceededError extends Error { | ||
| constructor(message: string) { | ||
| super(message) | ||
| this.name = "ApiRetryCapExceededError" | ||
| } | ||
| } | ||
|
|
||
| export interface TaskOptions extends CreateTaskOptions { | ||
| provider: ClineProvider | ||
|
|
@@ -3653,6 +3669,35 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |
|
|
||
| // Apply exponential backoff similar to first-chunk errors when auto-resubmit is enabled | ||
| const stateForBackoff = await this.providerRef.deref()?.getState() | ||
|
|
||
| // Terminal — auto-approval retry cap spent (roo-extensions#3195). Stop loudly | ||
| // instead of re-pushing: attemptApiRequest checks the cap only after an error, | ||
| // so every re-push would issue another full API request and the loop would | ||
| // never end. ApiRetryCapExceededError comes from the first-chunk path (only | ||
| // thrown with auto-approval on, so it stays terminal even if the toggle was | ||
| // flipped mid-flight); the retryAttempt check bounds mid-stream failures, | ||
| // which re-enter here directly with the counter already at the cap. | ||
| if ( | ||
| error instanceof ApiRetryCapExceededError || | ||
| (stateForBackoff?.autoApprovalEnabled && | ||
| (currentItem.retryAttempt ?? 0) >= MAX_AUTO_APPROVAL_RETRIES) | ||
| ) { | ||
| const capMessage = | ||
| error instanceof ApiRetryCapExceededError | ||
| ? error.message | ||
| : `[Task#recursivelyMakeClineRequests] task ${this.taskId}.${this.instanceId} aborted after ` + | ||
| `${MAX_AUTO_APPROVAL_RETRIES} mid-stream auto-approval retries — persistent API error ` + | ||
| `(last: ${rawErrorMessage}). Retry loop capped (roo-extensions#3195).` | ||
| await this.say("error", capMessage) | ||
| this.abortReason = "streaming_failed" | ||
| await this.abortTask() | ||
| // Re-throw into the loop's outer catch so the parent sees | ||
| // didEndLoop=true — a bare `break` exits the stack loop and | ||
| // reports "completed normally" (return false) for what is a | ||
| // task-level terminal stop. | ||
| throw error instanceof ApiRetryCapExceededError ? error : new Error(capMessage) | ||
| } | ||
|
|
||
| if (stateForBackoff?.autoApprovalEnabled) { | ||
| await this.backoffAndAnnounce(currentItem.retryAttempt ?? 0, error) | ||
|
|
||
|
|
@@ -4784,6 +4829,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |
|
|
||
| // note that this api_req_failed ask is unique in that we only present this option if the api hasn't streamed any content yet (ie it fails on the first chunk due), as it would allow them to hit a retry button. However if the api failed mid-stream, it could be in any arbitrary state where some tools may have executed, so that error is handled differently and requires cancelling the task entirely. | ||
| if (autoApprovalEnabled) { | ||
| // Bound the retry loop before backoff: a persistent API error (e.g. HTTP 429 fair usage, | ||
| // a rate limit on the whole account) is not going to resolve by retrying harder — each | ||
| // attempt is charged against the account and postpones recovery. Stop loudly instead of | ||
| // recursing until abort. | ||
| if (retryAttempt >= MAX_AUTO_APPROVAL_RETRIES) { | ||
| // Context-window errors fall through to this branch once their own retries are | ||
| // spent — name that cause instead of blaming the auto-approval cap. | ||
| const cause = isContextWindowExceededError | ||
| ? `context window retries exhausted (${MAX_CONTEXT_WINDOW_RETRIES}) — truncation did not make the request fit` | ||
| : `persistent API error after ${MAX_AUTO_APPROVAL_RETRIES} auto-approval retries` | ||
| throw new ApiRetryCapExceededError( | ||
| `[Task#attemptApiRequest] task ${this.taskId}.${this.instanceId} aborted — ${cause} ` + | ||
| `(last: ${error.message ?? JSON.stringify(serializeError(error))}). Retry loop capped (roo-extensions#3195).`, | ||
| ) | ||
| } | ||
|
Comment on lines
+4836
to
+4846
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this throw land in the |
||
|
|
||
| // Apply shared exponential backoff and countdown UX | ||
| await this.backoffAndAnnounce(retryAttempt, error) | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.