-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: improve Code Index error messages with detailed diagnostics #7745
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
- Enhanced validation-helpers.ts to preserve original error details - Added specific error messages for common configuration issues - Improved Gemini embedder error handling with provider-specific guidance - Added new i18n translations for detailed error messages - Updated tests to cover new error handling scenarios Fixes #7743
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 is like debugging in production - technically possible but morally questionable.
result.error.includes("403") || | ||
result.error.includes("Authentication") | ||
) { | ||
result.error = `${result.error}. For Gemini, ensure you have a valid API key from Google AI Studio (makersuite.google.com/app/apikey) and that it's correctly configured.` |
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 this the best approach for error enhancement? The string matching for "401", "403", "Authentication", "404", and "model" feels a bit fragile. Could we consider using a more structured approach with error codes or types instead of relying on string patterns?
"hostNotFound": "Cannot resolve host: {{url}}. Please verify the URL is correct and accessible.", | ||
"connectionRefused": "Connection refused to {{url}}. Please ensure the service is running and accessible.", | ||
"connectionTimeout": "Connection timed out. Please check your network connection and try again.", | ||
"geminiAuthDetails": "For Gemini, ensure you have a valid API key from Google AI Studio (makersuite.google.com/app/apikey)." |
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.
Should we add placeholder translations or TODO comments for other supported languages? I notice we only updated the English locale file, and it might be easy to forget updating the other language files later.
// Add provider-specific guidance | ||
if (config.embedderProvider === "gemini") { | ||
if (errorMessage.includes("401") || errorMessage.includes("403") || errorMessage.includes("API key")) { | ||
errorMessage += |
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 this provider-specific guidance potentially duplicate what's already added in the embedder's own validateConfiguration method? For example, GeminiEmbedder already adds similar guidance for authentication errors. Maybe we should check if the error message already contains provider-specific guidance before adding more?
* Extracts URL from error message if present | ||
*/ | ||
function extractUrlFromError(errorMessage: string): string | undefined { | ||
// Try to extract URL from common error patterns |
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 regex pattern here might miss some edge cases for URL extraction. Would it be worth using a more comprehensive URL pattern or even a dedicated URL parsing library for better reliability?
error: error instanceof Error ? error.message : String(error), | ||
stack: error instanceof Error ? error.stack : undefined, | ||
location: "GeminiEmbedder:validateConfiguration", | ||
modelId: this.modelId, |
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 adding more specific telemetry event types here. Instead of just capturing a generic CODE_INDEX_ERROR, we could track specific error types (auth_failure, connection_error, model_error) to get better insights into what's failing most often.
Thanks for the quick update, after tried the branch, I got more info: turns out to be a network problem, but not sure which uri returns 400. Because I need VPN to visit google, so I should setup a proxy for this? I have tried to set proxy on vscode settings but no luck. |
Closing this for now as this doesn't seem the right approach |
Summary
This PR attempts to address Issue #7743 by improving error messages for Code Index configuration issues. Users were receiving generic "Invalid embedder configuration" errors that made it difficult to diagnose what was wrong with their setup.
Changes
Enhanced Error Handling
Key Improvements
Testing
Example Error Messages
Before: "Invalid embedder configuration. Please review your settings."
After:
Related Issue
Fixes #7743
Testing Instructions
Feedback and guidance are welcome!
Important
Enhances error messages for Code Index configuration, focusing on Gemini embedder, with detailed diagnostics and comprehensive test coverage.
GeminiEmbedder
ingemini.ts
: Enhances error messages for authentication, model, and connection errors with specific guidance.service-factory.ts
: Adds provider-specific error context for Gemini, OpenAI, and Ollama.validation-helpers.ts
: Improves error message sanitization and handling for various error types.embeddings.json
for detailed diagnostics.gemini.spec.ts
: Adds tests for enhanced error messages, covering authentication, model, and connection errors.This description was created by
for 9263590. You can customize this summary. It will automatically update as commits are pushed.