fix: address PR review comments for telemetry queue implementation

- Fix memory leak by properly cleaning up connectionRestoredDebounceTimer in CloudService dispose()
- Implement AbortController pattern in TelemetryClient for robust timeout handling
- Make timeout value configurable using class constant in ConnectionMonitor
- Add security bounds checking with ABSOLUTE_MAX_QUEUE_SIZE to prevent memory exhaustion
- Add fallback for crypto.randomUUID() for environments where it's not available
- Improve error handling in processBatchedEvents() to handle individual event failures
- Add proper error logging for dynamic import failures in CloudService
- Fix typo in Catalan translation (s'encularan -> s'encolaran)

packages/cloud/src/CloudService.ts

@@ -39,6 +39,7 @@ export class CloudService extends EventEmitter<CloudServiceEvents> implements vs
 	private shareService: ShareService | null = null
 	private connectionMonitor: ConnectionMonitor | null = null
 	private queueManager: TelemetryQueueManager | null = null
+	private connectionRestoredDebounceTimer: NodeJS.Timeout | null = null
 	private isInitialized = false
 	private log: (...args: unknown[]) => void
 
@@ -103,26 +104,26 @@ export class CloudService extends EventEmitter<CloudServiceEvents> implements vs
 			try {
 				const { ContextProxy } = await import("../../../src/core/config/ContextProxy")
 				isQueueEnabled = ContextProxy.instance.getValue("telemetryQueueEnabled") ?? true
-			} catch (_error) {
+			} catch (error) {
 				// Default to enabled if we can't access settings
-				this.log("[CloudService] Could not access telemetryQueueEnabled setting, defaulting to enabled")
+				this.log("[CloudService] Could not access telemetryQueueEnabled setting:", error)
+				isQueueEnabled = true
 			}
 
 			if (isQueueEnabled) {
 				// Set up connection monitoring with debouncing
-				let connectionRestoredDebounceTimer: NodeJS.Timeout | null = null
 				const connectionRestoredDebounceDelay = 3000 // 3 seconds
 
 				this.connectionMonitor.onConnectionRestored(() => {
 					this.log("[CloudService] Connection restored, scheduling queue processing")
 
 					// Clear any existing timer
-					if (connectionRestoredDebounceTimer) {
-						clearTimeout(connectionRestoredDebounceTimer)
+					if (this.connectionRestoredDebounceTimer) {
+						clearTimeout(this.connectionRestoredDebounceTimer)
 					}
 
 					// Schedule queue processing with debounce
-					connectionRestoredDebounceTimer = setTimeout(() => {
+					this.connectionRestoredDebounceTimer = setTimeout(() => {
 						this.queueManager
 							?.processQueue()
 							.then(() => {
@@ -321,6 +322,11 @@ export class CloudService extends EventEmitter<CloudServiceEvents> implements vs
 		if (this.connectionMonitor) {
 			this.connectionMonitor.dispose()
 		}
+		// Clean up any pending debounce timer
+		if (this.connectionRestoredDebounceTimer) {
+			clearTimeout(this.connectionRestoredDebounceTimer)
+			this.connectionRestoredDebounceTimer = null
+		}
 
 		this.isInitialized = false
 	}

packages/cloud/src/ConnectionMonitor.ts

@@ -6,6 +6,7 @@ export class ConnectionMonitor extends EventEmitter {
 	private checkInterval: NodeJS.Timeout | null = null
 	private readonly healthCheckEndpoint = "/api/health"
 	private readonly defaultCheckInterval = 30000 // 30 seconds
+	private readonly defaultTimeoutMs = 5000 // 5 seconds
 
 	constructor() {
 		super()
@@ -17,7 +18,7 @@ export class ConnectionMonitor extends EventEmitter {
 	public async checkConnection(): Promise<boolean> {
 		try {
 			const controller = new AbortController()
-			const timeoutId = setTimeout(() => controller.abort(), 5000) // 5 second timeout
+			const timeoutId = setTimeout(() => controller.abort(), this.defaultTimeoutMs)
 
 			const response = await fetch(`${getRooCodeApiUrl()}${this.healthCheckEndpoint}`, {
 				method: "GET",

packages/cloud/src/TelemetryClient.ts

@@ -18,6 +18,7 @@ export class TelemetryClient extends BaseTelemetryClient {
 	private isQueueEnabled: boolean = false
 	private log: (...args: unknown[]) => void
 	private processQueueDebounceTimer: NodeJS.Timeout | null = null
+	private processQueueAbortController: AbortController | null = null
 	private readonly processQueueDebounceDelay = 5000 // 5 seconds
 
 	constructor(
@@ -123,8 +124,16 @@ export class TelemetryClient extends BaseTelemetryClient {
 		if (this.processQueueDebounceTimer) {
 			clearTimeout(this.processQueueDebounceTimer)
 		}
+		if (this.processQueueAbortController) {
+			this.processQueueAbortController.abort()
+		}
+		this.processQueueAbortController = new AbortController()
+		const signal = this.processQueueAbortController.signal
 
 		this.processQueueDebounceTimer = setTimeout(() => {
+			if (signal.aborted) {
+				return
+			}
 			this.queueManager.processQueue().catch((error) => {
 				this.log(`[TelemetryClient#debouncedProcessQueue] Error processing queue: ${error}`)
 			})
@@ -218,6 +227,11 @@ export class TelemetryClient extends BaseTelemetryClient {
 			clearTimeout(this.processQueueDebounceTimer)
 			this.processQueueDebounceTimer = null
 		}
+		// Abort any pending operations
+		if (this.processQueueAbortController) {
+			this.processQueueAbortController.abort()
+			this.processQueueAbortController = null
+		}
 
 		// Process any remaining queued events before shutdown if queue is enabled
 		if (this.isQueueEnabled) {
@@ -244,18 +258,25 @@ export class TelemetryClient extends BaseTelemetryClient {
 
 		// Process each event individually to maintain compatibility
 		for (const queuedEvent of events) {
-			const payload = {
-				type: queuedEvent.event.event,
-				properties: await this.getEventProperties(queuedEvent.event),
-			}
+			try {
+				const payload = {
+					type: queuedEvent.event.event,
+					properties: await this.getEventProperties(queuedEvent.event),
+				}
 
-			const result = rooCodeTelemetryEventSchema.safeParse(payload)
-			if (!result.success) {
-				this.log(`[TelemetryClient#processBatchedEvents] Invalid telemetry event: ${result.error.message}`)
-				continue
-			}
+				const result = rooCodeTelemetryEventSchema.safeParse(payload)
+				if (!result.success) {
+					this.log(`[TelemetryClient#processBatchedEvents] Invalid telemetry event: ${result.error.message}`)
+					continue
+				}
 
-			await this.fetch(`events`, { method: "POST", body: JSON.stringify(result.data) })
+				await this.fetch(`events`, { method: "POST", body: JSON.stringify(result.data) })
+			} catch (error) {
+				// Log the error but continue processing other events
+				this.log(`[TelemetryClient#processBatchedEvents] Error processing event ${queuedEvent.id}: ${error}`)
+				// Re-throw to let the queue manager handle retry logic
+				throw error
+			}
 		}
 	}
 }

packages/cloud/src/TelemetryQueueManager.ts

@@ -4,6 +4,7 @@ import { ContextProxy } from "../../../src/core/config/ContextProxy"
 
 export class TelemetryQueueManager {
 	private static instance: TelemetryQueueManager
+	private static readonly ABSOLUTE_MAX_QUEUE_SIZE = 5000
 	private queue: QueuedTelemetryEvent[] = []
 	private isProcessing = false
 	private maxQueueSize = 1000
@@ -37,7 +38,10 @@ export class TelemetryQueueManager {
 	 */
 	public async addToQueue(event: TelemetryEvent, priority: "high" | "normal" = "normal"): Promise<void> {
 		const queuedEvent: QueuedTelemetryEvent = {
-			id: crypto.randomUUID(),
+			id:
+				typeof crypto !== "undefined" && crypto.randomUUID
+					? crypto.randomUUID()
+					: `fallback-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,
 			event,
 			timestamp: Date.now(),
 			retryCount: 0,
@@ -185,8 +189,11 @@ export class TelemetryQueueManager {
 
 			if (storedQueue && Array.isArray(storedQueue)) {
 				// Add validation for queue size to prevent memory issues
-				if (storedQueue.length > this.maxQueueSize * 2) {
-					this.log("[TelemetryQueueManager] Queue size exceeds safety limit, truncating to max size")
+				const effectiveMaxSize = Math.min(this.maxQueueSize * 2, TelemetryQueueManager.ABSOLUTE_MAX_QUEUE_SIZE)
+				if (storedQueue.length > effectiveMaxSize) {
+					this.log(
+						`[TelemetryQueueManager] Queue size (${storedQueue.length}) exceeds safety limit (${effectiveMaxSize}), truncating to max size`,
+					)
 					this.queue = (storedQueue as QueuedTelemetryEvent[]).slice(-this.maxQueueSize)
 				} else {
 					this.queue = storedQueue as QueuedTelemetryEvent[]

webview-ui/src/i18n/locales/ca/account.json

@@ -11,5 +11,5 @@
 	"cloudBenefitSharing": "Funcions de compartició i col·laboració",
 	"cloudBenefitMetrics": "Mètriques d'ús basades en tasques, tokens i costos",
 	"visitCloudWebsite": "Visita Roo Code Cloud",
-	"offlineWarning": "Ara mateix estàs sense connexió. Els esdeveniments de telemetria s'encularan i s'enviaran quan es restableixi la connexió."
+	"offlineWarning": "Ara mateix estàs sense connexió. Els esdeveniments de telemetria s'encolaran i s'enviaran quan es restableixi la connexió."
 }