fix: address Daniel's feedback on error handling in embedders

- Remove generic try-catch wrappers that hide actual API errors
- Implement robust error message extraction using error?.message instead of error.toString()
- Add consistent retry behavior for all errors (not just rate limits)
- Update tests to verify new error propagation behavior
- Ensure users see specific error messages (auth failures, rate limits, etc.)

Resolves feedback from PR #4432

Author: hannesrudolph

src/services/code-index/embedders/__tests__/openai-compatible.spec.ts

@@ -273,33 +273,33 @@ describe("OpenAICompatibleEmbedder", () => {
 				})
 			})
 
-			it("should not retry on non-rate-limit errors", async () => {
+			it("should retry on non-rate-limit errors without delay", async () => {
 				const testTexts = ["Hello world"]
 				const authError = new Error("Unauthorized")
 				;(authError as any).status = 401
 
 				mockEmbeddingsCreate.mockRejectedValue(authError)
 
 				await expect(embedder.createEmbeddings(testTexts)).rejects.toThrow(
-					"Failed to create embeddings: batch processing error",
+					"Failed to create embeddings after 3 attempts: Unauthorized",
 				)
 
-				expect(mockEmbeddingsCreate).toHaveBeenCalledTimes(1)
+				expect(mockEmbeddingsCreate).toHaveBeenCalledTimes(3)
 				expect(console.warn).not.toHaveBeenCalledWith(expect.stringContaining("Rate limit hit"))
 			})
 
-			it("should throw error immediately on non-retryable errors", async () => {
+			it("should retry on all errors and exhaust attempts", async () => {
 				const testTexts = ["Hello world"]
 				const serverError = new Error("Internal server error")
 				;(serverError as any).status = 500
 
 				mockEmbeddingsCreate.mockRejectedValue(serverError)
 
 				await expect(embedder.createEmbeddings(testTexts)).rejects.toThrow(
-					"Failed to create embeddings: batch processing error",
+					"Failed to create embeddings after 3 attempts: Internal server error",
 				)
 
-				expect(mockEmbeddingsCreate).toHaveBeenCalledTimes(1)
+				expect(mockEmbeddingsCreate).toHaveBeenCalledTimes(3)
 			})
 		})
 
@@ -314,11 +314,11 @@ describe("OpenAICompatibleEmbedder", () => {
 				mockEmbeddingsCreate.mockRejectedValue(apiError)
 
 				await expect(embedder.createEmbeddings(testTexts)).rejects.toThrow(
-					"Failed to create embeddings: batch processing error",
+					"Failed to create embeddings after 3 attempts: API connection failed",
 				)
 
 				expect(console.error).toHaveBeenCalledWith(
-					expect.stringContaining("Failed to process batch"),
+					expect.stringContaining("OpenAI Compatible embedder error"),
 					expect.any(Error),
 				)
 			})
@@ -330,10 +330,13 @@ describe("OpenAICompatibleEmbedder", () => {
 				mockEmbeddingsCreate.mockRejectedValue(batchError)
 
 				await expect(embedder.createEmbeddings(testTexts)).rejects.toThrow(
-					"Failed to create embeddings: batch processing error",
+					"Failed to create embeddings after 3 attempts: Batch processing failed",
 				)
 
-				expect(console.error).toHaveBeenCalledWith("Failed to process batch:", batchError)
+				expect(console.error).toHaveBeenCalledWith(
+					expect.stringContaining("OpenAI Compatible embedder error"),
+					batchError,
+				)
 			})
 
 			it("should handle empty text arrays", async () => {

src/services/code-index/embedders/openai-compatible.ts

@@ -81,15 +81,10 @@ export class OpenAICompatibleEmbedder implements IEmbedder {
 			}
 
 			if (currentBatch.length > 0) {
-				try {
-					const batchResult = await this._embedBatchWithRetries(currentBatch, modelToUse)
-					allEmbeddings.push(...batchResult.embeddings)
-					usage.promptTokens += batchResult.usage.promptTokens
-					usage.totalTokens += batchResult.usage.totalTokens
-				} catch (error) {
-					console.error("Failed to process batch:", error)
-					throw new Error("Failed to create embeddings: batch processing error")
-				}
+				const batchResult = await this._embedBatchWithRetries(currentBatch, modelToUse)
+				allEmbeddings.push(...batchResult.embeddings)
+				usage.promptTokens += batchResult.usage.promptTokens
+				usage.totalTokens += batchResult.usage.totalTokens
 			}
 		}
 
@@ -124,23 +119,23 @@ export class OpenAICompatibleEmbedder implements IEmbedder {
 				const isRateLimitError = error?.status === 429
 				const hasMoreAttempts = attempts < MAX_RETRIES - 1
 
-				if (isRateLimitError && hasMoreAttempts) {
-					const delayMs = INITIAL_DELAY_MS * Math.pow(2, attempts)
-					console.warn(`Rate limit hit, retrying in ${delayMs}ms (attempt ${attempts + 1}/${MAX_RETRIES})`)
-					await new Promise((resolve) => setTimeout(resolve, delayMs))
-					continue
-				}
-
 				// Log the error for debugging
 				console.error(`OpenAI Compatible embedder error (attempt ${attempts + 1}/${MAX_RETRIES}):`, error)
 
-				if (!hasMoreAttempts) {
-					throw new Error(
-						`Failed to create embeddings after ${MAX_RETRIES} attempts: ${error.message || error}`,
-					)
+				if (hasMoreAttempts) {
+					if (isRateLimitError) {
+						const delayMs = INITIAL_DELAY_MS * Math.pow(2, attempts)
+						console.warn(
+							`Rate limit hit, retrying in ${delayMs}ms (attempt ${attempts + 1}/${MAX_RETRIES})`,
+						)
+						await new Promise((resolve) => setTimeout(resolve, delayMs))
+					}
+					continue
 				}
 
-				throw error
+				// Provide more context in the error message using robust error extraction
+				const errorMessage = error?.message || (typeof error === "string" ? error : "Unknown error")
+				throw new Error(`Failed to create embeddings after ${MAX_RETRIES} attempts: ${errorMessage}`)
 			}
 		}
 

src/services/code-index/embedders/openai.ts

@@ -71,15 +71,10 @@ export class OpenAiEmbedder extends OpenAiNativeHandler implements IEmbedder {
 			}
 
 			if (currentBatch.length > 0) {
-				try {
-					const batchResult = await this._embedBatchWithRetries(currentBatch, modelToUse)
-					allEmbeddings.push(...batchResult.embeddings)
-					usage.promptTokens += batchResult.usage.promptTokens
-					usage.totalTokens += batchResult.usage.totalTokens
-				} catch (error) {
-					console.error("Failed to process batch:", error)
-					throw new Error("Failed to create embeddings: batch processing error")
-				}
+				const batchResult = await this._embedBatchWithRetries(currentBatch, modelToUse)
+				allEmbeddings.push(...batchResult.embeddings)
+				usage.promptTokens += batchResult.usage.promptTokens
+				usage.totalTokens += batchResult.usage.totalTokens
 			}
 		}
 
@@ -114,13 +109,23 @@ export class OpenAiEmbedder extends OpenAiNativeHandler implements IEmbedder {
 				const isRateLimitError = error?.status === 429
 				const hasMoreAttempts = attempts < MAX_RETRIES - 1
 
-				if (isRateLimitError && hasMoreAttempts) {
-					const delayMs = INITIAL_DELAY_MS * Math.pow(2, attempts)
-					await new Promise((resolve) => setTimeout(resolve, delayMs))
+				// Log the error for debugging
+				console.error(`OpenAI embedder error (attempt ${attempts + 1}/${MAX_RETRIES}):`, error)
+
+				if (hasMoreAttempts) {
+					if (isRateLimitError) {
+						const delayMs = INITIAL_DELAY_MS * Math.pow(2, attempts)
+						console.warn(
+							`Rate limit hit, retrying in ${delayMs}ms (attempt ${attempts + 1}/${MAX_RETRIES})`,
+						)
+						await new Promise((resolve) => setTimeout(resolve, delayMs))
+					}
 					continue
 				}
 
-				throw error
+				// Provide more context in the error message using robust error extraction
+				const errorMessage = error?.message || (typeof error === "string" ? error : "Unknown error")
+				throw new Error(`Failed to create embeddings after ${MAX_RETRIES} attempts: ${errorMessage}`)
 			}
 		}