Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/cloud/src/TelemetryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/telemetry/src/BaseTelemetryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 22 additions & 4 deletions packages/telemetry/src/PostHogTelemetryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { PostHog } from "posthog-node"
import * as vscode from "vscode"

import {
TelemetryEventName,
type TelemetryProperties,
type TelemetryEvent,
TelemetryEventName,
getErrorStatusCode,
getErrorMessage,
shouldReportApiErrorToTelemetry,
Expand Down Expand Up @@ -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}`)
Expand All @@ -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(
Expand All @@ -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,
})
}

/**
Expand Down
139 changes: 84 additions & 55 deletions packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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", () => {
Expand All @@ -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,
})
})
})
Expand Down
15 changes: 15 additions & 0 deletions packages/types/src/__tests__/telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion packages/types/src/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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
])

Expand Down
Loading