feat(telemetry): add app version to exception captures and filter 402 errors (#9996)

Co-authored-by: cte <[email protected]>

packages/cloud/src/TelemetryClient.ts

@@ -69,9 +69,7 @@ abstract class BaseTelemetryClient implements TelemetryClient {
 
 	public abstract capture(event: TelemetryEvent): Promise<void>
 
-	public captureException(_error: Error, _additionalProperties?: Record<string, unknown>): void {
-		// No-op - exception capture is only supported by PostHog
-	}
+	public async captureException(_error: Error, _additionalProperties?: Record<string, unknown>): Promise<void> {}
 
 	public setProvider(provider: TelemetryPropertiesProvider): void {
 		this.providerRef = new WeakRef(provider)

packages/telemetry/src/BaseTelemetryClient.ts

@@ -59,7 +59,7 @@ export abstract class BaseTelemetryClient implements TelemetryClient {
 
 	public abstract capture(event: TelemetryEvent): Promise<void>
 
-	public abstract captureException(error: Error, additionalProperties?: Record<string, unknown>): void
+	public abstract captureException(error: Error, additionalProperties?: Record<string, unknown>): Promise<void>
 
 	public setProvider(provider: TelemetryPropertiesProvider): void {
 		this.providerRef = new WeakRef(provider)

packages/telemetry/src/PostHogTelemetryClient.ts

@@ -2,8 +2,9 @@ import { PostHog } from "posthog-node"
 import * as vscode from "vscode"
 
 import {
-	TelemetryEventName,
+	type TelemetryProperties,
 	type TelemetryEvent,
+	TelemetryEventName,
 	getErrorStatusCode,
 	getErrorMessage,
 	shouldReportApiErrorToTelemetry,
@@ -69,7 +70,10 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 		})
 	}
 
-	public override captureException(error: Error, additionalProperties?: Record<string, unknown>): void {
+	public override async captureException(
+		error: Error,
+		additionalProperties?: Record<string, unknown>,
+	): Promise<void> {
 		if (!this.isTelemetryEnabled()) {
 			if (this.debug) {
 				console.info(`[PostHogTelemetryClient#captureException] Skipping exception: ${error.message}`)
@@ -82,7 +86,7 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 		const errorCode = getErrorStatusCode(error)
 		const errorMessage = getErrorMessage(error) ?? error.message
 
-		// Filter out expected errors (e.g., 429 rate limits)
+		// Filter out expected errors (e.g., 402 billing, 429 rate limits)
 		if (!shouldReportApiErrorToTelemetry(errorCode, errorMessage)) {
 			if (this.debug) {
 				console.info(
@@ -108,7 +112,21 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 		// Override the error message with the extracted error message.
 		error.message = errorMessage
 
-		this.client.captureException(error, this.distinctId, mergedProperties)
+		const provider = this.providerRef?.deref()
+		let telemetryProperties: TelemetryProperties | undefined = undefined
+
+		if (provider) {
+			try {
+				telemetryProperties = await provider.getTelemetryProperties()
+			} catch (_error) {
+				// Ignore.
+			}
+		}
+
+		this.client.captureException(error, this.distinctId, {
+			...mergedProperties,
+			$app_version: telemetryProperties?.appVersion,
+		})
 	}
 
 	/**

packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts

@@ -367,37 +367,69 @@ describe("PostHogTelemetryClient", () => {
 		})
 	})
 
-	describe("shutdown", () => {
-		it("should call shutdown on the PostHog client", async () => {
-			const client = new PostHogTelemetryClient()
-			await client.shutdown()
-			expect(mockPostHogClient.shutdown).toHaveBeenCalled()
-		})
-	})
-
 	describe("captureException", () => {
-		it("should not capture exceptions when telemetry is disabled", () => {
+		it("should not capture exceptions when telemetry is disabled", async () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(false)
 
 			const error = new Error("Test error")
-			client.captureException(error)
+			await client.captureException(error)
 
 			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
 		})
 
-		it("should capture exceptions when telemetry is enabled", () => {
+		it("should capture exceptions with app version from provider", async () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const mockProvider: TelemetryPropertiesProvider = {
+				getTelemetryProperties: vi.fn().mockResolvedValue({
+					appVersion: "1.0.0",
+					vscodeVersion: "1.60.0",
+					platform: "darwin",
+					editorName: "vscode",
+					language: "en",
+					mode: "code",
+				}),
+			}
+
+			client.setProvider(mockProvider)
+
+			const error = new Error("Test error")
+			await client.captureException(error, { customProp: "value" })
+
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(
+				error,
+				"test-machine-id",
+				expect.objectContaining({
+					customProp: "value",
+					$app_version: "1.0.0",
+				}),
+			)
+		})
+
+		it("should capture exceptions with undefined app version when no provider is set", async () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
 
 			const error = new Error("Test error")
-			client.captureException(error, { provider: "TestProvider" })
+			await client.captureException(error)
 
 			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
-				provider: "TestProvider",
+				$app_version: undefined,
 			})
 		})
+	})
+
+	describe("shutdown", () => {
+		it("should call shutdown on the PostHog client", async () => {
+			const client = new PostHogTelemetryClient()
+			await client.shutdown()
+			expect(mockPostHogClient.shutdown).toHaveBeenCalled()
+		})
+	})
 
+	describe("captureException error filtering", () => {
 		it("should filter out 429 rate limit errors (via status property)", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
@@ -410,6 +442,18 @@ describe("PostHogTelemetryClient", () => {
 			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
 		})
 
+		it("should filter out 402 billing errors (via status property)", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			// Create an error with status 402 (Payment Required)
+			const error = Object.assign(new Error("Payment required"), { status: 402 })
+			client.captureException(error)
+
+			// Should NOT capture 402 errors
+			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
+		})
+
 		it("should filter out errors with '429' in message", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
@@ -439,7 +483,9 @@ describe("PostHogTelemetryClient", () => {
 			const error = new Error("Internal server error")
 			client.captureException(error)
 
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				$app_version: undefined,
+			})
 		})
 
 		it("should capture errors with non-429 status codes", () => {
@@ -449,7 +495,9 @@ describe("PostHogTelemetryClient", () => {
 			const error = Object.assign(new Error("Internal server error"), { status: 500 })
 			client.captureException(error)
 
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				$app_version: undefined,
+			})
 		})
 
 		it("should use nested error message from OpenAI SDK error structure for filtering", () => {
@@ -470,7 +518,7 @@ describe("PostHogTelemetryClient", () => {
 			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
 		})
 
-		it("should modify error.message with extracted message from nested metadata.raw", () => {
+		it("should capture errors with nested metadata and override error.message with extracted message", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
 
@@ -485,12 +533,14 @@ describe("PostHogTelemetryClient", () => {
 
 			client.captureException(error)
 
-			// Verify error message was modified to use metadata.raw (highest priority)
+			// The implementation overrides error.message with the extracted message from getErrorMessage
 			expect(error.message).toBe("Upstream provider error: model overloaded")
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				$app_version: undefined,
+			})
 		})
 
-		it("should modify error.message with nested error.message when metadata.raw is not available", () => {
+		it("should capture errors with nested error.message and override error.message with extracted message", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
 
@@ -504,9 +554,11 @@ describe("PostHogTelemetryClient", () => {
 
 			client.captureException(error)
 
-			// Verify error message was modified to use nested error.message
+			// The implementation overrides error.message with the extracted message from getErrorMessage
 			expect(error.message).toBe("Upstream provider: connection timeout")
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				$app_version: undefined,
+			})
 		})
 
 		it("should use primary message when no nested error structure exists", () => {
@@ -522,78 +574,55 @@ describe("PostHogTelemetryClient", () => {
 
 			// Verify error message remains the primary message
 			expect(error.message).toBe("Primary error message")
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				$app_version: undefined,
+			})
 		})
 
-		it("should auto-extract properties from ApiProviderError", () => {
+		it("should capture ApiProviderError and auto-extract properties", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
 
 			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
 			client.captureException(error)
 
+			// The implementation auto-extracts properties from ApiProviderError
 			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
 				provider: "OpenRouter",
 				modelId: "gpt-4",
 				operation: "createMessage",
 				errorCode: 500,
+				$app_version: undefined,
 			})
 		})
 
-		it("should auto-extract properties from ApiProviderError without errorCode", () => {
-			const client = new PostHogTelemetryClient()
-			client.updateTelemetryState(true)
-
-			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "completePrompt")
-			client.captureException(error)
-
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
-				provider: "OpenRouter",
-				modelId: "gpt-4",
-				operation: "completePrompt",
-			})
-		})
-
-		it("should merge auto-extracted properties with additionalProperties", () => {
+		it("should capture ApiProviderError with additionalProperties merged with auto-extracted properties", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
 
 			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
 			client.captureException(error, { customProperty: "value" })
 
+			// additionalProperties take precedence over auto-extracted properties
 			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
 				provider: "OpenRouter",
 				modelId: "gpt-4",
 				operation: "createMessage",
 				customProperty: "value",
+				$app_version: undefined,
 			})
 		})
 
-		it("should allow additionalProperties to override auto-extracted properties", () => {
-			const client = new PostHogTelemetryClient()
-			client.updateTelemetryState(true)
-
-			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
-			// Explicitly override the provider value
-			client.captureException(error, { provider: "OverriddenProvider" })
-
-			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
-				provider: "OverriddenProvider", // additionalProperties takes precedence
-				modelId: "gpt-4",
-				operation: "createMessage",
-			})
-		})
-
-		it("should not auto-extract for non-ApiProviderError errors", () => {
+		it("should capture regular errors with additionalProperties", () => {
 			const client = new PostHogTelemetryClient()
 			client.updateTelemetryState(true)
 
 			const error = new Error("Regular error")
 			client.captureException(error, { customProperty: "value" })
 
-			// Should only have the additionalProperties, not any auto-extracted ones
 			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
 				customProperty: "value",
+				$app_version: undefined,
 			})
 		})
 	})

packages/types/src/__tests__/telemetry.test.ts

@@ -109,6 +109,11 @@ describe("telemetry error utilities", () => {
 			}
 		})
 
+		it("should return false for 402 billing errors", () => {
+			expect(shouldReportApiErrorToTelemetry(402)).toBe(false)
+			expect(shouldReportApiErrorToTelemetry(402, "Payment required")).toBe(false)
+		})
+
 		it("should return false for 429 rate limit errors", () => {
 			expect(shouldReportApiErrorToTelemetry(429)).toBe(false)
 			expect(shouldReportApiErrorToTelemetry(429, "Rate limit exceeded")).toBe(false)
@@ -142,6 +147,16 @@ describe("telemetry error utilities", () => {
 		})
 	})
 
+	describe("EXPECTED_API_ERROR_CODES", () => {
+		it("should contain 402 (payment required)", () => {
+			expect(EXPECTED_API_ERROR_CODES.has(402)).toBe(true)
+		})
+
+		it("should contain 429 (rate limit)", () => {
+			expect(EXPECTED_API_ERROR_CODES.has(429)).toBe(true)
+		})
+	})
+
 	describe("ApiProviderError", () => {
 		it("should create an error with correct properties", () => {
 			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)

packages/types/src/telemetry.ts

@@ -262,7 +262,7 @@ export interface TelemetryClient {
 
 	setProvider(provider: TelemetryPropertiesProvider): void
 	capture(options: TelemetryEvent): Promise<void>
-	captureException(error: Error, additionalProperties?: Record<string, unknown>): void
+	captureException(error: Error, additionalProperties?: Record<string, unknown>): Promise<void>
 	updateTelemetryState(isOptedIn: boolean): void
 	isTelemetryEnabled(): boolean
 	shutdown(): Promise<void>
@@ -273,6 +273,7 @@ export interface TelemetryClient {
  * These are normal/expected errors that users can't do much about.
  */
 export const EXPECTED_API_ERROR_CODES = new Set([
+	402, // Payment required - billing issues
 	429, // Rate limit - expected when hitting API limits
 ])