mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
parent
dbf206f06b
commit
49ad8aee2c
2 changed files with 92 additions and 0 deletions
|
|
@ -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<TelemetryPropertiesProvider> | 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
|
||||
|
|
|
|||
|
|
@ -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<any>(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<string, any> }) => Promise<Record<string, any>>
|
||||
>(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<any>(client, "cachedStaticProperties")
|
||||
expect(cachedProps).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe("capture", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue