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", () => {