-
Notifications
You must be signed in to change notification settings - Fork 50
fix: add retry limits and improve builder prompt for data engineering #106
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
Changes from 1 commit
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 |
|---|---|---|
|
|
@@ -496,6 +496,28 @@ export namespace SessionProcessor { | |
| } | ||
| retryErrorType = e?.name ?? "UnknownError" | ||
| attempt++ | ||
|
|
||
| // Give up after max attempts or total retry time exceeded | ||
| const totalRetryTime = retryStartTime ? Date.now() - retryStartTime : 0 | ||
| if ( | ||
| attempt > SessionRetry.RETRY_MAX_ATTEMPTS || | ||
| totalRetryTime > SessionRetry.RETRY_MAX_TOTAL_TIME_MS | ||
|
Comment on lines
+501
to
+504
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. Bug: The time-based retry limit ( Suggested FixReset the Prompt for AI Agent |
||
| ) { | ||
| log.warn("retry limit reached", { | ||
| attempt, | ||
| totalRetryTime, | ||
| maxAttempts: SessionRetry.RETRY_MAX_ATTEMPTS, | ||
| maxTotalTime: SessionRetry.RETRY_MAX_TOTAL_TIME_MS, | ||
| }) | ||
| input.assistantMessage.error = error | ||
| Bus.publish(Session.Event.Error, { | ||
| sessionID: input.assistantMessage.sessionID, | ||
| error: input.assistantMessage.error, | ||
| }) | ||
| SessionStatus.set(input.sessionID, { type: "idle" }) | ||
| break | ||
| } | ||
|
|
||
| const delay = SessionRetry.delay(attempt, error.name === "APIError" ? error : undefined) | ||
| SessionStatus.set(input.sessionID, { | ||
| type: "retry", | ||
|
|
||
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.
Bug: The
attemptcounter is not reset after a successful recovery, causing subsequent retries to use an old count and fail prematurely.Severity: MEDIUM
Suggested Fix
Reset the
attemptcounter to 0 after a successful recovery, in the same place whereretryErrorTypeandretryStartTimeare reset tonull. This ensures that any new error sequence starts with a fresh retry state.Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.