fix(llm): retry on transient API errors (429, 50x) instead of crashing - #18
fix(llm): retry on transient API errors (429, 50x) instead of crashing#18tejasprasad2008-afk wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Improves the OpenAI-compatible LLM client’s robustness by adding automatic retries on transient failures, aiming to prevent long CLI runs from failing due to brief provider hiccups.
Changes:
- Add retry loop around OpenAI chat completions with retryable-error detection (429, 5xx, network-ish messages).
- Add a 2-second backoff before retrying HTTP 429 responses.
- Improve handling of “no choices”/error-payload responses and add tests covering retries and missing
choices.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/llm/openai.ts |
Adds retry logic, retryable error detection, and stronger validation/error wrapping for empty/invalid responses. |
src/llm/openai.test.ts |
Updates/extends tests to cover “no choices”, missing choices, and retry behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** HTTP status codes worth retrying with a different model. */ | ||
| const RETRYABLE_CODES = new Set([429, 500, 502, 503, 504]); |
| * When `fallbackModels` are configured, a retryable failure (504, 429, empty | ||
| * choices, etc.) on the primary model triggers an automatic retry with the | ||
| * next fallback before the error propagates. |
| if (isRetryableError(error) && attempt < MAX_RETRIES) { | ||
| process.stderr.write( | ||
| pc.yellow( | ||
| ` ⚠ Request failed (attempt ${attempt + 1}/${MAX_RETRIES + 1}), retrying...\n`, | ||
| ), | ||
| ); |
| if (!resp.choices || resp.choices.length === 0) { | ||
| const respStr = JSON.stringify(resp); | ||
| throw Object.assign( | ||
| new Error( | ||
| `${this.label} returned no choices or an error payload: ${respStr}`, | ||
| ), | ||
| { status: 502 }, | ||
| ); |
| if (callCount === 1) { | ||
| const err = new Error("rate limit exceeded") as Error & { | ||
| status: number; | ||
| }; | ||
| err.status = 429; | ||
| throw err; | ||
| } |
cb91b77 to
74033e4
Compare
|
YEAH THIS ONES ALL GOOD AND FIXED...... HOPEFULLY |
|
cool, will run claude code review/copilot and get back. thanks for the PR! |
|
bro istg sth wrong with the reviewer. i checked the code like a 100 times its in good shape |
|
k thats gone now. just some miscallaneous typescript issues and one extra line space |
|
that was a lot of trust issues but umm is it good now 😭 |
07357fa to
a24aa01
Compare
|
ive fixed this one also most issues is github failing to run it if you wanna test it in here itself u can do it in the GitHub Codespace. should i open a pr with these 3 combined? so tht u can try it out easier? its already there i just make it a pr |
Description: