Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,26 @@ vitest.mock("../../../../i18n", () => ({
"embeddings:textExceedsTokenLimit": `Text at index ${params?.index} exceeds maximum token limit (${params?.itemTokens} > ${params?.maxTokens}). Skipping.`,
"embeddings:rateLimitRetry": `Rate limit hit, retrying in ${params?.delayMs}ms (attempt ${params?.attempt}/${params?.maxRetries})`,
"embeddings:unknownError": "Unknown error",
"common:errors.api.invalidKeyInvalidChars":
"API key contains invalid characters. Please check your API key for special characters.",
}
return translations[key] || key
},
}))

// Mock i18n/setup module used by the error handler
vitest.mock("../../../../i18n/setup", () => ({
default: {
t: (key: string) => {
const translations: Record<string, string> = {
"common:errors.api.invalidKeyInvalidChars":
"API key contains invalid characters. Please check your API key for special characters.",
}
return translations[key] || key
},
},
}))

const MockedOpenAI = OpenAI as MockedClass<typeof OpenAI>

describe("OpenAICompatibleEmbedder", () => {
Expand Down Expand Up @@ -114,6 +129,22 @@ describe("OpenAICompatibleEmbedder", () => {
"embeddings:validation.baseUrlRequired",
)
})

it("should handle API key with invalid characters (ByteString conversion error)", () => {
// API key with special characters that cause ByteString conversion error
const invalidApiKey = "sk-test•invalid" // Contains bullet character (U+2022)

// Mock the OpenAI constructor to throw ByteString error
MockedOpenAI.mockImplementationOnce(() => {
throw new Error(
"Cannot convert argument to a ByteString because the character at index 7 has a value of 8226 which is greater than 255.",
)
})

expect(() => new OpenAICompatibleEmbedder(testBaseUrl, invalidApiKey, testModelId)).toThrow(
"API key contains invalid characters",
)
})
})

describe("embedderInfo", () => {
Expand Down
17 changes: 13 additions & 4 deletions src/services/code-index/embedders/openai-compatible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { withValidationErrorHandling, HttpError, formatEmbeddingError } from "..
import { TelemetryEventName } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
import { Mutex } from "async-mutex"
import { handleOpenAIError } from "../../../api/providers/utils/openai-error-handler"

interface EmbeddingItem {
embedding: string | number[]
Expand Down Expand Up @@ -66,10 +67,18 @@ export class OpenAICompatibleEmbedder implements IEmbedder {

this.baseUrl = baseUrl
this.apiKey = apiKey
this.embeddingsClient = new OpenAI({
baseURL: baseUrl,
apiKey: apiKey,
})

// Wrap OpenAI client creation to handle invalid API key characters
try {
this.embeddingsClient = new OpenAI({
baseURL: baseUrl,
apiKey: apiKey,
})
} catch (error) {
// Use the error handler to transform ByteString conversion errors
throw handleOpenAIError(error, "OpenAI Compatible")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent use of the existing error handler! The try-catch pattern here is clean and consistent with the codebase.

}

this.defaultModelId = modelId || getDefaultModelId("openai-compatible")
// Cache the URL type check for performance
this.isFullUrl = this.isFullEndpointUrl(baseUrl)
Expand Down
11 changes: 10 additions & 1 deletion src/services/code-index/embedders/openai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { t } from "../../../i18n"
import { withValidationErrorHandling, formatEmbeddingError, HttpError } from "../shared/validation-helpers"
import { TelemetryEventName } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
import { handleOpenAIError } from "../../../api/providers/utils/openai-error-handler"

/**
* OpenAI implementation of the embedder interface with batching and rate limiting
Expand All @@ -28,7 +29,15 @@ export class OpenAiEmbedder extends OpenAiNativeHandler implements IEmbedder {
constructor(options: ApiHandlerOptions & { openAiEmbeddingModelId?: string }) {
super(options)
const apiKey = this.options.openAiNativeApiKey ?? "not-provided"
this.embeddingsClient = new OpenAI({ apiKey })

// Wrap OpenAI client creation to handle invalid API key characters
try {
this.embeddingsClient = new OpenAI({ apiKey })
} catch (error) {
// Use the error handler to transform ByteString conversion errors
throw handleOpenAIError(error, "OpenAI")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good implementation of the error handler! Could we also add a test case for this in the OpenAI embedder's test file to ensure the ByteString error is properly handled here as well? I see we have a test for openai-compatible.spec.ts but not for the regular OpenAI embedder.

}

this.defaultModelId = options.openAiEmbeddingModelId || "text-embedding-3-small"
}

Expand Down
Loading