diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 3419cb85652..3e27a5cbb6a 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -2217,7 +2217,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)}`) diff --git a/src/services/code-index/__tests__/manager.spec.ts b/src/services/code-index/__tests__/manager.spec.ts index 3995825f707..49f725f69b2 100644 --- 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() + }) + }) }) diff --git a/src/services/code-index/manager.ts b/src/services/code-index/manager.ts index 027734d2132..1257b747c67 100644 --- 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 { 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 { + // 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. */