Skip to content

Commit 4217e2a

Browse files
committed
fix: aggregate file processing errors in telemetry to reduce volume
- Replace individual error telemetry events with aggregated reporting - Collect errors during batch processing and send single consolidated event - Include error count, type breakdown, and sanitized examples - Add comprehensive tests for error aggregation behavior
1 parent 6db7d02 commit 4217e2a

File tree

2 files changed

+285
-30
lines changed

2 files changed

+285
-30
lines changed

src/services/code-index/processors/__tests__/file-watcher.spec.ts

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import * as vscode from "vscode"
44

55
import { FileWatcher } from "../file-watcher"
6+
import { TelemetryService } from "@roo-code/telemetry"
7+
import { TelemetryEventName } from "@roo-code/types"
68

79
// Mock TelemetryService
810
vi.mock("../../../../../packages/telemetry/src/TelemetryService", () => ({
@@ -17,6 +19,14 @@ vi.mock("../../../../../packages/telemetry/src/TelemetryService", () => ({
1719
vi.mock("../../cache-manager")
1820
vi.mock("../../../core/ignore/RooIgnoreController")
1921
vi.mock("ignore")
22+
vi.mock("../parser", () => ({
23+
codeParser: {
24+
parseFile: vi.fn().mockResolvedValue([]),
25+
},
26+
}))
27+
vi.mock("../../../glob/ignore-utils", () => ({
28+
isPathInIgnoredDirectory: vi.fn().mockReturnValue(false),
29+
}))
2030

2131
// Mock vscode module
2232
vi.mock("vscode", () => ({
@@ -29,6 +39,10 @@ vi.mock("vscode", () => ({
2939
},
3040
},
3141
],
42+
fs: {
43+
stat: vi.fn().mockResolvedValue({ size: 1000 }),
44+
readFile: vi.fn().mockResolvedValue(Buffer.from("test content")),
45+
},
3246
},
3347
RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ base, pattern })),
3448
Uri: {
@@ -101,6 +115,7 @@ describe("FileWatcher", () => {
101115
mockVectorStore = {
102116
upsertPoints: vi.fn().mockResolvedValue(undefined),
103117
deletePointsByFilePath: vi.fn().mockResolvedValue(undefined),
118+
deletePointsByMultipleFilePaths: vi.fn().mockResolvedValue(undefined),
104119
}
105120

106121
mockIgnoreInstance = {
@@ -268,4 +283,196 @@ describe("FileWatcher", () => {
268283
expect(mockWatcher.dispose).toHaveBeenCalled()
269284
})
270285
})
286+
287+
describe("error aggregation", () => {
288+
beforeEach(() => {
289+
// Reset telemetry mock
290+
vi.mocked(TelemetryService.instance.captureEvent).mockClear()
291+
})
292+
293+
it("should aggregate file processing errors and send a single telemetry event", async () => {
294+
// Initialize the file watcher
295+
await fileWatcher.initialize()
296+
297+
// Mock processFile to throw errors for some files
298+
const processFileSpy = vi.spyOn(fileWatcher, "processFile")
299+
processFileSpy
300+
.mockRejectedValueOnce(new Error("File read error"))
301+
.mockRejectedValueOnce(new Error("Parse error"))
302+
.mockResolvedValueOnce({ path: "/mock/workspace/file3.ts", status: "skipped", reason: "Too large" })
303+
304+
// Trigger file creation events
305+
await mockOnDidCreate({ fsPath: "/mock/workspace/file1.ts" })
306+
await mockOnDidCreate({ fsPath: "/mock/workspace/file2.ts" })
307+
await mockOnDidCreate({ fsPath: "/mock/workspace/file3.ts" })
308+
309+
// Wait for batch processing
310+
await new Promise((resolve) => setTimeout(resolve, 600))
311+
312+
// Verify that only one aggregated telemetry event was sent
313+
const telemetryCalls = vi.mocked(TelemetryService.instance.captureEvent).mock.calls
314+
const codeIndexErrorCalls = telemetryCalls.filter((call) => call[0] === TelemetryEventName.CODE_INDEX_ERROR)
315+
316+
// Should have exactly one aggregated error event
317+
expect(codeIndexErrorCalls).toHaveLength(1)
318+
319+
const aggregatedEvent = codeIndexErrorCalls[0][1]
320+
expect(aggregatedEvent).toMatchObject({
321+
error: expect.stringContaining("Batch processing completed with 2 errors"),
322+
errorCount: 2,
323+
errorTypes: expect.objectContaining({
324+
Error: 2,
325+
}),
326+
sampleErrors: expect.arrayContaining([
327+
expect.objectContaining({
328+
path: expect.any(String),
329+
error: expect.any(String),
330+
location: "_processFilesAndPrepareUpserts",
331+
}),
332+
]),
333+
location: "processBatch_aggregated",
334+
})
335+
})
336+
337+
it("should not send telemetry event when no errors occur", async () => {
338+
// Initialize the file watcher
339+
await fileWatcher.initialize()
340+
341+
// Mock processFile to succeed for all files
342+
const processFileSpy = vi.spyOn(fileWatcher, "processFile")
343+
processFileSpy.mockResolvedValue({
344+
path: "/mock/workspace/file.ts",
345+
status: "processed_for_batching",
346+
pointsToUpsert: [],
347+
})
348+
349+
// Trigger file creation events
350+
await mockOnDidCreate({ fsPath: "/mock/workspace/file1.ts" })
351+
await mockOnDidCreate({ fsPath: "/mock/workspace/file2.ts" })
352+
353+
// Wait for batch processing
354+
await new Promise((resolve) => setTimeout(resolve, 600))
355+
356+
// Verify no telemetry events were sent
357+
const telemetryCalls = vi.mocked(TelemetryService.instance.captureEvent).mock.calls
358+
const codeIndexErrorCalls = telemetryCalls.filter((call) => call[0] === TelemetryEventName.CODE_INDEX_ERROR)
359+
360+
expect(codeIndexErrorCalls).toHaveLength(0)
361+
})
362+
363+
it("should include deletion errors in aggregated telemetry", async () => {
364+
// Initialize the file watcher
365+
await fileWatcher.initialize()
366+
367+
// Mock vector store to fail on deletion
368+
mockVectorStore.deletePointsByMultipleFilePaths.mockRejectedValueOnce(
369+
new Error("Database connection error"),
370+
)
371+
372+
// Trigger file deletion events
373+
await mockOnDidDelete({ fsPath: "/mock/workspace/file1.ts" })
374+
await mockOnDidDelete({ fsPath: "/mock/workspace/file2.ts" })
375+
376+
// Wait for batch processing
377+
await new Promise((resolve) => setTimeout(resolve, 600))
378+
379+
// Verify aggregated telemetry event includes deletion errors
380+
const telemetryCalls = vi.mocked(TelemetryService.instance.captureEvent).mock.calls
381+
const codeIndexErrorCalls = telemetryCalls.filter((call) => call[0] === TelemetryEventName.CODE_INDEX_ERROR)
382+
383+
expect(codeIndexErrorCalls).toHaveLength(1)
384+
385+
const aggregatedEvent = codeIndexErrorCalls[0][1]
386+
expect(aggregatedEvent).toMatchObject({
387+
error: expect.stringContaining("Batch processing completed with 2 errors"),
388+
errorCount: 2,
389+
sampleErrors: expect.arrayContaining([
390+
expect.objectContaining({
391+
location: "_handleBatchDeletions",
392+
}),
393+
]),
394+
})
395+
})
396+
397+
it("should include upsert errors in aggregated telemetry", async () => {
398+
// Initialize the file watcher
399+
await fileWatcher.initialize()
400+
401+
// Spy on processFile to make it return points for upserting
402+
const processFileSpy = vi.spyOn(fileWatcher, "processFile")
403+
processFileSpy.mockResolvedValue({
404+
path: "/mock/workspace/file.ts",
405+
status: "processed_for_batching",
406+
newHash: "abc123",
407+
pointsToUpsert: [
408+
{
409+
id: "test-id",
410+
vector: [0.1, 0.2, 0.3],
411+
payload: {
412+
filePath: "file.ts",
413+
codeChunk: "test code",
414+
startLine: 1,
415+
endLine: 10,
416+
},
417+
},
418+
],
419+
})
420+
421+
// Mock vector store to fail on upsert
422+
mockVectorStore.upsertPoints.mockRejectedValue(new Error("Vector dimension mismatch"))
423+
424+
// Trigger file creation event
425+
await mockOnDidCreate({ fsPath: "/mock/workspace/file.ts" })
426+
427+
// Wait for batch processing
428+
await new Promise((resolve) => setTimeout(resolve, 700))
429+
430+
// Verify aggregated telemetry event includes upsert errors
431+
const telemetryCalls = vi.mocked(TelemetryService.instance.captureEvent).mock.calls
432+
const codeIndexErrorCalls = telemetryCalls.filter((call) => call[0] === TelemetryEventName.CODE_INDEX_ERROR)
433+
434+
expect(codeIndexErrorCalls).toHaveLength(1)
435+
436+
const aggregatedEvent = codeIndexErrorCalls[0][1]
437+
expect(aggregatedEvent).toMatchObject({
438+
error: expect.stringContaining("Batch processing completed with 1 errors"),
439+
errorCount: 1,
440+
sampleErrors: expect.arrayContaining([
441+
expect.objectContaining({
442+
location: "_executeBatchUpsertOperations",
443+
}),
444+
]),
445+
})
446+
})
447+
448+
it("should limit sample errors to 3 in telemetry", async () => {
449+
// Initialize the file watcher
450+
await fileWatcher.initialize()
451+
452+
// Mock processFile to throw different errors
453+
const processFileSpy = vi.spyOn(fileWatcher, "processFile")
454+
for (let i = 0; i < 10; i++) {
455+
processFileSpy.mockRejectedValueOnce(new Error(`Error ${i + 1}`))
456+
}
457+
458+
// Trigger many file creation events
459+
for (let i = 0; i < 10; i++) {
460+
await mockOnDidCreate({ fsPath: `/mock/workspace/file${i + 1}.ts` })
461+
}
462+
463+
// Wait for batch processing
464+
await new Promise((resolve) => setTimeout(resolve, 600))
465+
466+
// Verify telemetry event has limited sample errors
467+
const telemetryCalls = vi.mocked(TelemetryService.instance.captureEvent).mock.calls
468+
const codeIndexErrorCalls = telemetryCalls.filter((call) => call[0] === TelemetryEventName.CODE_INDEX_ERROR)
469+
470+
expect(codeIndexErrorCalls).toHaveLength(1)
471+
472+
const aggregatedEvent = codeIndexErrorCalls[0][1]
473+
expect(aggregatedEvent).toBeDefined()
474+
expect(aggregatedEvent!.errorCount).toBe(10)
475+
expect(aggregatedEvent!.sampleErrors).toHaveLength(3) // Limited to 3 samples
476+
})
477+
})
271478
})

0 commit comments

Comments
 (0)