-
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -87,7 +87,24 @@ export abstract class BaseOpenAiCompatibleProvider<ModelName extends string> | |
|
|
||
| try { | ||
| return this.client.chat.completions.create(params, requestOptions) | ||
| } catch (error) { | ||
| } catch (error: any) { | ||
| // Log the raw error for debugging | ||
| console.error(`${this.providerName} raw error:`, { | ||
|
Contributor
Author
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. 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. |
||
| message: error.message, | ||
| status: error.status, | ||
| statusText: error.statusText, | ||
| response: error.response, | ||
| cause: error.cause, | ||
| stack: error.stack, | ||
| }) | ||
|
|
||
| // If it's an OpenAI API error with status code, preserve it | ||
| if (error.status) { | ||
| const enhancedError = handleOpenAIError(error, this.providerName) | ||
| ;(enhancedError as any).status = error.status | ||
| throw enhancedError | ||
| } | ||
|
|
||
| throw handleOpenAIError(error, this.providerName) | ||
| } | ||
| } | ||
|
|
@@ -97,25 +114,44 @@ export abstract class BaseOpenAiCompatibleProvider<ModelName extends string> | |
| messages: Anthropic.Messages.MessageParam[], | ||
| metadata?: ApiHandlerCreateMessageMetadata, | ||
| ): ApiStream { | ||
| const stream = await this.createStream(systemPrompt, messages, metadata) | ||
| try { | ||
| const stream = await this.createStream(systemPrompt, messages, metadata) | ||
|
|
||
| for await (const chunk of stream) { | ||
| const delta = chunk.choices[0]?.delta | ||
| for await (const chunk of stream) { | ||
| const delta = chunk.choices[0]?.delta | ||
|
|
||
| if (delta?.content) { | ||
| yield { | ||
| type: "text", | ||
| text: delta.content, | ||
| if (delta?.content) { | ||
| yield { | ||
| type: "text", | ||
| text: delta.content, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (chunk.usage) { | ||
| yield { | ||
| type: "usage", | ||
| inputTokens: chunk.usage.prompt_tokens || 0, | ||
| outputTokens: chunk.usage.completion_tokens || 0, | ||
| if (chunk.usage) { | ||
| yield { | ||
| type: "usage", | ||
| inputTokens: chunk.usage.prompt_tokens || 0, | ||
| outputTokens: chunk.usage.completion_tokens || 0, | ||
| } | ||
| } | ||
| } | ||
| } catch (error: any) { | ||
| // Log detailed error information | ||
| console.error(`${this.providerName} streaming error:`, { | ||
| message: error.message, | ||
| status: error.status, | ||
| statusText: error.statusText, | ||
| type: error.type, | ||
| code: error.code, | ||
| }) | ||
|
|
||
| // Re-throw with status preserved | ||
| if (error.status) { | ||
| const enhancedError = new Error(error.message || `${this.providerName} streaming failed`) | ||
| ;(enhancedError as any).status = error.status | ||
| throw enhancedError | ||
| } | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -129,7 +165,22 @@ export abstract class BaseOpenAiCompatibleProvider<ModelName extends string> | |
| }) | ||
|
|
||
| return response.choices[0]?.message.content || "" | ||
| } catch (error) { | ||
| } catch (error: any) { | ||
| // Log the raw error for debugging | ||
| console.error(`${this.providerName} completePrompt raw error:`, { | ||
| message: error.message, | ||
| status: error.status, | ||
| statusText: error.statusText, | ||
| response: error.response, | ||
| }) | ||
|
|
||
| // If it's an OpenAI API error with status code, preserve it | ||
| if (error.status) { | ||
| const enhancedError = handleOpenAIError(error, this.providerName) | ||
| ;(enhancedError as any).status = error.status | ||
| throw enhancedError | ||
| } | ||
|
|
||
| throw handleOpenAIError(error, this.providerName) | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,9 @@ import { ApiStream } from "../transform/stream" | |
| import { BaseOpenAiCompatibleProvider } from "./base-openai-compatible-provider" | ||
|
|
||
| export class ChutesHandler extends BaseOpenAiCompatibleProvider<ChutesModelId> { | ||
| private retryCount = 3 | ||
|
Contributor
Author
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. 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. |
||
| private retryDelay = 1000 // Start with 1 second delay | ||
|
|
||
| constructor(options: ApiHandlerOptions) { | ||
| super({ | ||
| ...options, | ||
|
|
@@ -47,46 +50,127 @@ export class ChutesHandler extends BaseOpenAiCompatibleProvider<ChutesModelId> { | |
| override async *createMessage(systemPrompt: string, messages: Anthropic.Messages.MessageParam[]): ApiStream { | ||
| const model = this.getModel() | ||
|
|
||
| if (model.id.includes("DeepSeek-R1")) { | ||
| const stream = await this.client.chat.completions.create({ | ||
| ...this.getCompletionParams(systemPrompt, messages), | ||
| messages: convertToR1Format([{ role: "user", content: systemPrompt }, ...messages]), | ||
| }) | ||
|
|
||
| const matcher = new XmlMatcher( | ||
| "think", | ||
| (chunk) => | ||
| ({ | ||
| type: chunk.matched ? "reasoning" : "text", | ||
| text: chunk.data, | ||
| }) as const, | ||
| ) | ||
|
|
||
| for await (const chunk of stream) { | ||
| const delta = chunk.choices[0]?.delta | ||
|
|
||
| if (delta?.content) { | ||
| for (const processedChunk of matcher.update(delta.content)) { | ||
| // Add retry logic for transient errors | ||
|
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. 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. |
||
| let lastError: Error | null = null | ||
| for (let attempt = 0; attempt < this.retryCount; attempt++) { | ||
| try { | ||
| if (model.id.includes("DeepSeek-R1")) { | ||
| const stream = await this.client.chat.completions.create({ | ||
| ...this.getCompletionParams(systemPrompt, messages), | ||
| messages: convertToR1Format([{ role: "user", content: systemPrompt }, ...messages]), | ||
| }) | ||
|
|
||
| const matcher = new XmlMatcher( | ||
| "think", | ||
| (chunk) => | ||
| ({ | ||
| type: chunk.matched ? "reasoning" : "text", | ||
| text: chunk.data, | ||
| }) as const, | ||
| ) | ||
|
|
||
| for await (const chunk of stream) { | ||
|
Contributor
Author
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. 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. |
||
| const delta = chunk.choices[0]?.delta | ||
|
|
||
| if (delta?.content) { | ||
| for (const processedChunk of matcher.update(delta.content)) { | ||
| yield processedChunk | ||
| } | ||
| } | ||
|
|
||
| if (chunk.usage) { | ||
| yield { | ||
| type: "usage", | ||
| inputTokens: chunk.usage.prompt_tokens || 0, | ||
| outputTokens: chunk.usage.completion_tokens || 0, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Process any remaining content | ||
| for (const processedChunk of matcher.final()) { | ||
| yield processedChunk | ||
| } | ||
| return // Success, exit the retry loop | ||
| } else { | ||
| yield* super.createMessage(systemPrompt, messages) | ||
| return // Success, exit the retry loop | ||
| } | ||
| } catch (error: any) { | ||
|
Contributor
Author
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. Using |
||
| lastError = error | ||
| console.error(`ChutesAI API error (attempt ${attempt + 1}/${this.retryCount}):`, { | ||
| status: error.status, | ||
| message: error.message, | ||
| response: error.response, | ||
| cause: error.cause, | ||
| }) | ||
|
|
||
| if (chunk.usage) { | ||
| yield { | ||
| type: "usage", | ||
| inputTokens: chunk.usage.prompt_tokens || 0, | ||
| outputTokens: chunk.usage.completion_tokens || 0, | ||
| } | ||
| // Check if it's a retryable error (5xx errors) | ||
| if (error.status && error.status >= 500 && error.status < 600 && attempt < this.retryCount - 1) { | ||
| // Exponential backoff | ||
| const delay = this.retryDelay * Math.pow(2, attempt) | ||
| console.log(`Retrying ChutesAI request after ${delay}ms...`) | ||
| await new Promise((resolve) => setTimeout(resolve, delay)) | ||
| continue | ||
| } | ||
|
|
||
| // 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"}. ` + | ||
|
Contributor
Author
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. The error message format here differs slightly from the one in |
||
| `This may be a temporary issue with the ChutesAI service. ` + | ||
| `Please verify your API key and try again.`, | ||
| ) | ||
| ;(enhancedError as any).status = error.status | ||
| ;(enhancedError as any).originalError = error | ||
| throw enhancedError | ||
| } | ||
| } | ||
|
|
||
| // Process any remaining content | ||
| for (const processedChunk of matcher.final()) { | ||
| yield processedChunk | ||
| // If we've exhausted all retries | ||
| if (lastError) { | ||
| throw lastError | ||
| } | ||
| } | ||
|
|
||
| override async completePrompt(prompt: string): Promise<string> { | ||
| let lastError: Error | null = null | ||
|
|
||
| for (let attempt = 0; attempt < this.retryCount; attempt++) { | ||
| try { | ||
| return await super.completePrompt(prompt) | ||
| } catch (error: any) { | ||
| lastError = error | ||
| console.error(`ChutesAI completePrompt error (attempt ${attempt + 1}/${this.retryCount}):`, { | ||
| status: error.status, | ||
| message: error.message, | ||
| }) | ||
|
|
||
| // Check if it's a retryable error (5xx errors) | ||
| if (error.status && error.status >= 500 && error.status < 600 && attempt < this.retryCount - 1) { | ||
| // Exponential backoff | ||
| const delay = this.retryDelay * Math.pow(2, attempt) | ||
| console.log(`Retrying ChutesAI completePrompt after ${delay}ms...`) | ||
| await new Promise((resolve) => setTimeout(resolve, delay)) | ||
| continue | ||
| } | ||
|
|
||
| // For non-retryable errors or final attempt, throw with more context | ||
| const enhancedError = new Error( | ||
| `ChutesAI completion error (${error.status || "unknown status"}): ${error.message || "Empty response body"}. ` + | ||
| `Please verify your API key and endpoint configuration.`, | ||
| ) | ||
| ;(enhancedError as any).status = error.status | ||
| ;(enhancedError as any).originalError = error | ||
| throw enhancedError | ||
| } | ||
| } else { | ||
| yield* super.createMessage(systemPrompt, messages) | ||
| } | ||
|
|
||
| // If we've exhausted all retries | ||
| if (lastError) { | ||
| throw lastError | ||
| } | ||
|
|
||
| throw new Error("ChutesAI completion failed after all retry attempts") | ||
| } | ||
|
|
||
| override getModel() { | ||
|
|
||
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.