-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: improve ChutesAI error handling for HTTP 500 errors #7834
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
Conversation
- Add retry logic with exponential backoff for transient 500 errors - Enhance error messages to provide more context when API returns empty response - Add detailed logging for debugging API errors - Preserve HTTP status codes in error objects for better error handling - Add comprehensive test coverage for error scenarios Fixes #7832
|
|
||
| if (delta?.content) { | ||
| for (const processedChunk of matcher.update(delta.content)) { | ||
| // Add retry logic for transient errors |
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.
The retry logic in createMessage (lines 54-127) is well implemented for transient 5xx errors. Consider extracting this common retry mechanism into a shared utility to reduce duplication with completePrompt.
This comment was generated because it violated a code review rule: irule_tTqpIuNs8DV0QFGj.
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 reviewed my own code and found bugs I introduced 5 minutes ago. Classic.
| } catch (error) { | ||
| } catch (error: any) { | ||
| // Log the raw error for debugging | ||
| console.error(`${this.providerName} raw error:`, { |
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.
These console.error statements should use the proper logging infrastructure instead. Console logs can clutter production environments and make debugging harder. Consider using a logger service that can be configured for different environments.
| import { BaseOpenAiCompatibleProvider } from "./base-openai-compatible-provider" | ||
|
|
||
| export class ChutesHandler extends BaseOpenAiCompatibleProvider<ChutesModelId> { | ||
| private retryCount = 3 |
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.
Is it intentional to hardcode these retry configuration values? They should probably be configurable through options or environment variables to allow flexibility in different deployment scenarios.
| yield* super.createMessage(systemPrompt, messages) | ||
| return // Success, exit the retry loop | ||
| } | ||
| } catch (error: any) { |
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.
Using error: any without proper type guards could lead to runtime errors. Consider adding a type guard to check if the error has a status property before accessing it.
| }) as const, | ||
| ) | ||
|
|
||
| for await (const chunk of stream) { |
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.
Could we ensure proper cleanup of the stream if an error occurs mid-stream? The current implementation might leave streams unclosed in error scenarios, potentially causing memory leaks. Consider wrapping the stream in a try-finally block or implementing proper stream cleanup.
|
|
||
| // For non-retryable errors or final attempt, throw with more context | ||
| const enhancedError = new Error( | ||
| `ChutesAI API error (${error.status || "unknown status"}): ${error.message || "Empty response body"}. ` + |
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.
The error message format here differs slightly from the one in completePrompt. Consider standardizing the error message format across both methods for consistency.
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks).toContainEqual({ type: "text", text: "Retry success" }) |
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.
Good test coverage for the retry logic! However, could we add a test case for when all retry attempts are exhausted in streaming operations? This would ensure the error handling works correctly in that edge case.
|
Closing this PR as the retry logic treats the symptom rather than the root cause. We need more information to understand why ChutesAI returns 500 errors for Roo Code but works with curl. |
Summary
This PR attempts to address Issue #7832 where the ChutesAI provider returns HTTP 500 errors with empty response bodies, even though direct curl requests to the same API work correctly.
Problem
Users reported that Roo Code 3.27.0 cannot complete requests when using ChutesAI provider, consistently returning HTTP 500 errors with empty JSON bodies. The same requests work fine when sent directly via curl, indicating an issue with how Roo Code handles the ChutesAI API responses.
Solution
1. Enhanced Error Handling
2. Retry Logic
3. Improved Error Messages
Changes
Testing
Impact
This fix should resolve the ChutesAI integration issues without affecting other providers. The retry logic will help handle transient server errors, while the enhanced error messages will provide better debugging information if issues persist.
Fixes #7832
Feedback and guidance are welcome!
Important
Improves ChutesAI error handling by adding retry logic for 5xx errors and enhancing error messages in
ChutesHandler.ChutesHandlerfor 5xx errors, with 3 attempts and delays of 1s, 2s, and 4s.ChutesHandlerto include HTTP status codes and context.base-openai-compatible-provider.tsto capture raw error details.chutes.spec.tsfor retry logic on 500 errors, handling empty response bodies, and ensuring no retry on 4xx errors.getModel()inChutesHandlerto set temperature based on model type.This description was created by
for 3651928. You can customize this summary. It will automatically update as commits are pushed.