Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/i18n/locales/en/embeddings.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@
"invalidModel": "Invalid model. Please check your model configuration.",
"invalidResponse": "Invalid response from embedder service. Please check your configuration.",
"apiKeyRequired": "API key is required for this embedder",
"baseUrlRequired": "Base URL is required for this embedder"
"baseUrlRequired": "Base URL is required for this embedder",
"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)."
Copy link
Contributor Author

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.

},
"serviceFactory": {
"openAiConfigMissing": "OpenAI configuration missing for embedder creation",
Expand Down
93 changes: 85 additions & 8 deletions src/services/code-index/embedders/__tests__/gemini.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,32 +162,109 @@ describe("GeminiEmbedder", () => {
expect(result).toEqual({ valid: true })
})

it("should pass through validation errors from OpenAICompatibleEmbedder", async () => {
it("should enhance authentication error messages with Gemini-specific guidance", async () => {
// Arrange
embedder = new GeminiEmbedder("test-api-key")
mockValidateConfiguration.mockResolvedValue({
valid: false,
error: "embeddings:validation.authenticationFailed",
error: "Authentication failed (HTTP 401)",
})

// Act
const result = await embedder.validateConfiguration()

// Assert
expect(mockValidateConfiguration).toHaveBeenCalled()
expect(result).toEqual({
expect(result.valid).toBe(false)
expect(result.error).toContain("Authentication failed (HTTP 401)")
expect(result.error).toContain("Google AI Studio")
expect(result.error).toContain("makersuite.google.com/app/apikey")
})

it("should enhance model error messages with supported models list", async () => {
// Arrange
embedder = new GeminiEmbedder("test-api-key", "invalid-model")
mockValidateConfiguration.mockResolvedValue({
valid: false,
error: "embeddings:validation.authenticationFailed",
error: "Model not found (HTTP 404)",
})

// Act
const result = await embedder.validateConfiguration()

// Assert
expect(result.valid).toBe(false)
expect(result.error).toContain("Model not found")
expect(result.error).toContain("text-embedding-004")
expect(result.error).toContain("gemini-embedding-001")
expect(result.error).toContain("dimension: 768")
expect(result.error).toContain("dimension: 2048")
})

it("should handle validation exceptions", async () => {
it("should enhance connection error messages with API endpoint info", async () => {
// Arrange
embedder = new GeminiEmbedder("test-api-key")
mockValidateConfiguration.mockRejectedValue(new Error("Validation failed"))
mockValidateConfiguration.mockResolvedValue({
valid: false,
error: "connection refused",
})

// Act & Assert
await expect(embedder.validateConfiguration()).rejects.toThrow("Validation failed")
// Act
const result = await embedder.validateConfiguration()

// Assert
expect(result.valid).toBe(false)
expect(result.error).toContain("connection refused")
expect(result.error).toContain("https://generativelanguage.googleapis.com/v1beta/openai/")
})

it("should pass through validation errors without enhancement for non-specific errors", async () => {
// Arrange
embedder = new GeminiEmbedder("test-api-key")
mockValidateConfiguration.mockResolvedValue({
valid: false,
error: "Some other error",
})

// Act
const result = await embedder.validateConfiguration()

// Assert
expect(mockValidateConfiguration).toHaveBeenCalled()
expect(result).toEqual({
valid: false,
error: "Some other error",
})
})

it("should handle validation exceptions with detailed error message", async () => {
// Arrange
embedder = new GeminiEmbedder("test-api-key", "test-model")
mockValidateConfiguration.mockRejectedValue(new Error("Network error"))

// Act
const result = await embedder.validateConfiguration()

// Assert
expect(result.valid).toBe(false)
expect(result.error).toContain("Gemini embedder validation failed")
expect(result.error).toContain("Network error")
expect(result.error).toContain("test-model")
})

it("should handle non-Error exceptions gracefully", async () => {
// Arrange
embedder = new GeminiEmbedder("test-api-key", "test-model")
mockValidateConfiguration.mockRejectedValue("String error")

// Act
const result = await embedder.validateConfiguration()

// Assert
expect(result.valid).toBe(false)
expect(result.error).toContain("Gemini embedder validation failed")
expect(result.error).toContain("String error")
expect(result.error).toContain("test-model")
})
})
})
30 changes: 27 additions & 3 deletions src/services/code-index/embedders/gemini.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,39 @@ export class GeminiEmbedder implements IEmbedder {
async validateConfiguration(): Promise<{ valid: boolean; error?: string }> {
try {
// Delegate validation to the OpenAI-compatible embedder
// The error messages will be specific to Gemini since we're using Gemini's base URL
return await this.openAICompatibleEmbedder.validateConfiguration()
const result = await this.openAICompatibleEmbedder.validateConfiguration()

// If validation failed, enhance the error message with Gemini-specific guidance
if (!result.valid && result.error) {
// Check for common Gemini-specific issues
if (
result.error.includes("401") ||
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.`
Copy link
Contributor Author

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?

} else if (result.error.includes("404") || result.error.includes("model")) {
result.error = `${result.error}. Supported Gemini models: text-embedding-004 (dimension: 768), gemini-embedding-001 (dimension: 2048).`
} else if (result.error.includes("connection") || result.error.includes("host")) {
result.error = `${result.error}. Gemini API endpoint: ${GeminiEmbedder.GEMINI_BASE_URL}`
}
}

return result
} catch (error) {
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {
error: error instanceof Error ? error.message : String(error),
stack: error instanceof Error ? error.stack : undefined,
location: "GeminiEmbedder:validateConfiguration",
modelId: this.modelId,
Copy link
Contributor Author

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.

})
throw error

// Provide a more informative error message
const errorMessage = error instanceof Error ? error.message : String(error)
return {
valid: false,
error: `Gemini embedder validation failed: ${errorMessage}. Please check your API key and model configuration (current model: ${this.modelId}).`,
}
}
}

Expand Down
39 changes: 35 additions & 4 deletions src/services/code-index/service-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,19 +93,50 @@ export class CodeIndexServiceFactory {
*/
public async validateEmbedder(embedder: IEmbedder): Promise<{ valid: boolean; error?: string }> {
try {
return await embedder.validateConfiguration()
const result = await embedder.validateConfiguration()

// If validation failed but no specific error was provided, add context
if (!result.valid && !result.error) {
const config = this.configManager.getConfig()
result.error =
t("embeddings:validation.configurationError") +
` (Provider: ${config.embedderProvider}, Model: ${config.modelId || "default"})`
}

return result
} catch (error) {
// Capture telemetry for the error
// Capture telemetry for the error with additional context
const config = this.configManager.getConfig()
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {
error: error instanceof Error ? error.message : String(error),
stack: error instanceof Error ? error.stack : undefined,
location: "validateEmbedder",
provider: config.embedderProvider,
model: config.modelId,
})

// If validation throws an exception, preserve the original error message
// Provide detailed error message with context
let errorMessage = error instanceof Error ? error.message : String(error)

// Add provider-specific guidance
if (config.embedderProvider === "gemini") {
if (errorMessage.includes("401") || errorMessage.includes("403") || errorMessage.includes("API key")) {
errorMessage +=
Copy link
Contributor Author

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?

". Please ensure your Gemini API key is valid and correctly configured in the settings."
}
} else if (config.embedderProvider === "openai") {
if (errorMessage.includes("401") || errorMessage.includes("403")) {
errorMessage += ". Please check your OpenAI API key in the settings."
}
} else if (config.embedderProvider === "ollama") {
if (errorMessage.includes("connection") || errorMessage.includes("ECONNREFUSED")) {
errorMessage += ". Please ensure Ollama is running locally and accessible."
}
}

return {
valid: false,
error: error instanceof Error ? error.message : "embeddings:validation.configurationError",
error: errorMessage,
}
}
}
Expand Down
80 changes: 67 additions & 13 deletions src/services/code-index/shared/validation-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,34 +167,88 @@ export function handleValidationError(
// Check for status-based errors first
const statusError = getErrorMessageForStatus(statusCode, embedderType)
if (statusError) {
return { valid: false, error: statusError }
// Include additional context for authentication errors
if (statusCode === 401 || statusCode === 403) {
const details = embedderType === "gemini" ? t("embeddings:validation.geminiAuthDetails") : ""
return { valid: false, error: details ? `${statusError} ${details}` : statusError }
}
// Include status code in error for better debugging
return { valid: false, error: `${statusError} (HTTP ${statusCode})` }
}

// Check for connection errors
// Check for connection errors with more specific messages
if (errorMessage) {
if (
errorMessage.includes("ENOTFOUND") ||
errorMessage.includes("ECONNREFUSED") ||
errorMessage.includes("ETIMEDOUT") ||
errorMessage === "AbortError" ||
errorMessage.includes("HTTP 0:") ||
errorMessage === "No response"
) {
if (errorMessage.includes("ENOTFOUND")) {
const url = extractUrlFromError(errorMessage)
return {
valid: false,
error: t("embeddings:validation.hostNotFound", { url: url || "configured endpoint" }),
}
}

if (errorMessage.includes("ECONNREFUSED")) {
const url = extractUrlFromError(errorMessage)
return {
valid: false,
error: t("embeddings:validation.connectionRefused", { url: url || "configured endpoint" }),
}
}

if (errorMessage.includes("ETIMEDOUT")) {
return {
valid: false,
error: t("embeddings:validation.connectionTimeout"),
}
}

if (errorMessage === "AbortError" || errorMessage.includes("HTTP 0:") || errorMessage === "No response") {
return { valid: false, error: t("embeddings:validation.connectionFailed") }
}

if (errorMessage.includes("Failed to parse response JSON")) {
return { valid: false, error: t("embeddings:validation.invalidResponse") }
}

// Check for API key related errors
if (errorMessage.includes("API key") || errorMessage.includes("api key") || errorMessage.includes("api-key")) {
return { valid: false, error: t("embeddings:validation.invalidApiKey") }
}

// Check for model-related errors
if (
errorMessage.includes("model") &&
(errorMessage.includes("not found") || errorMessage.includes("does not exist"))
) {
return { valid: false, error: t("embeddings:validation.modelNotAvailable") }
}
}

// For generic errors, preserve the original error message if it's not a standard one
if (errorMessage && errorMessage !== "Unknown error") {
return { valid: false, error: errorMessage }
// Provide more context with the error
return {
valid: false,
error: `${t("embeddings:validation.configurationError")}: ${errorMessage}`,
}
}

// Fallback to generic error
return { valid: false, error: t("embeddings:validation.configurationError") }
// Fallback to generic error with embedder type for context
return {
valid: false,
error: `${t("embeddings:validation.configurationError")} (${embedderType})`,
}
}

/**
* Extracts URL from error message if present
*/
function extractUrlFromError(errorMessage: string): string | undefined {
// Try to extract URL from common error patterns
Copy link
Contributor Author

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?

const urlMatch = errorMessage.match(/(?:https?:\/\/[^\s]+)|(?:getaddrinfo\s+\w+\s+([^\s]+))/i)
if (urlMatch) {
return urlMatch[1] || urlMatch[0]
}
return undefined
}

/**
Expand Down
Loading