Skip to content

Commit 2517289

Browse files
committed
refactor: simplify validation helpers by removing unnecessary wrapper functions
- Removed getErrorMessageForConnectionError and inlined logic into handleValidationError - Removed isRateLimitError, logRateLimitRetry, and logEmbeddingError wrapper functions - Updated openai.ts and openai-compatible.ts to inline rate limit checking and logging - Reduced code complexity while maintaining all functionality - All 311 tests continue to pass
1 parent f9dea8b commit 2517289

File tree

3 files changed

+58
-81
lines changed

3 files changed

+58
-81
lines changed

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,7 @@ import {
88
} from "../constants"
99
import { getDefaultModelId, getModelQueryPrefix } from "../../../shared/embeddingModels"
1010
import { t } from "../../../i18n"
11-
import {
12-
withValidationErrorHandling,
13-
HttpError,
14-
formatEmbeddingError,
15-
isRateLimitError,
16-
logRateLimitRetry,
17-
logEmbeddingError,
18-
} from "../shared/validation-helpers"
11+
import { withValidationErrorHandling, HttpError, formatEmbeddingError } from "../shared/validation-helpers"
1912

2013
interface EmbeddingItem {
2114
embedding: string | number[]
@@ -293,15 +286,23 @@ export class OpenAICompatibleEmbedder implements IEmbedder {
293286
} catch (error) {
294287
const hasMoreAttempts = attempts < MAX_RETRIES - 1
295288

296-
if (isRateLimitError(error) && hasMoreAttempts) {
289+
// Check if it's a rate limit error
290+
const httpError = error as HttpError
291+
if (httpError?.status === 429 && hasMoreAttempts) {
297292
const delayMs = INITIAL_DELAY_MS * Math.pow(2, attempts)
298-
logRateLimitRetry(delayMs, attempts + 1, MAX_RETRIES)
293+
console.warn(
294+
t("embeddings:rateLimitRetry", {
295+
delayMs,
296+
attempt: attempts + 1,
297+
maxRetries: MAX_RETRIES,
298+
}),
299+
)
299300
await new Promise((resolve) => setTimeout(resolve, delayMs))
300301
continue
301302
}
302303

303304
// Log the error for debugging
304-
logEmbeddingError("OpenAI Compatible", error, attempts + 1, MAX_RETRIES)
305+
console.error(`OpenAI Compatible embedder error (attempt ${attempts + 1}/${MAX_RETRIES}):`, error)
305306

306307
// Format and throw the error
307308
throw formatEmbeddingError(error, MAX_RETRIES)

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,7 @@ import {
1010
} from "../constants"
1111
import { getModelQueryPrefix } from "../../../shared/embeddingModels"
1212
import { t } from "../../../i18n"
13-
import {
14-
withValidationErrorHandling,
15-
formatEmbeddingError,
16-
isRateLimitError,
17-
logRateLimitRetry,
18-
logEmbeddingError,
19-
} from "../shared/validation-helpers"
13+
import { withValidationErrorHandling, formatEmbeddingError, HttpError } from "../shared/validation-helpers"
2014

2115
/**
2216
* OpenAI implementation of the embedder interface with batching and rate limiting
@@ -147,15 +141,23 @@ export class OpenAiEmbedder extends OpenAiNativeHandler implements IEmbedder {
147141
} catch (error: any) {
148142
const hasMoreAttempts = attempts < MAX_RETRIES - 1
149143

150-
if (isRateLimitError(error) && hasMoreAttempts) {
144+
// Check if it's a rate limit error
145+
const httpError = error as HttpError
146+
if (httpError?.status === 429 && hasMoreAttempts) {
151147
const delayMs = INITIAL_DELAY_MS * Math.pow(2, attempts)
152-
logRateLimitRetry(delayMs, attempts + 1, MAX_RETRIES)
148+
console.warn(
149+
t("embeddings:rateLimitRetry", {
150+
delayMs,
151+
attempt: attempts + 1,
152+
maxRetries: MAX_RETRIES,
153+
}),
154+
)
153155
await new Promise((resolve) => setTimeout(resolve, delayMs))
154156
continue
155157
}
156158

157159
// Log the error for debugging
158-
logEmbeddingError("OpenAI", error, attempts + 1, MAX_RETRIES)
160+
console.error(`OpenAI embedder error (attempt ${attempts + 1}/${MAX_RETRIES}):`, error)
159161

160162
// Format and throw the error
161163
throw formatEmbeddingError(error, MAX_RETRIES)

src/services/code-index/shared/validation-helpers.ts

Lines changed: 34 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { t } from "../../../i18n"
2+
import { serializeError } from "serialize-error"
23

34
/**
45
* HTTP error interface for embedder errors
@@ -42,31 +43,6 @@ export function getErrorMessageForStatus(status: number | undefined, embedderTyp
4243
}
4344
}
4445

45-
/**
46-
* Maps connection error messages to appropriate error messages
47-
*/
48-
export function getErrorMessageForConnectionError(errorMessage: string | undefined): string | undefined {
49-
if (!errorMessage) return undefined
50-
51-
if (errorMessage.includes("ENOTFOUND") || errorMessage.includes("ECONNREFUSED")) {
52-
return "embeddings:validation.connectionFailed"
53-
}
54-
55-
if (errorMessage.includes("ETIMEDOUT") || errorMessage === "AbortError") {
56-
return "embeddings:validation.connectionFailed"
57-
}
58-
59-
if (errorMessage.includes("Failed to parse response JSON")) {
60-
return "embeddings:validation.invalidResponse"
61-
}
62-
63-
if (errorMessage.includes("HTTP 0:") || errorMessage === "No response") {
64-
return "embeddings:validation.connectionFailed"
65-
}
66-
67-
return undefined
68-
}
69-
7046
/**
7147
* Extracts status code from various error formats
7248
*/
@@ -85,6 +61,11 @@ export function extractStatusCode(error: any): number | undefined {
8561
}
8662
}
8763

64+
// Use serialize-error as fallback for complex objects
65+
const serialized = serializeError(error)
66+
if (serialized?.status) return serialized.status
67+
if (serialized?.response?.status) return serialized.response.status
68+
8869
return undefined
8970
}
9071

@@ -108,6 +89,12 @@ export function extractErrorMessage(error: any): string {
10889
}
10990
}
11091

92+
// Use serialize-error as fallback for complex objects
93+
const serialized = serializeError(error)
94+
if (serialized?.message) {
95+
return serialized.message
96+
}
97+
11198
return "Unknown error"
11299
}
113100

@@ -122,15 +109,18 @@ export function handleValidationError(
122109
beforeStandardHandling?: (error: any) => { valid: boolean; error: string } | undefined
123110
},
124111
): { valid: boolean; error: string } {
125-
// Allow custom handling first
112+
// Serialize the error to ensure we have access to all properties
113+
const serializedError = serializeError(error)
114+
115+
// Allow custom handling first (pass original error for backward compatibility)
126116
if (customHandlers?.beforeStandardHandling) {
127117
const customResult = customHandlers.beforeStandardHandling(error)
128118
if (customResult) return customResult
129119
}
130120

131-
// Extract status code and error message
132-
const statusCode = extractStatusCode(error)
133-
const errorMessage = extractErrorMessage(error)
121+
// Extract status code and error message from serialized error
122+
const statusCode = extractStatusCode(serializedError)
123+
const errorMessage = extractErrorMessage(serializedError)
134124

135125
// Check for status-based errors first
136126
const statusError = getErrorMessageForStatus(statusCode, embedderType)
@@ -139,9 +129,21 @@ export function handleValidationError(
139129
}
140130

141131
// Check for connection errors
142-
const connectionError = getErrorMessageForConnectionError(errorMessage)
143-
if (connectionError) {
144-
return { valid: false, error: connectionError }
132+
if (errorMessage) {
133+
if (
134+
errorMessage.includes("ENOTFOUND") ||
135+
errorMessage.includes("ECONNREFUSED") ||
136+
errorMessage.includes("ETIMEDOUT") ||
137+
errorMessage === "AbortError" ||
138+
errorMessage.includes("HTTP 0:") ||
139+
errorMessage === "No response"
140+
) {
141+
return { valid: false, error: "embeddings:validation.connectionFailed" }
142+
}
143+
144+
if (errorMessage.includes("Failed to parse response JSON")) {
145+
return { valid: false, error: "embeddings:validation.invalidResponse" }
146+
}
145147
}
146148

147149
// For generic errors, preserve the original error message if it's not a standard one
@@ -183,31 +185,3 @@ export function formatEmbeddingError(error: any, maxRetries: number): Error {
183185
return new Error(t("embeddings:failedWithError", { attempts: maxRetries, errorMessage }))
184186
}
185187
}
186-
187-
/**
188-
* Checks if an error is a rate limit error
189-
*/
190-
export function isRateLimitError(error: any): boolean {
191-
const httpError = error as HttpError
192-
return httpError?.status === 429
193-
}
194-
195-
/**
196-
* Logs retry attempt for rate limit errors
197-
*/
198-
export function logRateLimitRetry(delayMs: number, attempt: number, maxRetries: number): void {
199-
console.warn(
200-
t("embeddings:rateLimitRetry", {
201-
delayMs,
202-
attempt,
203-
maxRetries,
204-
}),
205-
)
206-
}
207-
208-
/**
209-
* Logs embedding error with context
210-
*/
211-
export function logEmbeddingError(embedderName: string, error: any, attempt: number, maxRetries: number): void {
212-
console.error(`${embedderName} embedder error (attempt ${attempt}/${maxRetries}):`, error)
213-
}

0 commit comments

Comments
 (0)