-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat: resubmit overloaded errors #9082
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?
Conversation
|
Keep this PR in a mergeable state → Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 1 file
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="gui/src/redux/thunks/streamThunkWrapper.tsx">
<violation number="1" location="gui/src/redux/thunks/streamThunkWrapper.tsx:53">
P1: Missing `cancelStream()` call for non-retryable errors. The original code always called `cancelStream()` when any error occurred, but this branch doesn't.</violation>
<violation number="2" location="gui/src/redux/thunks/streamThunkWrapper.tsx:64">
P1: Missing `return` after handling non-retryable error. Without it, the loop continues after showing the error dialog, causing the operation to retry even for errors that shouldn't be retried.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| const delayMs = OVERLOADED_DELAY_MS * 2 ** attempt; | ||
| await new Promise((resolve) => setTimeout(resolve, delayMs)); | ||
| await dispatch(cancelStream()); | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Missing cancelStream() call for non-retryable errors. The original code always called cancelStream() when any error occurred, but this branch doesn't.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At gui/src/redux/thunks/streamThunkWrapper.tsx, line 53:
<comment>Missing `cancelStream()` call for non-retryable errors. The original code always called `cancelStream()` when any error occurred, but this branch doesn't.</comment>
<file context>
@@ -8,43 +8,61 @@ import { ThunkApiType } from "../store";
+ const delayMs = OVERLOADED_DELAY_MS * 2 ** attempt;
+ await new Promise((resolve) => setTimeout(resolve, delayMs));
+ await dispatch(cancelStream());
+ } else {
+ dispatch(setDialogMessage(<StreamErrorDialog error={e} />));
+ dispatch(setShowDialog(true));
</file context>
| } else { | |
| } else { | |
| await dispatch(cancelStream()); |
✅ Addressed in c391353
| }; | ||
|
|
||
| posthog.capture("gui_stream_error", errorData); | ||
| posthog.capture("gui_stream_error", errorData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Missing return after handling non-retryable error. Without it, the loop continues after showing the error dialog, causing the operation to retry even for errors that shouldn't be retried.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At gui/src/redux/thunks/streamThunkWrapper.tsx, line 64:
<comment>Missing `return` after handling non-retryable error. Without it, the loop continues after showing the error dialog, causing the operation to retry even for errors that shouldn't be retried.</comment>
<file context>
@@ -8,43 +8,61 @@ import { ThunkApiType } from "../store";
+ };
- posthog.capture("gui_stream_error", errorData);
+ posthog.capture("gui_stream_error", errorData);
+ }
+ }
</file context>
| posthog.capture("gui_stream_error", errorData); | |
| posthog.capture("gui_stream_error", errorData); | |
| return; |
✅ Addressed in c391353
Review Feedback AddressedI've reviewed the issues identified by the code review and they are valid. Here are the fixes: Issue 1: Missing
|
- Add cancelStream() call before showing error dialog for non-retryable errors - Add return statement to prevent loop continuation after handling non-retryable error Co-authored-by: nate <[email protected]> Generated with [Continue](https://continue.dev) Co-Authored-By: Continue <[email protected]>
| const state = getState(); | ||
| const selectedModel = selectSelectedChatModel(state); | ||
| const shouldRetry = | ||
| isOverloadedErrorMessage(message) && attempt < OVERLOADED_RETRIES; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think checking attempt < OVERLOADED_RETRIES here has 2 issues
- it's duplicate of having the for loop with attempt <= OVERLOADED_RETRIES
- it wills top at e.g. 2 when OVERLOADED_RETRIES = 3
Let's change to either use recursive stop at depth OVERLOADED_RETRIES approach or just the for loop
Updated: Improved Retry ConditionI've addressed the additional feedback about the retry condition logic. Issue 3: Redundant retry condition checkProblem: The condition Fix: Changed to explicitly check Updated Codeconst isLastAttempt = attempt === OVERLOADED_RETRIES;
const shouldRetry = isOverloadedErrorMessage(message) && !isLastAttempt;This approach:
Latest commit: b510535 |
Updated: Improved Retry ConditionI have addressed the additional feedback about the retry condition logic. Issue 3: Redundant retry condition checkProblem: The condition Fix: Changed to explicitly check Updated Codeconst isLastAttempt = attempt === OVERLOADED_RETRIES;
const shouldRetry = isOverloadedErrorMessage(message) && !isLastAttempt;This approach:
Latest commit: b510535 |
Description
Retry overloaded errors from the model provider's server with an exponential backoff.
resolves CON-5040
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
feat.mp4
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Add automatic retries with exponential backoff when the model provider is overloaded to reduce failed chats and improve reliability. Addresses CON-5040.
Written for commit c391353. Summary will update automatically on new commits.