-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: improve error handling for Qwen API 400 status codes #8143
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 comprehensive error handling in QwenCodeHandler.callApiWithRetry - Handle 400, 403, 429, and 5xx errors with user-friendly messages - Add try-catch blocks to createMessage and completePrompt methods - Include detailed error context to help users understand issues - Add comprehensive test coverage for error scenarios Fixes #8142
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.
Reviewing my own code because apparently I trust no one, not even myself.
| } else if (error.status === 400) { | ||
| // Bad request error - provide more context | ||
| const errorMessage = error?.message || error?.error?.message || "Bad request" | ||
| const detailedMessage = `Qwen API Error (400): ${errorMessage}. This may be due to invalid input format, unsupported file type, or request size limits.` |
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.
Consider extracting this error handling logic into a separate utility function like handleQwenError similar to how OpenAI provider uses handleOpenAIError in src/api/providers/utils/openai-error-handler.ts. This would:
- Improve maintainability
- Make it easier to add new error cases
- Keep the main logic cleaner
You could create src/api/providers/utils/qwen-error-handler.ts with a similar pattern.
| } else if (error.status === 403) { | ||
| // Forbidden error | ||
| const errorMessage = error?.message || error?.error?.message || "Forbidden" | ||
| throw new Error(`Qwen API Error (403): ${errorMessage}. Please check your API permissions.`) |
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.
For consistency with other providers, consider using i18n for these error messages. The OpenAI provider uses internationalized messages like:
| throw new Error(`Qwen API Error (403): ${errorMessage}. Please check your API permissions.`) | |
| throw new Error(i18n.t('providers:qwen.errors.badRequest', { details: errorMessage })) |
This would make the extension more accessible to non-English users.
| console.error("Error in QwenCodeHandler.createMessage:", error) | ||
|
|
||
| // Re-throw with a more user-friendly message if it's not already formatted | ||
| if (error.message && !error.message.startsWith("Qwen API 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.
The error handling here could be more specific. Since callApiWithRetry already handles specific HTTP status codes, this catch block might miss those detailed error messages if the error occurs during streaming setup. Consider checking if the error has a status property and applying the same detailed handling.
| } else if (error.status >= 500) { | ||
| // Server error | ||
| const errorMessage = error?.message || error?.error?.message || "Server error" | ||
| throw new 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.
Service unavailable errors (503) are often temporary. Would it be worth adding automatic retry logic with exponential backoff for these cases? Something like:
| throw new Error( | |
| } else if (error.status >= 500) { | |
| // For 503 specifically, could retry with backoff | |
| if (error.status === 503 && retryCount < maxRetries) { | |
| await sleep(Math.pow(2, retryCount) * 1000) | |
| return await this.callApiWithRetry(apiCall, retryCount + 1) | |
| } | |
| // Otherwise throw the error | |
| const errorMessage = error?.message || error?.error?.message || "Server error" | |
| throw new Error( | |
| `Qwen API Error (${error.status}): ${errorMessage}. The Qwen service may be temporarily unavailable.`, | |
| ) |
| "Qwen API Error (400): Already formatted 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.
Great test coverage for the completePrompt method! Consider adding similar tests for error scenarios in the createMessage streaming method to ensure the error handling works correctly in both streaming and non-streaming contexts.
Description
This PR addresses Issue #8142 by improving error handling in the QwenCodeHandler to provide more informative error messages when API requests fail.
Problem
Users were encountering generic 400 status code errors when using the Qwen model with the read_file tool, making it difficult to understand and resolve the underlying issue.
Solution
Enhanced the error handling in QwenCodeHandler to:
Changes
src/api/providers/qwen-code.ts:
callApiWithRetrymethod to handle multiple HTTP error codescreateMessageandcompletePromptmethodssrc/api/providers/tests/qwen-code.spec.ts:
Testing
Impact
This fix will help users better understand and resolve API errors when using the Qwen model, particularly when encountering:
Fixes #8142
Important
Improves error handling in
QwenCodeHandlerfor various HTTP status codes with detailed messages and adds comprehensive tests.QwenCodeHandlerinqwen-code.tsnow handles HTTP errors 400, 401, 403, 429, and 500+ with specific messages.callApiWithRetry,createMessage, andcompletePromptmethods to include try-catch blocks and improved error messages.qwen-code.spec.tsfor error handling scenarios.This description was created by
for 37925d7. You can customize this summary. It will automatically update as commits are pushed.