scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-07 08:26:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

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

Browse source 
- 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
This commit is contained in:
hannesrudolph 2025-06-11 17:04:35 -06:00
parent db66396e9b
commit 93dbf552f3
3 changed files with 47 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -273,7 +273,7 @@ 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
@ -281,14 +281,14 @@ describe("OpenAICompatibleEmbedder", () => {
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
@ -296,10 +296,10 @@ describe("OpenAICompatibleEmbedder", () => {
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 () => {

37
src/services/code-index/embedders/openai-compatible.ts
View file

@ -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}`)
}
}

31
src/services/code-index/embedders/openai.ts
View file

@ -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}`)
}
}