Skip to content

Commit 6159b31

Browse files
roomoteroomote
committed
fix: preserve indexing progress when rate limit (429) errors occur
- Modified orchestrator to detect 429 errors and skip cache/vector store cleanup - Added new translation key for rate limit error messages - Added comprehensive tests to verify the behavior - Fixes #6650
1 parent 2882d99 commit 6159b31

File tree

3 files changed

+257
-16
lines changed

3 files changed

+257
-16
lines changed

‎src/i18n/locales/en/embeddings.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
"fileWatcherStarted": "File watcher started.",
6060
"fileWatcherStopped": "File watcher stopped.",
6161
"failedDuringInitialScan": "Failed during initial scan: {{errorMessage}}",
62+
"rateLimitError": "Indexing paused due to rate limit: {{errorMessage}}. Progress has been preserved. Please try again later.",
6263
"unknownError": "Unknown error",
6364
"indexingRequiresWorkspace": "Indexing requires an open workspace folder"
6465
}

‎src/services/code-index/__tests__/orchestrator.spec.ts‎

Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
2+
import { CodeIndexOrchestrator } from "../orchestrator"
3+
import { CodeIndexConfigManager } from "../config-manager"
4+
import { CodeIndexStateManager } from "../state-manager"
5+
import { CacheManager } from "../cache-manager"
6+
import { IVectorStore, IFileWatcher } from "../interfaces"
7+
import { DirectoryScanner } from "../processors"
8+
import * as vscode from "vscode"
9+
10+
// Mock vscode module
11+
vi.mock("vscode", () => ({
12+
workspace: {
13+
workspaceFolders: [
14+
{
15+
uri: { fsPath: "/test/workspace" },
16+
name: "test",
17+
index: 0,
18+
},
19+
],
20+
},
21+
}))
22+
23+
// Mock TelemetryService
24+
vi.mock("@roo-code/telemetry", () => ({
25+
TelemetryService: {
26+
instance: {
27+
captureEvent: vi.fn(),
28+
},
29+
},
30+
}))
31+
32+
// Mock i18n
33+
vi.mock("../../../i18n", () => ({
34+
t: vi.fn((key: string, params?: any) => {
35+
if (key === "embeddings:orchestrator.rateLimitError") {
36+
return `Indexing paused due to rate limit: ${params?.errorMessage}. Progress has been preserved. Please try again later.`
37+
}
38+
if (key === "embeddings:orchestrator.failedDuringInitialScan") {
39+
return `Failed during initial scan: ${params?.errorMessage}`
40+
}
41+
if (key === "embeddings:orchestrator.unknownError") {
42+
return "Unknown error"
43+
}
44+
return key
45+
}),
46+
}))
47+
48+
describe("CodeIndexOrchestrator - Rate Limit Error Handling", () => {
49+
let orchestrator: CodeIndexOrchestrator
50+
let mockConfigManager: any
51+
let mockStateManager: any
52+
let mockCacheManager: any
53+
let mockVectorStore: any
54+
let mockScanner: any
55+
let mockFileWatcher: any
56+
57+
beforeEach(() => {
58+
// Create mock instances
59+
mockConfigManager = {
60+
isFeatureConfigured: true,
61+
isFeatureEnabled: true,
62+
}
63+
64+
mockStateManager = {
65+
setSystemState: vi.fn(),
66+
state: "Standby",
67+
}
68+
69+
mockCacheManager = {
70+
clearCacheFile: vi.fn(),
71+
initialize: vi.fn(),
72+
}
73+
74+
mockVectorStore = {
75+
initialize: vi.fn().mockResolvedValue(false), // Return false to indicate collection already exists
76+
clearCollection: vi.fn(),
77+
deleteCollection: vi.fn(),
78+
}
79+
80+
mockScanner = {
81+
scanDirectory: vi.fn(),
82+
}
83+
84+
mockFileWatcher = {
85+
initialize: vi.fn(),
86+
onDidStartBatchProcessing: vi.fn().mockReturnValue({ dispose: vi.fn() }),
87+
onBatchProgressUpdate: vi.fn().mockReturnValue({ dispose: vi.fn() }),
88+
onDidFinishBatchProcessing: vi.fn().mockReturnValue({ dispose: vi.fn() }),
89+
dispose: vi.fn(),
90+
}
91+
92+
// Create orchestrator instance
93+
orchestrator = new CodeIndexOrchestrator(
94+
mockConfigManager as CodeIndexConfigManager,
95+
mockStateManager as CodeIndexStateManager,
96+
"/test/workspace",
97+
mockCacheManager as CacheManager,
98+
mockVectorStore as IVectorStore,
99+
mockScanner as DirectoryScanner,
100+
mockFileWatcher as IFileWatcher,
101+
)
102+
})
103+
104+
afterEach(() => {
105+
vi.clearAllMocks()
106+
})
107+
108+
it("should preserve cache and not clear vector store on 429 rate limit error", async () => {
109+
// Create a 429 error
110+
const rateLimitError = new Error("Rate limit exceeded") as any
111+
rateLimitError.status = 429
112+
113+
// Mock scanner to throw rate limit error
114+
mockScanner.scanDirectory.mockRejectedValue(rateLimitError)
115+
116+
// Start indexing
117+
await orchestrator.startIndexing()
118+
119+
// Verify that cache was NOT cleared
120+
expect(mockCacheManager.clearCacheFile).not.toHaveBeenCalled()
121+
122+
// Verify that vector store was NOT cleared
123+
expect(mockVectorStore.clearCollection).not.toHaveBeenCalled()
124+
125+
// Verify that the appropriate error message was set
126+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith(
127+
"Error",
128+
expect.stringContaining("Indexing paused due to rate limit"),
129+
)
130+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith(
131+
"Error",
132+
expect.stringContaining("Progress has been preserved"),
133+
)
134+
})
135+
136+
it("should clear cache and vector store on non-rate-limit errors", async () => {
137+
// Create a generic error (not 429)
138+
const genericError = new Error("Connection failed")
139+
140+
// Mock scanner to throw generic error
141+
mockScanner.scanDirectory.mockRejectedValue(genericError)
142+
143+
// Start indexing
144+
await orchestrator.startIndexing()
145+
146+
// Verify that cache WAS cleared
147+
expect(mockCacheManager.clearCacheFile).toHaveBeenCalled()
148+
149+
// Verify that vector store WAS cleared
150+
expect(mockVectorStore.clearCollection).toHaveBeenCalled()
151+
152+
// Verify that the appropriate error message was set
153+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith(
154+
"Error",
155+
expect.stringContaining("Failed during initial scan"),
156+
)
157+
})
158+
159+
it("should handle 429 errors with response.status property", async () => {
160+
// Create a 429 error with response.status property
161+
const rateLimitError = new Error("Rate limit exceeded") as any
162+
rateLimitError.response = { status: 429 }
163+
164+
// Mock scanner to throw rate limit error
165+
mockScanner.scanDirectory.mockRejectedValue(rateLimitError)
166+
167+
// Start indexing
168+
await orchestrator.startIndexing()
169+
170+
// Verify that cache was NOT cleared
171+
expect(mockCacheManager.clearCacheFile).not.toHaveBeenCalled()
172+
173+
// Verify that vector store was NOT cleared
174+
expect(mockVectorStore.clearCollection).not.toHaveBeenCalled()
175+
176+
// Verify that the appropriate error message was set
177+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith(
178+
"Error",
179+
expect.stringContaining("Indexing paused due to rate limit"),
180+
)
181+
})
182+
183+
it("should handle errors during vector store cleanup gracefully", async () => {
184+
// Create a generic error (not 429)
185+
const genericError = new Error("Connection failed")
186+
187+
// Mock scanner to throw generic error
188+
mockScanner.scanDirectory.mockRejectedValue(genericError)
189+
190+
// Mock vector store to throw error during cleanup
191+
mockVectorStore.clearCollection.mockRejectedValue(new Error("Cleanup failed"))
192+
193+
// Start indexing
194+
await orchestrator.startIndexing()
195+
196+
// Verify that cache WAS still cleared even if vector store cleanup failed
197+
expect(mockCacheManager.clearCacheFile).toHaveBeenCalled()
198+
199+
// Verify that the appropriate error message was set
200+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith(
201+
"Error",
202+
expect.stringContaining("Failed during initial scan"),
203+
)
204+
})
205+
206+
it("should clear cache when vector store creates new collection", async () => {
207+
// Mock vector store to return true (new collection created)
208+
mockVectorStore.initialize.mockResolvedValue(true)
209+
210+
// Mock successful scan
211+
mockScanner.scanDirectory.mockImplementation(async () => {
212+
return {
213+
stats: { processed: 1, skipped: 0 },
214+
totalBlockCount: 1,
215+
}
216+
})
217+
218+
// Start indexing
219+
await orchestrator.startIndexing()
220+
221+
// Verify that cache WAS cleared when new collection is created
222+
expect(mockCacheManager.clearCacheFile).toHaveBeenCalled()
223+
224+
// Verify that the success state was set
225+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith("Indexed", expect.any(String))
226+
})
227+
})

‎src/services/code-index/orchestrator.ts‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { CacheManager } from "./cache-manager"
88
import { TelemetryService } from "@roo-code/telemetry"
99
import { TelemetryEventName } from "@roo-code/types"
1010
import { t } from "../../i18n"
11+
import { extractStatusCode } from "./shared/validation-helpers"
1112

1213
/**
1314
* Manages the code indexing workflow, coordinating between different services and managers.
@@ -210,25 +211,37 @@ export class CodeIndexOrchestrator {
210211
stack: error instanceof Error ? error.stack : undefined,
211212
location: "startIndexing",
212213
})
213-
try {
214-
await this.vectorStore.clearCollection()
215-
} catch (cleanupError) {
216-
console.error("[CodeIndexOrchestrator] Failed to clean up after error:", cleanupError)
217-
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {
218-
error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError),
219-
stack: cleanupError instanceof Error ? cleanupError.stack : undefined,
220-
location: "startIndexing.cleanup",
221-
})
214+
215+
// Check if this is a rate limit error (429)
216+
const statusCode = extractStatusCode(error)
217+
const isRateLimitError = statusCode === 429
218+
219+
// Only clear vector store and cache if it's NOT a rate limit error
220+
if (!isRateLimitError) {
221+
try {
222+
await this.vectorStore.clearCollection()
223+
} catch (cleanupError) {
224+
console.error("[CodeIndexOrchestrator] Failed to clean up after error:", cleanupError)
225+
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {
226+
error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError),
227+
stack: cleanupError instanceof Error ? cleanupError.stack : undefined,
228+
location: "startIndexing.cleanup",
229+
})
230+
}
231+
232+
await this.cacheManager.clearCacheFile()
222233
}
223234

224-
await this.cacheManager.clearCacheFile()
235+
// Set appropriate error message based on error type
236+
const errorMessage = isRateLimitError
237+
? t("embeddings:orchestrator.rateLimitError", {
238+
errorMessage: error.message || t("embeddings:orchestrator.unknownError"),
239+
})
240+
: t("embeddings:orchestrator.failedDuringInitialScan", {
241+
errorMessage: error.message || t("embeddings:orchestrator.unknownError"),
242+
})
225243

226-
this.stateManager.setSystemState(
227-
"Error",
228-
t("embeddings:orchestrator.failedDuringInitialScan", {
229-
errorMessage: error.message || t("embeddings:orchestrator.unknownError"),
230-
}),
231-
)
244+
this.stateManager.setSystemState("Error", errorMessage)
232245
this.stopWatcher()
233246
} finally {
234247
this._isProcessing = false

0 commit comments

Comments
 (0)