From a629172910d9862551ff2b50b70cbe93599705df Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 1 Aug 2025 21:48:41 +0000 Subject: [PATCH] feat: add persistent retry queue for failed telemetry events - Implement TelemetryQueue class with FIFO queue structure - Add persistence using VSCode globalState - Integrate queue into TelemetryClient for automatic retry - Process pending events when new events are captured - Add comprehensive test coverage for queue functionality - Limit queue size to 1000 events and max 3 retries per event Fixes #4940 --- packages/cloud/src/CloudService.ts | 2 +- packages/cloud/src/TelemetryClient.ts | 90 +++- packages/cloud/src/TelemetryQueue.ts | 186 +++++++ .../__tests__/TelemetryClient.queue.test.ts | 504 ++++++++++++++++++ .../src/__tests__/TelemetryClient.test.ts | 128 +++-- .../src/__tests__/TelemetryQueue.test.ts | 341 ++++++++++++ 6 files changed, 1194 insertions(+), 57 deletions(-) create mode 100644 packages/cloud/src/TelemetryQueue.ts create mode 100644 packages/cloud/src/__tests__/TelemetryClient.queue.test.ts create mode 100644 packages/cloud/src/__tests__/TelemetryQueue.test.ts diff --git a/packages/cloud/src/CloudService.ts b/packages/cloud/src/CloudService.ts index ff33671a40..d88f905a7a 100644 --- a/packages/cloud/src/CloudService.ts +++ b/packages/cloud/src/CloudService.ts @@ -87,7 +87,7 @@ export class CloudService extends EventEmitter implements vs this.settingsService = cloudSettingsService } - this.telemetryClient = new TelemetryClient(this.authService, this.settingsService) + this.telemetryClient = new TelemetryClient(this.context, this.authService, this.settingsService) this.shareService = new ShareService(this.authService, this.settingsService, this.log) try { diff --git a/packages/cloud/src/TelemetryClient.ts b/packages/cloud/src/TelemetryClient.ts index e33843a30c..70086a0930 100644 --- a/packages/cloud/src/TelemetryClient.ts +++ b/packages/cloud/src/TelemetryClient.ts @@ -1,3 +1,4 @@ +import * as vscode from "vscode" import { TelemetryEventName, type TelemetryEvent, @@ -9,9 +10,13 @@ import { BaseTelemetryClient } from "@roo-code/telemetry" import { getRooCodeApiUrl } from "./Config" import type { AuthService } from "./auth" import type { SettingsService } from "./SettingsService" +import { TelemetryQueue } from "./TelemetryQueue" export class TelemetryClient extends BaseTelemetryClient { + private queue: TelemetryQueue + constructor( + private context: vscode.ExtensionContext, private authService: AuthService, private settingsService: SettingsService, debug = false, @@ -23,18 +28,19 @@ export class TelemetryClient extends BaseTelemetryClient { }, debug, ) + this.queue = new TelemetryQueue(context, debug) } - private async fetch(path: string, options: RequestInit) { + private async fetch(path: string, options: RequestInit): Promise { if (!this.authService.isAuthenticated()) { - return + return undefined } const token = this.authService.getSessionToken() if (!token) { console.error(`[TelemetryClient#fetch] Unauthorized: No session token available.`) - return + return undefined } const response = await fetch(`${getRooCodeApiUrl()}/api/${path}`, { @@ -47,6 +53,8 @@ export class TelemetryClient extends BaseTelemetryClient { `[TelemetryClient#fetch] ${options.method} ${path} -> ${response.status} ${response.statusText}`, ) } + + return response } public override async capture(event: TelemetryEvent) { @@ -77,10 +85,80 @@ export class TelemetryClient extends BaseTelemetryClient { return } + // Add event to queue + await this.queue.enqueue(result.data) + + // Process queue asynchronously if not already processing + if (!this.queue.isProcessingQueue()) { + // Don't await - let it process in the background + this.processQueue().catch((error) => { + if (this.debug) { + console.error(`[TelemetryClient#capture] Error processing queue:`, error) + } + }) + } + } + + /** + * Processes the telemetry queue, sending events to the cloud service + */ + private async processQueue(): Promise { + if (!this.authService.isAuthenticated()) { + if (this.debug) { + console.info("[TelemetryClient#processQueue] Skipping: Not authenticated") + } + return + } + + this.queue.setProcessingState(true) + try { - await this.fetch(`events`, { method: "POST", body: JSON.stringify(result.data) }) - } catch (error) { - console.error(`[TelemetryClient#capture] Error sending telemetry event: ${error}`) + while (true) { + const queuedEvent = await this.queue.peek() + if (!queuedEvent) { + break // Queue is empty + } + + try { + // Attempt to send the event + const response = await this.fetch(`events`, { + method: "POST", + body: JSON.stringify(queuedEvent.event), + }) + + // Check if response indicates success (fetch doesn't throw on HTTP errors) + if (response === undefined || (response && response.ok !== false)) { + // Success - remove from queue + await this.queue.dequeue(queuedEvent.id) + + if (this.debug) { + console.info(`[TelemetryClient#processQueue] Successfully sent event ${queuedEvent.id}`) + } + } else { + // HTTP error - mark as failed + await this.queue.markFailed(queuedEvent.id) + + if (this.debug) { + console.error(`[TelemetryClient#processQueue] HTTP error for event ${queuedEvent.id}`) + } + + // Stop processing on error to avoid rapid retry loops + break + } + } catch (error) { + // Network or other error - mark as failed and move to end of queue + await this.queue.markFailed(queuedEvent.id) + + if (this.debug) { + console.error(`[TelemetryClient#processQueue] Failed to send event ${queuedEvent.id}:`, error) + } + + // Stop processing on error to avoid rapid retry loops + break + } + } + } finally { + this.queue.setProcessingState(false) } } diff --git a/packages/cloud/src/TelemetryQueue.ts b/packages/cloud/src/TelemetryQueue.ts new file mode 100644 index 0000000000..eabf8ad7d8 --- /dev/null +++ b/packages/cloud/src/TelemetryQueue.ts @@ -0,0 +1,186 @@ +import * as vscode from "vscode" +import { randomUUID } from "crypto" +import type { RooCodeTelemetryEvent } from "@roo-code/types" + +export interface QueuedTelemetryEvent { + id: string + event: RooCodeTelemetryEvent + timestamp: number + retryCount: number +} + +export class TelemetryQueue { + private static readonly QUEUE_KEY = "rooCode.telemetryQueue" + private static readonly MAX_QUEUE_SIZE = 1000 // Prevent unbounded growth + private static readonly MAX_RETRY_COUNT = 3 // Limit retries per event + + private context: vscode.ExtensionContext + private isProcessing = false + private debug: boolean + + constructor(context: vscode.ExtensionContext, debug = false) { + this.context = context + this.debug = debug + } + + /** + * Adds a telemetry event to the queue + */ + public async enqueue(event: RooCodeTelemetryEvent): Promise { + const queue = await this.getQueue() + + // Prevent unbounded growth + if (queue.length >= TelemetryQueue.MAX_QUEUE_SIZE) { + if (this.debug) { + console.warn( + `[TelemetryQueue] Queue is full (${TelemetryQueue.MAX_QUEUE_SIZE} items), dropping oldest event`, + ) + } + queue.shift() // Remove oldest event + } + + const queuedEvent: QueuedTelemetryEvent = { + id: randomUUID(), + event, + timestamp: Date.now(), + retryCount: 0, + } + + queue.push(queuedEvent) + await this.saveQueue(queue) + + if (this.debug) { + console.info( + `[TelemetryQueue] Enqueued event ${queuedEvent.id} (${event.type}), queue size: ${queue.length}`, + ) + } + } + + /** + * Retrieves the next event from the queue without removing it + */ + public async peek(): Promise { + const queue = await this.getQueue() + return queue.length > 0 ? queue[0] : null + } + + /** + * Removes a successfully sent event from the queue + */ + public async dequeue(eventId: string): Promise { + const queue = await this.getQueue() + const filteredQueue = queue.filter((e) => e.id !== eventId) + + if (queue.length !== filteredQueue.length) { + await this.saveQueue(filteredQueue) + if (this.debug) { + console.info(`[TelemetryQueue] Dequeued event ${eventId}, queue size: ${filteredQueue.length}`) + } + } + } + + /** + * Increments retry count for a failed event and moves it to the end of the queue + */ + public async markFailed(eventId: string): Promise { + const queue = await this.getQueue() + const eventIndex = queue.findIndex((e) => e.id === eventId) + + if (eventIndex === -1) { + return + } + + const event = queue[eventIndex] + event.retryCount++ + + // Remove from current position + queue.splice(eventIndex, 1) + + // If max retries not exceeded, add back to end of queue + if (event.retryCount < TelemetryQueue.MAX_RETRY_COUNT) { + queue.push(event) + if (this.debug) { + console.info( + `[TelemetryQueue] Marked event ${eventId} as failed (retry ${event.retryCount}/${TelemetryQueue.MAX_RETRY_COUNT})`, + ) + } + } else { + if (this.debug) { + console.warn(`[TelemetryQueue] Event ${eventId} exceeded max retries, removing from queue`) + } + } + + await this.saveQueue(queue) + } + + /** + * Gets the current queue size + */ + public async size(): Promise { + const queue = await this.getQueue() + return queue.length + } + + /** + * Checks if the queue is currently being processed + */ + public isProcessingQueue(): boolean { + return this.isProcessing + } + + /** + * Sets the processing state + */ + public setProcessingState(processing: boolean): void { + this.isProcessing = processing + } + + /** + * Clears all events from the queue + */ + public async clear(): Promise { + await this.saveQueue([]) + if (this.debug) { + console.info("[TelemetryQueue] Queue cleared") + } + } + + /** + * Gets all queued events (for testing/debugging) + */ + public async getAll(): Promise { + return await this.getQueue() + } + + private async getQueue(): Promise { + try { + const queue = this.context.globalState.get(TelemetryQueue.QUEUE_KEY) + // Validate that we got an array + if (Array.isArray(queue)) { + return queue + } + // If we got corrupted data, try to reset to empty array + if (queue !== undefined) { + console.warn("[TelemetryQueue] Corrupted queue data detected, resetting to empty array") + try { + await this.context.globalState.update(TelemetryQueue.QUEUE_KEY, []) + } catch (updateError) { + // If update fails, just log and continue with empty array + console.error("[TelemetryQueue] Failed to reset corrupted queue:", updateError) + } + } + return [] + } catch (error) { + console.error("[TelemetryQueue] Failed to get queue:", error) + return [] + } + } + + private async saveQueue(queue: QueuedTelemetryEvent[]): Promise { + try { + await this.context.globalState.update(TelemetryQueue.QUEUE_KEY, queue) + } catch (error) { + console.error("[TelemetryQueue] Failed to save queue:", error) + } + } +} diff --git a/packages/cloud/src/__tests__/TelemetryClient.queue.test.ts b/packages/cloud/src/__tests__/TelemetryClient.queue.test.ts new file mode 100644 index 0000000000..fd38f4c4cf --- /dev/null +++ b/packages/cloud/src/__tests__/TelemetryClient.queue.test.ts @@ -0,0 +1,504 @@ +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest" +import * as vscode from "vscode" +import { TelemetryEventName } from "@roo-code/types" + +import { TelemetryClient } from "../TelemetryClient" +import type { AuthService } from "../auth" +import type { SettingsService } from "../SettingsService" +import type { QueuedTelemetryEvent } from "../TelemetryQueue" + +// Mock vscode +vi.mock("vscode", () => ({ + ExtensionContext: vi.fn(), +})) + +// Mock fetch globally +const mockFetch = vi.fn() +global.fetch = mockFetch + +describe("TelemetryClient with Queue", () => { + let mockContext: vscode.ExtensionContext + let mockGlobalState: Map + let mockAuthService: AuthService + let mockSettingsService: SettingsService + let client: TelemetryClient + + beforeEach(() => { + // Reset mocks + vi.clearAllMocks() + mockGlobalState = new Map() + + mockContext = { + globalState: { + get: vi.fn((key: string) => mockGlobalState.get(key)), + update: vi.fn(async (key: string, value: unknown) => { + mockGlobalState.set(key, value) + }), + }, + } as unknown as vscode.ExtensionContext + + mockAuthService = { + isAuthenticated: vi.fn().mockReturnValue(true), + getSessionToken: vi.fn().mockReturnValue("test-token"), + getUserInfo: vi.fn().mockReturnValue(null), + hasActiveSession: vi.fn().mockReturnValue(true), + hasOrIsAcquiringActiveSession: vi.fn().mockReturnValue(true), + getStoredOrganizationId: vi.fn().mockReturnValue(null), + getState: vi.fn().mockReturnValue("authenticated"), + initialize: vi.fn(), + login: vi.fn(), + logout: vi.fn(), + handleCallback: vi.fn(), + on: vi.fn(), + off: vi.fn(), + } as unknown as AuthService + + mockSettingsService = { + getSettings: vi.fn().mockReturnValue({ + cloudSettings: { + recordTaskMessages: false, + }, + }), + getAllowList: vi.fn().mockReturnValue({}), + dispose: vi.fn(), + } as unknown as SettingsService + + // Reset fetch mock + mockFetch.mockReset() + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + statusText: "OK", + }) + + client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService, false) + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + describe("Queue Integration", () => { + it("should add events to queue instead of sending directly", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + const event = { + event: TelemetryEventName.TASK_CREATED, + properties: { customProp: "value" }, + } + + await client.capture(event) + + // Should not have called fetch immediately + expect(mockFetch).not.toHaveBeenCalled() + + // Event should be in the queue + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toBeDefined() + expect(queue).toHaveLength(1) + expect(queue[0].event.type).toBe(TelemetryEventName.TASK_CREATED) + }) + + it("should process queue when adding events", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + const event = { + event: TelemetryEventName.TASK_CREATED, + properties: { customProp: "value" }, + } + + await client.capture(event) + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Should have attempted to send the event + expect(mockFetch).toHaveBeenCalledTimes(1) + expect(mockFetch).toHaveBeenCalledWith( + expect.stringContaining("/api/events"), + expect.objectContaining({ + method: "POST", + headers: expect.objectContaining({ + Authorization: "Bearer test-token", + }), + }), + ) + + // Queue should be empty after successful send + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(0) + }) + + it("should keep events in queue on send failure", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + // Mock fetch to fail + mockFetch.mockRejectedValueOnce(new Error("Network error")) + + const event = { + event: TelemetryEventName.TASK_CREATED, + properties: { customProp: "value" }, + } + + await client.capture(event) + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Should have attempted to send + expect(mockFetch).toHaveBeenCalledTimes(1) + + // Event should still be in queue with incremented retry count + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(1) + expect(queue[0].retryCount).toBe(1) + }) + + it("should not process queue when not authenticated", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + mockAuthService.isAuthenticated = vi.fn().mockReturnValue(false) + + const event = { + event: TelemetryEventName.TASK_CREATED, + properties: { customProp: "value" }, + } + + await client.capture(event) + + // Wait for any async processing + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Should not have attempted to send + expect(mockFetch).not.toHaveBeenCalled() + + // Event should still be in queue + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(1) + }) + + it("should process multiple events in FIFO order", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + const event1 = { + event: TelemetryEventName.TASK_CREATED, + properties: { order: 1 }, + } + const event2 = { + event: TelemetryEventName.TASK_COMPLETED, + properties: { order: 2 }, + } + const event3 = { + event: TelemetryEventName.MODE_SWITCH, + properties: { order: 3 }, + } + + // Add events without waiting for processing + await client.capture(event1) + await client.capture(event2) + await client.capture(event3) + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Should have sent all events + expect(mockFetch).toHaveBeenCalledTimes(3) + + // Verify order by checking the body of each call + const calls = mockFetch.mock.calls + const bodies = calls.map((call) => JSON.parse(call[1].body)) + + expect(bodies[0].type).toBe(TelemetryEventName.TASK_CREATED) + expect(bodies[1].type).toBe(TelemetryEventName.TASK_COMPLETED) + expect(bodies[2].type).toBe(TelemetryEventName.MODE_SWITCH) + + // Queue should be empty + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(0) + }) + + it("should stop processing on first failure", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + // Mock fetch to fail on second call + mockFetch + .mockResolvedValueOnce({ ok: true, status: 200, statusText: "OK" }) + .mockRejectedValueOnce(new Error("Network error")) + + const event1 = { + event: TelemetryEventName.TASK_CREATED, + properties: { order: 1 }, + } + const event2 = { + event: TelemetryEventName.TASK_COMPLETED, + properties: { order: 2 }, + } + const event3 = { + event: TelemetryEventName.MODE_SWITCH, + properties: { order: 3 }, + } + + await client.capture(event1) + await client.capture(event2) + await client.capture(event3) + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Should have attempted to send first two events + expect(mockFetch).toHaveBeenCalledTimes(2) + + // Queue should have 2 events (failed one moved to end, third one untouched) + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(2) + expect(queue[0].event.type).toBe(TelemetryEventName.MODE_SWITCH) + expect(queue[1].event.type).toBe(TelemetryEventName.TASK_COMPLETED) + expect(queue[1].retryCount).toBe(1) + }) + + it("should handle HTTP error responses", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + // Mock fetch to return error response + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 500, + statusText: "Internal Server Error", + }) + + const event = { + event: TelemetryEventName.TASK_CREATED, + properties: { customProp: "value" }, + } + + await client.capture(event) + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Should have attempted to send + expect(mockFetch).toHaveBeenCalledTimes(1) + + // Event should still be in queue with incremented retry count + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(1) + expect(queue[0].retryCount).toBe(1) + }) + + it("should not process queue if already processing", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + // Create a slow response to keep processing active + let resolveFirstRequest: () => void + const firstRequestPromise = new Promise((resolve) => { + resolveFirstRequest = resolve + }) + + mockFetch.mockImplementationOnce(async () => { + await firstRequestPromise + return { ok: true, status: 200, statusText: "OK" } + }) + + // Add first event + await client.capture({ + event: TelemetryEventName.TASK_CREATED, + properties: { order: 1 }, + }) + + // Add second event while first is still processing + await client.capture({ + event: TelemetryEventName.TASK_COMPLETED, + properties: { order: 2 }, + }) + + // Should only have one fetch call (for the first event) + expect(mockFetch).toHaveBeenCalledTimes(1) + + // Complete the first request + resolveFirstRequest!() + + // Wait for processing to complete + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Now both events should have been processed + expect(mockFetch).toHaveBeenCalledTimes(2) + + // Queue should be empty + const queue = mockGlobalState.get("rooCode.telemetryQueue") as QueuedTelemetryEvent[] + expect(queue).toHaveLength(0) + }) + }) + + describe("Event Filtering", () => { + it("should not queue events that are not capturable", async () => { + const event = { + event: TelemetryEventName.TASK_CONVERSATION_MESSAGE, // In exclude list + properties: { test: "value" }, + } + + await client.capture(event) + + // Should not have called fetch + expect(mockFetch).not.toHaveBeenCalled() + + // Should not be in queue + const queue = mockGlobalState.get("rooCode.telemetryQueue") + expect(queue).toBeUndefined() + }) + + it("should not queue TASK_MESSAGE events when recordTaskMessages is false", async () => { + const event = { + event: TelemetryEventName.TASK_MESSAGE, + properties: { taskId: "test-task" }, + } + + await client.capture(event) + + // Should not have called fetch + expect(mockFetch).not.toHaveBeenCalled() + + // Should not be in queue + const queue = mockGlobalState.get("rooCode.telemetryQueue") + expect(queue).toBeUndefined() + }) + + it("should queue TASK_MESSAGE events when recordTaskMessages is true", async () => { + // Mock provider to provide required properties + const mockProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + language: "en", + mode: "code", + }), + } + client.setProvider(mockProvider) + + mockSettingsService.getSettings = vi.fn().mockReturnValue({ + cloudSettings: { + recordTaskMessages: true, + }, + }) + + const event = { + event: TelemetryEventName.TASK_MESSAGE, + properties: { + taskId: "test-task", + message: { + ts: Date.now(), + type: "say", + say: "text", + text: "test message", + }, + }, + } + + await client.capture(event) + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Should have attempted to send + expect(mockFetch).toHaveBeenCalledTimes(1) + }) + }) +}) diff --git a/packages/cloud/src/__tests__/TelemetryClient.test.ts b/packages/cloud/src/__tests__/TelemetryClient.test.ts index e4c62b1e4e..562c8cdb79 100644 --- a/packages/cloud/src/__tests__/TelemetryClient.test.ts +++ b/packages/cloud/src/__tests__/TelemetryClient.test.ts @@ -3,9 +3,15 @@ // npx vitest run src/__tests__/TelemetryClient.test.ts import { type TelemetryPropertiesProvider, TelemetryEventName } from "@roo-code/types" +import * as vscode from "vscode" import { TelemetryClient } from "../TelemetryClient" +// Mock vscode +vi.mock("vscode", () => ({ + ExtensionContext: vi.fn(), +})) + const mockFetch = vi.fn() global.fetch = mockFetch as any @@ -14,12 +20,27 @@ describe("TelemetryClient", () => { return instance[propertyName] } + let mockContext: vscode.ExtensionContext + let mockGlobalState: Map let mockAuthService: any let mockSettingsService: any + let mockQueue: any beforeEach(() => { vi.clearAllMocks() + // Reset mocks + mockGlobalState = new Map() + + mockContext = { + globalState: { + get: vi.fn((key: string) => mockGlobalState.get(key)), + update: vi.fn(async (key: string, value: any) => { + mockGlobalState.set(key, value) + }), + }, + } as unknown as vscode.ExtensionContext + // Create a mock AuthService instead of using the singleton mockAuthService = { getSessionToken: vi.fn().mockReturnValue("mock-token"), @@ -37,6 +58,19 @@ describe("TelemetryClient", () => { }), } + // Create a mock queue + mockQueue = { + enqueue: vi.fn().mockResolvedValue(undefined), + peek: vi.fn().mockResolvedValue(null), + dequeue: vi.fn().mockResolvedValue(undefined), + markFailed: vi.fn().mockResolvedValue(undefined), + size: vi.fn().mockResolvedValue(0), + isProcessingQueue: vi.fn().mockReturnValue(false), + setProcessingState: vi.fn(), + clear: vi.fn().mockResolvedValue(undefined), + getAll: vi.fn().mockResolvedValue([]), + } + mockFetch.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({}), @@ -52,7 +86,7 @@ describe("TelemetryClient", () => { describe("isEventCapturable", () => { it("should return true for events not in exclude list", () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -66,7 +100,7 @@ describe("TelemetryClient", () => { }) it("should return false for events in exclude list", () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -83,7 +117,7 @@ describe("TelemetryClient", () => { }, }) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -100,7 +134,7 @@ describe("TelemetryClient", () => { }, }) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -115,7 +149,7 @@ describe("TelemetryClient", () => { cloudSettings: {}, }) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -128,7 +162,7 @@ describe("TelemetryClient", () => { it("should return false for TASK_MESSAGE events when cloudSettings is undefined", () => { mockSettingsService.getSettings.mockReturnValue({}) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -141,7 +175,7 @@ describe("TelemetryClient", () => { it("should return false for TASK_MESSAGE events when getSettings returns undefined", () => { mockSettingsService.getSettings.mockReturnValue(undefined) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const isEventCapturable = getPrivateProperty<(eventName: TelemetryEventName) => boolean>( client, @@ -154,7 +188,7 @@ describe("TelemetryClient", () => { describe("getEventProperties", () => { it("should merge provider properties with event properties", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const mockProvider: TelemetryPropertiesProvider = { getTelemetryProperties: vi.fn().mockResolvedValue({ @@ -195,7 +229,7 @@ describe("TelemetryClient", () => { }) it("should handle errors from provider gracefully", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const mockProvider: TelemetryPropertiesProvider = { getTelemetryProperties: vi.fn().mockRejectedValue(new Error("Provider error")), @@ -221,7 +255,7 @@ describe("TelemetryClient", () => { }) it("should return event properties when no provider is set", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const getEventProperties = getPrivateProperty< (event: { event: TelemetryEventName; properties?: Record }) => Promise> @@ -238,7 +272,7 @@ describe("TelemetryClient", () => { describe("capture", () => { it("should not capture events that are not capturable", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) await client.capture({ event: TelemetryEventName.TASK_CONVERSATION_MESSAGE, // In exclude list. @@ -255,7 +289,7 @@ describe("TelemetryClient", () => { }, }) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) await client.capture({ event: TelemetryEventName.TASK_MESSAGE, @@ -278,7 +312,7 @@ describe("TelemetryClient", () => { cloudSettings: {}, }) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) await client.capture({ event: TelemetryEventName.TASK_MESSAGE, @@ -297,7 +331,7 @@ describe("TelemetryClient", () => { }) it("should not send request when schema validation fails", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) await client.capture({ event: TelemetryEventName.TASK_CREATED, @@ -308,8 +342,10 @@ describe("TelemetryClient", () => { expect(console.error).toHaveBeenCalledWith(expect.stringContaining("Invalid telemetry event")) }) - it("should send request when event is capturable and validation passes", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + it("should enqueue event when event is capturable and validation passes", async () => { + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) + // Replace the queue with our mock + ;(client as any).queue = mockQueue const providerProperties = { appName: "roo-code", @@ -325,14 +361,6 @@ describe("TelemetryClient", () => { taskId: "test-task-id", } - const mockValidatedData = { - type: TelemetryEventName.TASK_CREATED, - properties: { - ...providerProperties, - taskId: "test-task-id", - }, - } - const mockProvider: TelemetryPropertiesProvider = { getTelemetryProperties: vi.fn().mockResolvedValue(providerProperties), } @@ -344,11 +372,14 @@ describe("TelemetryClient", () => { properties: eventProperties, }) - expect(mockFetch).toHaveBeenCalledWith( - "https://app.roocode.com/api/events", + // Should enqueue the event + expect(mockQueue.enqueue).toHaveBeenCalledWith( expect.objectContaining({ - method: "POST", - body: JSON.stringify(mockValidatedData), + type: TelemetryEventName.TASK_CREATED, + properties: expect.objectContaining({ + ...providerProperties, + taskId: "test-task-id", + }), }), ) }) @@ -377,29 +408,26 @@ describe("TelemetryClient", () => { }, } - const mockValidatedData = { - type: TelemetryEventName.TASK_MESSAGE, - properties: eventProperties, - } - - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) + // Replace the queue with our mock + ;(client as any).queue = mockQueue await client.capture({ event: TelemetryEventName.TASK_MESSAGE, properties: eventProperties, }) - expect(mockFetch).toHaveBeenCalledWith( - "https://app.roocode.com/api/events", + // Should enqueue the event + expect(mockQueue.enqueue).toHaveBeenCalledWith( expect.objectContaining({ - method: "POST", - body: JSON.stringify(mockValidatedData), + type: TelemetryEventName.TASK_MESSAGE, + properties: eventProperties, }), ) }) it("should handle fetch errors gracefully", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) mockFetch.mockRejectedValue(new Error("Network error")) @@ -414,12 +442,12 @@ describe("TelemetryClient", () => { describe("telemetry state methods", () => { it("should always return true for isTelemetryEnabled", () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) expect(client.isTelemetryEnabled()).toBe(true) }) it("should have empty implementations for updateTelemetryState and shutdown", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) client.updateTelemetryState(true) await client.shutdown() }) @@ -428,7 +456,7 @@ describe("TelemetryClient", () => { describe("backfillMessages", () => { it("should not send request when not authenticated", async () => { mockAuthService.isAuthenticated.mockReturnValue(false) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const messages = [ { @@ -446,7 +474,7 @@ describe("TelemetryClient", () => { it("should not send request when no session token available", async () => { mockAuthService.getSessionToken.mockReturnValue(null) - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const messages = [ { @@ -466,7 +494,7 @@ describe("TelemetryClient", () => { }) it("should send FormData request with correct structure when authenticated", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const providerProperties = { appName: "roo-code", @@ -537,7 +565,7 @@ describe("TelemetryClient", () => { }) it("should handle provider errors gracefully", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const mockProvider: TelemetryPropertiesProvider = { getTelemetryProperties: vi.fn().mockRejectedValue(new Error("Provider error")), @@ -589,7 +617,7 @@ describe("TelemetryClient", () => { }) it("should work without provider set", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) const messages = [ { @@ -635,7 +663,7 @@ describe("TelemetryClient", () => { }) it("should handle fetch errors gracefully", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) mockFetch.mockRejectedValue(new Error("Network error")) @@ -658,7 +686,7 @@ describe("TelemetryClient", () => { }) it("should handle HTTP error responses", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) mockFetch.mockResolvedValue({ ok: false, @@ -683,7 +711,7 @@ describe("TelemetryClient", () => { }) it("should log debug information when debug is enabled", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService, true) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService, true) const messages = [ { @@ -705,7 +733,7 @@ describe("TelemetryClient", () => { }) it("should handle empty messages array", async () => { - const client = new TelemetryClient(mockAuthService, mockSettingsService) + const client = new TelemetryClient(mockContext, mockAuthService, mockSettingsService) await client.backfillMessages([], "test-task-id") diff --git a/packages/cloud/src/__tests__/TelemetryQueue.test.ts b/packages/cloud/src/__tests__/TelemetryQueue.test.ts new file mode 100644 index 0000000000..d941568b82 --- /dev/null +++ b/packages/cloud/src/__tests__/TelemetryQueue.test.ts @@ -0,0 +1,341 @@ +import { describe, it, expect, beforeEach, vi } from "vitest" +import * as vscode from "vscode" +import { TelemetryQueue } from "../TelemetryQueue" +import { TelemetryEventName, type RooCodeTelemetryEvent } from "@roo-code/types" + +// Mock vscode +vi.mock("vscode", () => ({ + ExtensionContext: vi.fn(), +})) + +describe("TelemetryQueue", () => { + let mockContext: vscode.ExtensionContext + let mockGlobalState: Map + let queue: TelemetryQueue + + const createMockEvent = (type: TelemetryEventName = TelemetryEventName.TASK_CREATED): RooCodeTelemetryEvent => { + const baseProperties = { + appName: "test-app", + appVersion: "1.0.0", + vscodeVersion: "1.0.0", + platform: "test-platform", + editorName: "test-editor", + language: "en", + mode: "test", + } + + // Handle special event types that require additional properties + if (type === TelemetryEventName.TASK_MESSAGE) { + return { + type: TelemetryEventName.TASK_MESSAGE, + properties: { + ...baseProperties, + taskId: "test-task-id", + message: { + ts: Date.now(), + type: "say", + say: "text", + text: "test message", + }, + }, + } + } else if (type === TelemetryEventName.LLM_COMPLETION) { + return { + type: TelemetryEventName.LLM_COMPLETION, + properties: { + ...baseProperties, + inputTokens: 100, + outputTokens: 200, + }, + } + } + + // For all other event types + return { + type: type as TelemetryEventName, // Type assertion needed due to discriminated union + properties: baseProperties, + } as RooCodeTelemetryEvent + } + + beforeEach(() => { + // Reset mocks + mockGlobalState = new Map() + + mockContext = { + globalState: { + get: vi.fn((key: string) => mockGlobalState.get(key)), + update: vi.fn(async (key: string, value: unknown) => { + mockGlobalState.set(key, value) + }), + }, + } as unknown as vscode.ExtensionContext + + queue = new TelemetryQueue(mockContext, false) + }) + + describe("enqueue", () => { + it("should add an event to the queue", async () => { + const event = createMockEvent() + + await queue.enqueue(event) + + const size = await queue.size() + expect(size).toBe(1) + + const peeked = await queue.peek() + expect(peeked).toBeDefined() + expect(peeked?.event).toEqual(event) + expect(peeked?.retryCount).toBe(0) + expect(peeked?.timestamp).toBeGreaterThan(0) + expect(peeked?.id).toBeDefined() + }) + + it("should maintain FIFO order", async () => { + const event1 = createMockEvent(TelemetryEventName.TASK_CREATED) + const event2 = createMockEvent(TelemetryEventName.TASK_COMPLETED) + const event3 = createMockEvent(TelemetryEventName.MODE_SWITCH) + + await queue.enqueue(event1) + await queue.enqueue(event2) + await queue.enqueue(event3) + + const all = await queue.getAll() + expect(all).toHaveLength(3) + expect(all[0].event.type).toBe(TelemetryEventName.TASK_CREATED) + expect(all[1].event.type).toBe(TelemetryEventName.TASK_COMPLETED) + expect(all[2].event.type).toBe(TelemetryEventName.MODE_SWITCH) + }) + + it("should drop oldest event when queue is full", async () => { + // Set up a smaller queue for testing + const smallQueue = new TelemetryQueue(mockContext, false) + // Override the max size for testing + const originalMaxSize = (TelemetryQueue as unknown as { MAX_QUEUE_SIZE: number }).MAX_QUEUE_SIZE + ;(TelemetryQueue as unknown as { MAX_QUEUE_SIZE: number }).MAX_QUEUE_SIZE = 3 + + try { + // Fill the queue + await smallQueue.enqueue(createMockEvent(TelemetryEventName.TASK_CREATED)) + await smallQueue.enqueue(createMockEvent(TelemetryEventName.TASK_COMPLETED)) + await smallQueue.enqueue(createMockEvent(TelemetryEventName.MODE_SWITCH)) + + // Add one more - should drop the first + await smallQueue.enqueue(createMockEvent(TelemetryEventName.TOOL_USED)) + + const all = await smallQueue.getAll() + expect(all).toHaveLength(3) + expect(all[0].event.type).toBe(TelemetryEventName.TASK_COMPLETED) + expect(all[1].event.type).toBe(TelemetryEventName.MODE_SWITCH) + expect(all[2].event.type).toBe(TelemetryEventName.TOOL_USED) + } finally { + // Restore original max size + ;(TelemetryQueue as unknown as { MAX_QUEUE_SIZE: number }).MAX_QUEUE_SIZE = originalMaxSize + } + }) + }) + + describe("dequeue", () => { + it("should remove a specific event from the queue", async () => { + const event1 = createMockEvent(TelemetryEventName.TASK_CREATED) + const event2 = createMockEvent(TelemetryEventName.TASK_COMPLETED) + + await queue.enqueue(event1) + await queue.enqueue(event2) + + const peeked = await queue.peek() + expect(peeked).toBeDefined() + + await queue.dequeue(peeked!.id) + + const size = await queue.size() + expect(size).toBe(1) + + const newPeeked = await queue.peek() + expect(newPeeked?.event.type).toBe(TelemetryEventName.TASK_COMPLETED) + }) + + it("should handle dequeuing non-existent event gracefully", async () => { + await queue.enqueue(createMockEvent()) + + await queue.dequeue("non-existent-id") + + const size = await queue.size() + expect(size).toBe(1) + }) + }) + + describe("markFailed", () => { + it("should increment retry count and move event to end of queue", async () => { + const event1 = createMockEvent(TelemetryEventName.TASK_CREATED) + const event2 = createMockEvent(TelemetryEventName.TASK_COMPLETED) + + await queue.enqueue(event1) + await queue.enqueue(event2) + + const firstEvent = await queue.peek() + expect(firstEvent).toBeDefined() + + await queue.markFailed(firstEvent!.id) + + const all = await queue.getAll() + expect(all).toHaveLength(2) + expect(all[0].event.type).toBe(TelemetryEventName.TASK_COMPLETED) + expect(all[1].event.type).toBe(TelemetryEventName.TASK_CREATED) + expect(all[1].retryCount).toBe(1) + }) + + it("should remove event after max retries", async () => { + const event = createMockEvent() + await queue.enqueue(event) + + const peeked = await queue.peek() + expect(peeked).toBeDefined() + + // Override max retry count for testing + const originalMaxRetry = (TelemetryQueue as unknown as { MAX_RETRY_COUNT: number }).MAX_RETRY_COUNT + ;(TelemetryQueue as unknown as { MAX_RETRY_COUNT: number }).MAX_RETRY_COUNT = 2 + + try { + // Fail twice - should still be in queue + await queue.markFailed(peeked!.id) + expect(await queue.size()).toBe(1) + + const peeked2 = await queue.peek() + await queue.markFailed(peeked2!.id) + + // Third failure should remove it + expect(await queue.size()).toBe(0) + } finally { + // Restore original max retry + ;(TelemetryQueue as unknown as { MAX_RETRY_COUNT: number }).MAX_RETRY_COUNT = originalMaxRetry + } + }) + + it("should handle marking non-existent event as failed gracefully", async () => { + await queue.enqueue(createMockEvent()) + + await queue.markFailed("non-existent-id") + + const size = await queue.size() + expect(size).toBe(1) + }) + }) + + describe("peek", () => { + it("should return null for empty queue", async () => { + const peeked = await queue.peek() + expect(peeked).toBeNull() + }) + + it("should return first event without removing it", async () => { + const event = createMockEvent() + await queue.enqueue(event) + + const peeked1 = await queue.peek() + const peeked2 = await queue.peek() + + expect(peeked1).toEqual(peeked2) + expect(await queue.size()).toBe(1) + }) + }) + + describe("clear", () => { + it("should remove all events from the queue", async () => { + await queue.enqueue(createMockEvent()) + await queue.enqueue(createMockEvent()) + await queue.enqueue(createMockEvent()) + + expect(await queue.size()).toBe(3) + + await queue.clear() + + expect(await queue.size()).toBe(0) + expect(await queue.peek()).toBeNull() + }) + }) + + describe("processing state", () => { + it("should track processing state correctly", () => { + expect(queue.isProcessingQueue()).toBe(false) + + queue.setProcessingState(true) + expect(queue.isProcessingQueue()).toBe(true) + + queue.setProcessingState(false) + expect(queue.isProcessingQueue()).toBe(false) + }) + }) + + describe("persistence", () => { + it("should persist queue to global state", async () => { + const event = createMockEvent() + await queue.enqueue(event) + + // Create a new queue instance with same context + const newQueue = new TelemetryQueue(mockContext, false) + + const size = await newQueue.size() + expect(size).toBe(1) + + const peeked = await newQueue.peek() + expect(peeked?.event).toEqual(event) + }) + + it("should handle corrupted state gracefully", async () => { + // Corrupt the state with a non-array value + mockGlobalState.set("rooCode.telemetryQueue", "invalid-json") + + // Track if update was called to reset the corrupted state + let updateCalled = false + + // Update the mock to return the corrupted value initially, then the updated value + mockContext.globalState.get = vi.fn((key: string) => { + if (key === "rooCode.telemetryQueue") { + // After update is called, return the actual value from mockGlobalState + if (updateCalled) { + return mockGlobalState.get(key) + } + return "invalid-json" // Return non-array value initially + } + return mockGlobalState.get(key) + }) + + // Track when update is called + const originalUpdate = mockContext.globalState.update + mockContext.globalState.update = vi.fn(async (key: string, value: unknown) => { + updateCalled = true + await originalUpdate(key, value) + }) + + // Create a new queue instance that will try to read the corrupted state + const corruptedQueue = new TelemetryQueue(mockContext, false) + + const size = await corruptedQueue.size() + expect(size).toBe(0) + + // Should still be able to add events + await corruptedQueue.enqueue(createMockEvent()) + expect(await corruptedQueue.size()).toBe(1) + }) + }) + + describe("error handling", () => { + it("should handle globalState.get errors gracefully", async () => { + mockContext.globalState.get = vi.fn(() => { + throw new Error("Storage error") + }) + + const size = await queue.size() + expect(size).toBe(0) + }) + + it("should handle globalState.update errors gracefully", async () => { + mockContext.globalState.update = vi.fn(() => { + throw new Error("Storage error") + }) + + // Should not throw + await expect(queue.enqueue(createMockEvent())).resolves.not.toThrow() + }) + }) +})