From 49ad8aee2cdf8caf327006beb6851dcd6d472f06 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Thu, 15 Jan 2026 02:06:04 +0000 Subject: [PATCH] fix: cache static telemetry properties to prevent validation errors when provider is disposed When ClineProvider is disposed (e.g., webview closed), the WeakRef in TelemetryClient returns undefined, causing required telemetry properties (appName, appVersion, vscodeVersion, platform, editorName) to be undefined. This fix: - Caches static app properties when setProvider() is called - Uses cached properties as fallback when provider is unavailable - Ensures telemetry events always have required fields Fixes #10737 --- packages/cloud/src/TelemetryClient.ts | 21 ++++++ .../src/__tests__/TelemetryClient.test.ts | 71 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/packages/cloud/src/TelemetryClient.ts b/packages/cloud/src/TelemetryClient.ts index f8e8640323a..e0eb9ca24fd 100644 --- a/packages/cloud/src/TelemetryClient.ts +++ b/packages/cloud/src/TelemetryClient.ts @@ -8,6 +8,7 @@ import { rooCodeTelemetryEventSchema, TelemetryPropertiesProvider, TelemetryEventSubscription, + type StaticAppProperties, } from "@roo-code/types" import { getRooCodeApiUrl } from "./config.js" @@ -16,6 +17,12 @@ import type { RetryQueue } from "./retry-queue/index.js" abstract class BaseTelemetryClient implements TelemetryClient { protected providerRef: WeakRef | null = null protected telemetryEnabled: boolean = false + /** + * Cached static app properties captured when the provider is set. + * These are used as fallback when the provider is no longer available + * (e.g., after ClineProvider is disposed). + */ + protected cachedStaticProperties: StaticAppProperties | null = null constructor( public readonly subscription?: TelemetryEventSubscription, @@ -54,6 +61,10 @@ abstract class BaseTelemetryClient implements TelemetryClient { `Error getting telemetry properties: ${error instanceof Error ? error.message : String(error)}`, ) } + } else if (this.cachedStaticProperties) { + // Provider is no longer available (e.g., ClineProvider was disposed). + // Use cached static properties to ensure required telemetry fields are present. + providerProperties = { ...this.cachedStaticProperties } } // Merge provider properties with event-specific properties. @@ -73,6 +84,16 @@ abstract class BaseTelemetryClient implements TelemetryClient { public setProvider(provider: TelemetryPropertiesProvider): void { this.providerRef = new WeakRef(provider) + + // Capture static app properties immediately so they remain available + // even after the provider is garbage collected. + // This is especially important for ClineProvider which may be disposed + // when the webview is closed, but telemetry events may still be in flight. + if ("appProperties" in provider) { + this.cachedStaticProperties = ( + provider as TelemetryPropertiesProvider & { appProperties: StaticAppProperties } + ).appProperties + } } public abstract updateTelemetryState(didUserOptIn: boolean): void diff --git a/packages/cloud/src/__tests__/TelemetryClient.test.ts b/packages/cloud/src/__tests__/TelemetryClient.test.ts index 5cb952dc280..3255020e890 100644 --- a/packages/cloud/src/__tests__/TelemetryClient.test.ts +++ b/packages/cloud/src/__tests__/TelemetryClient.test.ts @@ -195,6 +195,77 @@ describe("TelemetryClient", () => { expect(result).toEqual({ customProp: "value" }) }) + + it("should use cached static properties when provider is no longer available", async () => { + const client = new TelemetryClient(mockAuthService, mockSettingsService) + + const staticProperties = { + appName: "roo-code", + appVersion: "1.0.0", + vscodeVersion: "1.60.0", + platform: "darwin", + editorName: "vscode", + } + + // Create a mock provider with appProperties + const mockProvider: TelemetryPropertiesProvider & { appProperties: any } = { + appProperties: staticProperties, + getTelemetryProperties: vi.fn().mockResolvedValue({ + ...staticProperties, + language: "en", + mode: "code", + }), + } + + // Set the provider (this should cache the static properties) + client.setProvider(mockProvider) + + // Verify cached properties were set + const cachedProps = getPrivateProperty(client, "cachedStaticProperties") + expect(cachedProps).toEqual(staticProperties) + + // Now simulate the provider being garbage collected by setting providerRef to return undefined + // We do this by clearing the providerRef directly + ;(client as any).providerRef = new WeakRef({} as any) // WeakRef to an empty object that will be different + // Force the WeakRef to return undefined by overriding deref + ;(client as any).providerRef = { deref: () => undefined } + + const getEventProperties = getPrivateProperty< + (event: { event: TelemetryEventName; properties?: Record }) => Promise> + >(client, "getEventProperties").bind(client) + + const result = await getEventProperties({ + event: TelemetryEventName.TASK_CREATED, + properties: { customProp: "value" }, + }) + + // Should include cached static properties when provider is unavailable + expect(result).toEqual({ + ...staticProperties, + customProp: "value", + }) + }) + + it("should not cache properties when provider does not have appProperties", async () => { + const client = new TelemetryClient(mockAuthService, mockSettingsService) + + // Create a mock provider WITHOUT appProperties + const mockProvider: TelemetryPropertiesProvider = { + getTelemetryProperties: vi.fn().mockResolvedValue({ + appName: "roo-code", + appVersion: "1.0.0", + language: "en", + mode: "code", + }), + } + + // Set the provider + client.setProvider(mockProvider) + + // Verify cached properties were NOT set (provider doesn't have appProperties) + const cachedProps = getPrivateProperty(client, "cachedStaticProperties") + expect(cachedProps).toBeNull() + }) }) describe("capture", () => {