Enhance error handling for collection checks on initialize (#3937)

This commit is contained in:
Daniel 2025-05-24 15:23:43 -05:00 committed by GitHub
parent b3962246d3
commit def222f501
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 67 additions and 60 deletions

View file

@ -201,19 +201,23 @@ describe("QdrantVectorStore", () => {
expect(mockQdrantClientInstance.createPayloadIndex).toHaveBeenCalledTimes(5)
;(console.warn as jest.Mock).mockRestore() // Restore console.warn
})
it("should re-throw error from getCollection if it is not a 404 error", async () => {
it("should log warning for non-404 errors but still create collection", async () => {
const genericError = new Error("Generic Qdrant Error")
mockQdrantClientInstance.getCollection.mockRejectedValue(genericError)
jest.spyOn(console, "error").mockImplementation(() => {}) // Suppress console.error
jest.spyOn(console, "warn").mockImplementation(() => {}) // Suppress console.warn
await expect(vectorStore.initialize()).rejects.toThrow(genericError)
const result = await vectorStore.initialize()
expect(result).toBe(true) // Collection was created
expect(mockQdrantClientInstance.getCollection).toHaveBeenCalledTimes(1)
expect(mockQdrantClientInstance.createCollection).not.toHaveBeenCalled()
expect(mockQdrantClientInstance.createCollection).toHaveBeenCalledTimes(1)
expect(mockQdrantClientInstance.deleteCollection).not.toHaveBeenCalled()
expect(mockQdrantClientInstance.createPayloadIndex).not.toHaveBeenCalled()
expect(console.error).toHaveBeenCalledTimes(2) // Once in the try/catch for getCollection, once in the outer try/catch
;(console.error as jest.Mock).mockRestore()
expect(mockQdrantClientInstance.createPayloadIndex).toHaveBeenCalledTimes(5)
expect(console.warn).toHaveBeenCalledWith(
expect.stringContaining(`Warning during getCollectionInfo for "${expectedCollectionName}"`),
genericError.message,
)
;(console.warn as jest.Mock).mockRestore()
})
it("should re-throw error from createCollection when no collection initially exists", async () => {
mockQdrantClientInstance.getCollection.mockRejectedValue({
@ -260,7 +264,7 @@ describe("QdrantVectorStore", () => {
for (let i = 0; i <= 4; i++) {
expect(console.warn).toHaveBeenCalledWith(
expect.stringContaining(`Could not create payload index for pathSegments.${i}`),
indexError,
indexError.message,
)
}
@ -322,17 +326,20 @@ describe("QdrantVectorStore", () => {
expect(mockQdrantClientInstance.getCollection).toHaveBeenCalledWith(expectedCollectionName)
})
it("should return false and log error for non-404 errors", async () => {
it("should return false and log warning for non-404 errors", async () => {
const genericError = new Error("Network error")
mockQdrantClientInstance.getCollection.mockRejectedValue(genericError)
jest.spyOn(console, "error").mockImplementation(() => {})
jest.spyOn(console, "warn").mockImplementation(() => {})
const result = await vectorStore.collectionExists()
expect(result).toBe(false)
expect(mockQdrantClientInstance.getCollection).toHaveBeenCalledTimes(1)
expect(console.error).toHaveBeenCalledWith("Error checking collection existence:", genericError)
;(console.error as jest.Mock).mockRestore()
expect(console.warn).toHaveBeenCalledWith(
expect.stringContaining(`Warning during getCollectionInfo for "${expectedCollectionName}"`),
genericError.message,
)
;(console.warn as jest.Mock).mockRestore()
})
describe("collectionExists", () => {
// Test scenarios for collectionExists will go here

View file

@ -1,4 +1,4 @@
import { QdrantClient } from "@qdrant/js-client-rest"
import { QdrantClient, Schemas } from "@qdrant/js-client-rest"
import { createHash } from "crypto"
import * as path from "path"
import { getWorkspacePath } from "../../../utils/path"
@ -37,30 +37,50 @@ export class QdrantVectorStore implements IVectorStore {
this.collectionName = `ws-${hash.substring(0, 16)}`
}
private async getCollectionInfo(): Promise<Schemas["CollectionInfo"] | null> {
try {
const collectionInfo = await this.client.getCollection(this.collectionName)
return collectionInfo
} catch (error: unknown) {
if (error instanceof Error) {
console.warn(
`[QdrantVectorStore] Warning during getCollectionInfo for "${this.collectionName}". Collection may not exist or another error occurred:`,
error.message,
)
}
return null
}
}
/**
* Initializes the vector store
* @returns Promise resolving to boolean indicating if a new collection was created
*/
async initialize(): Promise<boolean> {
let created = false
try {
let created = false
const collectionInfo = await this.getCollectionInfo()
try {
// Directly attempt to fetch the specific collection
const collectionInfo = await this.client.getCollection(this.collectionName)
// Collection exists - check if vector size matches
if (collectionInfo === null) {
// Collection info not retrieved (assume not found or inaccessible), create it
await this.client.createCollection(this.collectionName, {
vectors: {
size: this.vectorSize,
distance: this.DISTANCE_METRIC,
},
})
created = true
} else {
// Collection exists, check vector size
const existingVectorSize = collectionInfo.config?.params?.vectors?.size
if (existingVectorSize === this.vectorSize) {
// Collection exists and has correct vector size
created = false
created = false // Exists and correct
} else {
// Collection exists but has wrong vector size - recreate it
// Exists but wrong vector size, recreate
console.warn(
`[QdrantVectorStore] Collection ${this.collectionName} exists with vector size ${existingVectorSize}, but expected ${this.vectorSize}. Recreating collection.`,
)
await this.client.deleteCollection(this.collectionName)
await this.client.deleteCollection(this.collectionName) // Known to exist
await this.client.createCollection(this.collectionName, {
vectors: {
size: this.vectorSize,
@ -69,42 +89,31 @@ export class QdrantVectorStore implements IVectorStore {
})
created = true
}
} catch (error: any) {
// Check if this is a "Not Found" error (collection doesn't exist)
if (error?.response?.status === 404) {
// Collection doesn't exist - create it
await this.client.createCollection(this.collectionName, {
vectors: {
size: this.vectorSize,
distance: this.DISTANCE_METRIC,
},
})
created = true
} else {
// Other error - log and re-throw
console.error(`[QdrantVectorStore] Error checking collection ${this.collectionName}:`, error)
throw error
}
}
// Create payload indexes for pathSegments up to depth 5
// Create payload indexes
for (let i = 0; i <= 4; i++) {
try {
await this.client.createPayloadIndex(this.collectionName, {
field_name: `pathSegments.${i}`,
field_schema: "keyword",
})
} catch (indexError) {
console.warn(
`[QdrantVectorStore] Could not create payload index for pathSegments.${i} on ${this.collectionName}. It might already exist or there was an issue.`,
indexError,
)
} catch (indexError: any) {
const errorMessage = (indexError?.message || "").toLowerCase()
if (!errorMessage.includes("already exists")) {
console.warn(
`[QdrantVectorStore] Could not create payload index for pathSegments.${i} on ${this.collectionName}. Details:`,
indexError?.message || indexError,
)
}
}
}
return created
} catch (error) {
console.error("Failed to initialize Qdrant collection:", error)
} catch (error: any) {
console.error(
`[QdrantVectorStore] Failed to initialize Qdrant collection "${this.collectionName}":`,
error?.message || error,
)
throw error
}
}
@ -295,16 +304,7 @@ export class QdrantVectorStore implements IVectorStore {
* @returns Promise resolving to boolean indicating if the collection exists
*/
async collectionExists(): Promise<boolean> {
try {
// Prefer direct API call if supported
await this.client.getCollection(this.collectionName)
return true
} catch (error: any) {
if (error?.response?.status === 404) {
return false
}
console.error("Error checking collection existence:", error)
return false
}
const collectionInfo = await this.getCollectionInfo()
return collectionInfo !== null
}
}