fix: recover from error state when Qdrant becomes available (#6661)

* fix: recover from error state when Qdrant becomes available - Add recoverFromError method to CodeIndexManager to clear error state and reset internal services - Update startIndexing handler to check for error state and recover before initialization - Add comprehensive tests for error recovery functionality Fixes #6660 * fix: address PR review comments for code indexing error recovery - Add race condition protection for multiple rapid clicks on Start Indexing button - Add error handling for setSystemState in recoverFromError method - Enhance JSDoc documentation for recoverFromError method - Add test cases for recoverFromError idempotency and error handling * refactor: move error recovery logic into startIndexing method - Moved error recovery from webviewMessageHandler into CodeIndexManager.startIndexing() - This ensures error recovery happens whenever indexing is started, not just from UI - Added race condition prevention flag within CodeIndexManager - Simplified webviewMessageHandler by removing error state checking - The startIndexing method now automatically recovers from error state before proceeding * fix: remove await from startIndexing calls and update JSDoc - startIndexing should never be awaited as it's a long-running background process - Added JSDoc warning to never await this method - Updated webviewMessageHandler to not await startIndexing calls * fix: use platform-agnostic paths in code-index manager tests for Windows compatibility --------- Co-authored-by: Roo Code <roomote@roocode.com> Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
2026-09-11 22:51:26 +00:00 · 2025-08-06 06:26:34 -07:00 · 2025-08-06 06:26:34 -07:00 · 34fb5b7c37
commit 34fb5b7c37
parent 142cdb5cb1
3 changed files with 300 additions and 24 deletions
--- a/src/core/webview/webviewMessageHandler.ts
+++ b/src/core/webview/webviewMessageHandler.ts
@ -2224,7 +2224,15 @@ export const webviewMessageHandler = async (
 						await manager.initialize(provider.contextProxy)
 					}

+					// startIndexing now handles error recovery internally
 					manager.startIndexing()
+
+					// If startIndexing recovered from error, we need to reinitialize
+					if (!manager.isInitialized) {
+						await manager.initialize(provider.contextProxy)
+						// Try starting again after initialization
+						manager.startIndexing()
+					}
 				}
 			} catch (error) {
 				provider.log(`Error starting indexing: ${error instanceof Error ? error.message : String(error)}`)
--- a/src/services/code-index/tests/manager.spec.ts
+++ b/src/services/code-index/tests/manager.spec.ts
@ -1,27 +1,36 @@
 import { CodeIndexManager } from "../manager"
 import { CodeIndexServiceFactory } from "../service-factory"
 import type { MockedClass } from "vitest"
+import * as path from "path"

 // Mock vscode module
-vi.mock("vscode", () => ({
-	window: {
-		activeTextEditor: null,
-	},
-	workspace: {
-		workspaceFolders: [
-			{
-				uri: { fsPath: "/test/workspace" },
-				name: "test",
-				index: 0,
-			},
-		],
-	},
-}))
+vi.mock("vscode", () => {
+	const testPath = require("path")
+	const testWorkspacePath = testPath.join(testPath.sep, "test", "workspace")
+	return {
+		window: {
+			activeTextEditor: null,
+		},
+		workspace: {
+			workspaceFolders: [
+				{
+					uri: { fsPath: testWorkspacePath },
+					name: "test",
+					index: 0,
+				},
+			],
+		},
+	}
+})

 // Mock only the essential dependencies
-vi.mock("../../../utils/path", () => ({
-	getWorkspacePath: vi.fn(() => "/test/workspace"),
-}))
+vi.mock("../../../utils/path", () => {
+	const testPath = require("path")
+	const testWorkspacePath = testPath.join(testPath.sep, "test", "workspace")
+	return {
+		getWorkspacePath: vi.fn(() => testWorkspacePath),
+	}
+})

 vi.mock("../state-manager", () => ({
 	CodeIndexStateManager: vi.fn().mockImplementation(() => ({
@ -48,6 +57,13 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
 	let mockContext: any
 	let manager: CodeIndexManager

+	// Define test paths for use in tests
+	const testWorkspacePath = path.join(path.sep, "test", "workspace")
+	const testExtensionPath = path.join(path.sep, "test", "extension")
+	const testStoragePath = path.join(path.sep, "test", "storage")
+	const testGlobalStoragePath = path.join(path.sep, "test", "global-storage")
+	const testLogPath = path.join(path.sep, "test", "log")
+
 	beforeEach(() => {
 		// Clear all instances before each test
 		CodeIndexManager.disposeAll()
@ -57,14 +73,14 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
 			workspaceState: {} as any,
 			globalState: {} as any,
 			extensionUri: {} as any,
-			extensionPath: "/test/extension",
+			extensionPath: testExtensionPath,
 			asAbsolutePath: vi.fn(),
 			storageUri: {} as any,
-			storagePath: "/test/storage",
+			storagePath: testStoragePath,
 			globalStorageUri: {} as any,
-			globalStoragePath: "/test/global-storage",
+			globalStoragePath: testGlobalStoragePath,
 			logUri: {} as any,
-			logPath: "/test/log",
+			logPath: testLogPath,
 			extensionMode: 3, // vscode.ExtensionMode.Test
 			secrets: {} as any,
 			environmentVariableCollection: {} as any,
@ -118,7 +134,7 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
 			// Mock service factory to handle _recreateServices call
 			const mockServiceFactoryInstance = {
 				configManager: mockConfigManager,
-				workspacePath: "/test/workspace",
+				workspacePath: testWorkspacePath,
 				cacheManager: mockCacheManager,
 				createEmbedder: vi.fn().mockReturnValue({ embedderInfo: { name: "openai" } }),
 				createVectorStore: vi.fn().mockReturnValue({}),
@ -192,7 +208,7 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
 			// Mock service factory to handle _recreateServices call
 			const mockServiceFactoryInstance = {
 				configManager: mockConfigManager,
-				workspacePath: "/test/workspace",
+				workspacePath: testWorkspacePath,
 				cacheManager: mockCacheManager,
 				createEmbedder: vi.fn().mockReturnValue({ embedderInfo: { name: "openai" } }),
 				createVectorStore: vi.fn().mockReturnValue({}),
@ -370,4 +386,199 @@ describe("CodeIndexManager - handleSettingsChange regression", () => {
 			expect(mockServiceFactoryInstance.validateEmbedder).not.toHaveBeenCalled()
 		})
 	})
+
+	describe("recoverFromError", () => {
+		let mockConfigManager: any
+		let mockCacheManager: any
+		let mockStateManager: any
+
+		beforeEach(() => {
+			// Mock config manager
+			mockConfigManager = {
+				loadConfiguration: vi.fn().mockResolvedValue({ requiresRestart: false }),
+				isFeatureConfigured: true,
+				isFeatureEnabled: true,
+				getConfig: vi.fn().mockReturnValue({
+					isConfigured: true,
+					embedderProvider: "openai",
+					modelId: "text-embedding-3-small",
+					openAiOptions: { openAiNativeApiKey: "test-key" },
+					qdrantUrl: "http://localhost:6333",
+					qdrantApiKey: "test-key",
+					searchMinScore: 0.4,
+				}),
+			}
+			;(manager as any)._configManager = mockConfigManager
+
+			// Mock cache manager
+			mockCacheManager = {
+				initialize: vi.fn(),
+				clearCacheFile: vi.fn(),
+			}
+			;(manager as any)._cacheManager = mockCacheManager
+
+			// Mock state manager
+			mockStateManager = (manager as any)._stateManager
+			mockStateManager.setSystemState = vi.fn()
+			mockStateManager.getCurrentStatus = vi.fn().mockReturnValue({
+				systemStatus: "Error",
+				message: "Failed during initial scan: fetch failed",
+				processedItems: 0,
+				totalItems: 0,
+				currentItemUnit: "items",
+			})
+
+			// Mock orchestrator and search service to simulate initialized state
+			;(manager as any)._orchestrator = { stopWatcher: vi.fn(), state: "Error" }
+			;(manager as any)._searchService = {}
+			;(manager as any)._serviceFactory = {}
+		})
+
+		it("should clear error state when recoverFromError is called", async () => {
+			// Act
+			await manager.recoverFromError()
+
+			// Assert
+			expect(mockStateManager.setSystemState).toHaveBeenCalledWith("Standby", "")
+		})
+
+		it("should reset internal service instances", async () => {
+			// Verify initial state
+			expect((manager as any)._configManager).toBeDefined()
+			expect((manager as any)._serviceFactory).toBeDefined()
+			expect((manager as any)._orchestrator).toBeDefined()
+			expect((manager as any)._searchService).toBeDefined()
+
+			// Act
+			await manager.recoverFromError()
+
+			// Assert - all service instances should be undefined
+			expect((manager as any)._configManager).toBeUndefined()
+			expect((manager as any)._serviceFactory).toBeUndefined()
+			expect((manager as any)._orchestrator).toBeUndefined()
+			expect((manager as any)._searchService).toBeUndefined()
+		})
+
+		it("should make manager report as not initialized after recovery", async () => {
+			// Verify initial state
+			expect(manager.isInitialized).toBe(true)
+
+			// Act
+			await manager.recoverFromError()
+
+			// Assert
+			expect(manager.isInitialized).toBe(false)
+		})
+
+		it("should allow re-initialization after recovery", async () => {
+			// Setup mock for re-initialization
+			const mockServiceFactoryInstance = {
+				createServices: vi.fn().mockReturnValue({
+					embedder: { embedderInfo: { name: "openai" } },
+					vectorStore: {},
+					scanner: {},
+					fileWatcher: {
+						onDidStartBatchProcessing: vi.fn(),
+						onBatchProgressUpdate: vi.fn(),
+						watch: vi.fn(),
+						stopWatcher: vi.fn(),
+						dispose: vi.fn(),
+					},
+				}),
+				validateEmbedder: vi.fn().mockResolvedValue({ valid: true }),
+			}
+			MockedCodeIndexServiceFactory.mockImplementation(() => mockServiceFactoryInstance as any)
+
+			// Act - recover from error
+			await manager.recoverFromError()
+
+			// Verify manager is not initialized
+			expect(manager.isInitialized).toBe(false)
+
+			// Mock context proxy for initialization
+			const mockContextProxy = {
+				getValue: vi.fn(),
+				setValue: vi.fn(),
+				storeSecret: vi.fn(),
+				getSecret: vi.fn(),
+				refreshSecrets: vi.fn().mockResolvedValue(undefined),
+				getGlobalState: vi.fn().mockReturnValue({
+					codebaseIndexEnabled: true,
+					codebaseIndexQdrantUrl: "http://localhost:6333",
+					codebaseIndexEmbedderProvider: "openai",
+					codebaseIndexEmbedderModelId: "text-embedding-3-small",
+					codebaseIndexEmbedderModelDimension: 1536,
+					codebaseIndexSearchMaxResults: 10,
+					codebaseIndexSearchMinScore: 0.4,
+				}),
+			}
+
+			// Re-initialize
+			await manager.initialize(mockContextProxy as any)
+
+			// Assert - manager should be initialized again
+			expect(manager.isInitialized).toBe(true)
+			expect(mockServiceFactoryInstance.createServices).toHaveBeenCalled()
+			expect(mockServiceFactoryInstance.validateEmbedder).toHaveBeenCalled()
+		})
+
+		it("should be safe to call when not in error state (idempotent)", async () => {
+			// Setup manager in non-error state
+			mockStateManager.getCurrentStatus.mockReturnValue({
+				systemStatus: "Standby",
+				message: "",
+				processedItems: 0,
+				totalItems: 0,
+				currentItemUnit: "items",
+			})
+
+			// Verify initial state is not error
+			const initialStatus = manager.getCurrentStatus()
+			expect(initialStatus.systemStatus).not.toBe("Error")
+
+			// Act - call recoverFromError when not in error state
+			await expect(manager.recoverFromError()).resolves.not.toThrow()
+
+			// Assert - should still clear state and service instances
+			expect(mockStateManager.setSystemState).toHaveBeenCalledWith("Standby", "")
+			expect((manager as any)._configManager).toBeUndefined()
+			expect((manager as any)._serviceFactory).toBeUndefined()
+			expect((manager as any)._orchestrator).toBeUndefined()
+			expect((manager as any)._searchService).toBeUndefined()
+		})
+
+		it("should continue recovery even if setSystemState throws", async () => {
+			// Setup state manager to throw on setSystemState
+			mockStateManager.setSystemState.mockImplementation(() => {
+				throw new Error("State update failed")
+			})
+
+			// Setup manager with service instances
+			;(manager as any)._configManager = mockConfigManager
+			;(manager as any)._serviceFactory = {}
+			;(manager as any)._orchestrator = { stopWatcher: vi.fn() }
+			;(manager as any)._searchService = {}
+
+			// Spy on console.error
+			const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
+
+			// Act - should not throw despite setSystemState error
+			await expect(manager.recoverFromError()).resolves.not.toThrow()
+
+			// Assert - error should be logged
+			expect(consoleErrorSpy).toHaveBeenCalledWith(
+				"Failed to clear error state during recovery:",
+				expect.any(Error),
+			)
+
+			// Assert - service instances should still be cleared
+			expect((manager as any)._configManager).toBeUndefined()
+			expect((manager as any)._serviceFactory).toBeUndefined()
+			expect((manager as any)._orchestrator).toBeUndefined()
+			expect((manager as any)._searchService).toBeUndefined()
+
+			// Cleanup
+			consoleErrorSpy.mockRestore()
+		})
+	})
 })
--- a/src/services/code-index/manager.ts
+++ b/src/services/code-index/manager.ts
@ -28,6 +28,9 @@ export class CodeIndexManager {
 	private _searchService: CodeIndexSearchService | undefined
 	private _cacheManager: CacheManager | undefined

+	// Flag to prevent race conditions during error recovery
+	private _isRecoveringFromError = false
+
 	public static getInstance(context: vscode.ExtensionContext, workspacePath?: string): CodeIndexManager | undefined {
 		// If workspacePath is not provided, try to get it from the active editor or first workspace folder
 		if (!workspacePath) {
@ -164,12 +167,26 @@ export class CodeIndexManager {

 	/**
 	 * Initiates the indexing process (initial scan and starts watcher).
+	 * Automatically recovers from error state if needed before starting.
+	 *
+	 * @important This method should NEVER be awaited as it starts a long-running background process.
+	 * The indexing will continue asynchronously and progress will be reported through events.
 	 */
-
 	public async startIndexing(): Promise<void> {
 		if (!this.isFeatureEnabled) {
 			return
 		}
+
+		// Check if we're in error state and recover if needed
+		const currentStatus = this.getCurrentStatus()
+		if (currentStatus.systemStatus === "Error") {
+			await this.recoverFromError()
+
+			// After recovery, we need to reinitialize since recoverFromError clears all services
+			// This will be handled by the caller (webviewMessageHandler) checking isInitialized
+			return
+		}
+
 		this.assertInitialized()
 		await this._orchestrator!.startIndexing()
 	}
@ -186,6 +203,46 @@ export class CodeIndexManager {
 		}
 	}

+	/**
+	 * Recovers from error state by clearing the error and resetting internal state.
+	 * This allows the manager to be re-initialized after a recoverable error.
+	 *
+	 * This method clears all service instances (configManager, serviceFactory, orchestrator, searchService)
+	 * to force a complete re-initialization on the next operation. This ensures a clean slate
+	 * after recovering from errors such as network failures or configuration issues.
+	 *
+	 * @remarks
+	 * - Safe to call even when not in error state (idempotent)
+	 * - Does not restart indexing automatically - call initialize() after recovery
+	 * - Service instances will be recreated on next initialize() call
+	 * - Prevents race conditions from multiple concurrent recovery attempts
+	 */
+	public async recoverFromError(): Promise<void> {
+		// Prevent race conditions from multiple rapid recovery attempts
+		if (this._isRecoveringFromError) {
+			return
+		}
+
+		this._isRecoveringFromError = true
+		try {
+			// Clear error state
+			this._stateManager.setSystemState("Standby", "")
+		} catch (error) {
+			// Log error but continue with recovery - clearing service instances is more important
+			console.error("Failed to clear error state during recovery:", error)
+		} finally {
+			// Force re-initialization by clearing service instances
+			// This ensures a clean slate even if state update failed
+			this._configManager = undefined
+			this._serviceFactory = undefined
+			this._orchestrator = undefined
+			this._searchService = undefined
+
+			// Reset the flag after recovery is complete
+			this._isRecoveringFromError = false
+		}
+	}
+
 	/**
 	 * Cleans up the manager instance.
 	 */