Skip to content

Commit e894bee

Browse files
ctecte
committed
More progress
1 parent 8365490 commit e894bee

File tree

5 files changed

+244
-34
lines changed

5 files changed

+244
-34
lines changed

‎packages/telemetry/src/PostHogTelemetryClient.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import {
77
getErrorStatusCode,
88
getOpenAISdkErrorMessage,
99
shouldReportApiErrorToTelemetry,
10+
isApiProviderError,
11+
extractApiProviderErrorProperties,
1012
} from "@roo-code/types"
1113

1214
import { BaseTelemetryClient } from "./BaseTelemetryClient"
@@ -94,7 +96,16 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
9496
console.info(`[PostHogTelemetryClient#captureException] ${error.message}`)
9597
}
9698

97-
this.client.captureException(error, this.distinctId, additionalProperties)
99+
// Auto-extract properties from ApiProviderError and merge with additionalProperties.
100+
// Explicit additionalProperties take precedence over auto-extracted properties.
101+
let mergedProperties = additionalProperties
102+
103+
if (isApiProviderError(error)) {
104+
const extractedProperties = extractApiProviderErrorProperties(error)
105+
mergedProperties = { ...extractedProperties, ...additionalProperties }
106+
}
107+
108+
this.client.captureException(error, this.distinctId, mergedProperties)
98109
}
99110

100111
/**

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

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import * as vscode from "vscode"
66
import { PostHog } from "posthog-node"
77

8-
import { type TelemetryPropertiesProvider, TelemetryEventName } from "@roo-code/types"
8+
import { type TelemetryPropertiesProvider, TelemetryEventName, ApiProviderError } from "@roo-code/types"
99

1010
import { PostHogTelemetryClient } from "../PostHogTelemetryClient"
1111

@@ -469,5 +469,77 @@ describe("PostHogTelemetryClient", () => {
469469
// Should NOT capture - the nested metadata.raw contains rate limit message
470470
expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
471471
})
472+
473+
it("should auto-extract properties from ApiProviderError", () => {
474+
const client = new PostHogTelemetryClient()
475+
client.updateTelemetryState(true)
476+
477+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
478+
client.captureException(error)
479+
480+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
481+
provider: "OpenRouter",
482+
modelId: "gpt-4",
483+
operation: "createMessage",
484+
errorCode: 500,
485+
})
486+
})
487+
488+
it("should auto-extract properties from ApiProviderError without errorCode", () => {
489+
const client = new PostHogTelemetryClient()
490+
client.updateTelemetryState(true)
491+
492+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "completePrompt")
493+
client.captureException(error)
494+
495+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
496+
provider: "OpenRouter",
497+
modelId: "gpt-4",
498+
operation: "completePrompt",
499+
})
500+
})
501+
502+
it("should merge auto-extracted properties with additionalProperties", () => {
503+
const client = new PostHogTelemetryClient()
504+
client.updateTelemetryState(true)
505+
506+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
507+
client.captureException(error, { customProperty: "value" })
508+
509+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
510+
provider: "OpenRouter",
511+
modelId: "gpt-4",
512+
operation: "createMessage",
513+
customProperty: "value",
514+
})
515+
})
516+
517+
it("should allow additionalProperties to override auto-extracted properties", () => {
518+
const client = new PostHogTelemetryClient()
519+
client.updateTelemetryState(true)
520+
521+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
522+
// Explicitly override the provider value
523+
client.captureException(error, { provider: "OverriddenProvider" })
524+
525+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
526+
provider: "OverriddenProvider", // additionalProperties takes precedence
527+
modelId: "gpt-4",
528+
operation: "createMessage",
529+
})
530+
})
531+
532+
it("should not auto-extract for non-ApiProviderError errors", () => {
533+
const client = new PostHogTelemetryClient()
534+
client.updateTelemetryState(true)
535+
536+
const error = new Error("Regular error")
537+
client.captureException(error, { customProperty: "value" })
538+
539+
// Should only have the additionalProperties, not any auto-extracted ones
540+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
541+
customProperty: "value",
542+
})
543+
})
472544
})
473545
})

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

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { describe, it, expect } from "vitest"
2-
31
import {
42
getErrorStatusCode,
53
getOpenAISdkErrorMessage,
64
shouldReportApiErrorToTelemetry,
75
EXPECTED_API_ERROR_CODES,
6+
ApiProviderError,
7+
isApiProviderError,
8+
extractApiProviderErrorProperties,
89
} from "../telemetry.js"
910

1011
describe("telemetry error utilities", () => {
@@ -138,4 +139,105 @@ describe("telemetry error utilities", () => {
138139
expect(shouldReportApiErrorToTelemetry(undefined, "Connection timeout")).toBe(true)
139140
})
140141
})
142+
143+
describe("ApiProviderError", () => {
144+
it("should create an error with correct properties", () => {
145+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
146+
147+
expect(error.message).toBe("Test error")
148+
expect(error.name).toBe("ApiProviderError")
149+
expect(error.provider).toBe("OpenRouter")
150+
expect(error.modelId).toBe("gpt-4")
151+
expect(error.operation).toBe("createMessage")
152+
expect(error.errorCode).toBe(500)
153+
})
154+
155+
it("should work without optional errorCode", () => {
156+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
157+
158+
expect(error.message).toBe("Test error")
159+
expect(error.provider).toBe("OpenRouter")
160+
expect(error.modelId).toBe("gpt-4")
161+
expect(error.operation).toBe("createMessage")
162+
expect(error.errorCode).toBeUndefined()
163+
})
164+
165+
it("should be an instance of Error", () => {
166+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
167+
expect(error).toBeInstanceOf(Error)
168+
})
169+
})
170+
171+
describe("isApiProviderError", () => {
172+
it("should return true for ApiProviderError instances", () => {
173+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
174+
expect(isApiProviderError(error)).toBe(true)
175+
})
176+
177+
it("should return true for ApiProviderError with errorCode", () => {
178+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 429)
179+
expect(isApiProviderError(error)).toBe(true)
180+
})
181+
182+
it("should return false for regular Error instances", () => {
183+
const error = new Error("Test error")
184+
expect(isApiProviderError(error)).toBe(false)
185+
})
186+
187+
it("should return false for null and undefined", () => {
188+
expect(isApiProviderError(null)).toBe(false)
189+
expect(isApiProviderError(undefined)).toBe(false)
190+
})
191+
192+
it("should return false for non-error objects", () => {
193+
expect(isApiProviderError({})).toBe(false)
194+
expect(isApiProviderError({ provider: "test", modelId: "test", operation: "test" })).toBe(false)
195+
})
196+
197+
it("should return false for Error with wrong name", () => {
198+
const error = new Error("Test error")
199+
error.name = "CustomError"
200+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
201+
;(error as any).provider = "OpenRouter"
202+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
203+
;(error as any).modelId = "gpt-4"
204+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
205+
;(error as any).operation = "createMessage"
206+
expect(isApiProviderError(error)).toBe(false)
207+
})
208+
})
209+
210+
describe("extractApiProviderErrorProperties", () => {
211+
it("should extract all properties from ApiProviderError", () => {
212+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
213+
const properties = extractApiProviderErrorProperties(error)
214+
215+
expect(properties).toEqual({
216+
provider: "OpenRouter",
217+
modelId: "gpt-4",
218+
operation: "createMessage",
219+
errorCode: 500,
220+
})
221+
})
222+
223+
it("should not include errorCode when undefined", () => {
224+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
225+
const properties = extractApiProviderErrorProperties(error)
226+
227+
expect(properties).toEqual({
228+
provider: "OpenRouter",
229+
modelId: "gpt-4",
230+
operation: "createMessage",
231+
})
232+
expect(properties).not.toHaveProperty("errorCode")
233+
})
234+
235+
it("should include errorCode when it is 0", () => {
236+
const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 0)
237+
const properties = extractApiProviderErrorProperties(error)
238+
239+
// errorCode of 0 is falsy but !== undefined, so it should be included
240+
expect(properties).toHaveProperty("errorCode", 0)
241+
})
242+
})
141243
})

‎packages/types/src/telemetry.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,3 +390,30 @@ export class ApiProviderError extends Error {
390390
this.name = "ApiProviderError"
391391
}
392392
}
393+
394+
/**
395+
* Type guard to check if an error is an ApiProviderError.
396+
* Used by telemetry to automatically extract structured properties.
397+
*/
398+
export function isApiProviderError(error: unknown): error is ApiProviderError {
399+
return (
400+
error instanceof Error &&
401+
error.name === "ApiProviderError" &&
402+
"provider" in error &&
403+
"modelId" in error &&
404+
"operation" in error
405+
)
406+
}
407+
408+
/**
409+
* Extracts properties from an ApiProviderError for telemetry.
410+
* Returns the structured properties that can be merged with additionalProperties.
411+
*/
412+
export function extractApiProviderErrorProperties(error: ApiProviderError): Record<string, unknown> {
413+
return {
414+
provider: error.provider,
415+
modelId: error.modelId,
416+
operation: error.operation,
417+
...(error.errorCode !== undefined && { errorCode: error.errorCode }),
418+
}
419+
}

‎src/api/providers/openrouter.ts‎

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,8 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
233233
modelId,
234234
"createMessage",
235235
),
236-
{ provider: this.providerName, modelId, operation: "createMessage", originalError: error },
237236
)
237+
238238
throw handleOpenAIError(error, this.providerName)
239239
}
240240

@@ -259,22 +259,20 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
259259
if ("error" in chunk) {
260260
const error = chunk.error as { message?: string; code?: number }
261261
console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`)
262-
const errorWithStatus = Object.assign(
263-
new ApiProviderError(
264-
error?.message ?? "Unknown error",
265-
this.providerName,
266-
modelId,
267-
"createMessage",
268-
error?.code,
262+
263+
TelemetryService.instance.captureException(
264+
Object.assign(
265+
new ApiProviderError(
266+
error?.message ?? "Unknown error",
267+
this.providerName,
268+
modelId,
269+
"createMessage",
270+
error?.code,
271+
),
272+
{ status: error?.code },
269273
),
270-
{ status: error?.code },
271274
)
272-
TelemetryService.instance.captureException(errorWithStatus, {
273-
provider: this.providerName,
274-
modelId,
275-
operation: "createMessage",
276-
errorCode: error?.code,
277-
})
275+
278276
throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`)
279277
}
280278

@@ -466,6 +464,7 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
466464
: undefined
467465

468466
let response
467+
469468
try {
470469
response = await this.client.chat.completions.create(completionParams, requestOptions)
471470
} catch (error) {
@@ -476,29 +475,28 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
476475
modelId,
477476
"completePrompt",
478477
),
479-
{ provider: this.providerName, modelId, operation: "completePrompt", originalError: error },
480478
)
479+
481480
throw handleOpenAIError(error, this.providerName)
482481
}
483482

484483
if ("error" in response) {
485484
const error = response.error as { message?: string; code?: number }
486-
const errorWithStatus = Object.assign(
487-
new ApiProviderError(
488-
error?.message ?? "Unknown error",
489-
this.providerName,
490-
modelId,
491-
"completePrompt",
492-
error?.code,
485+
console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`)
486+
487+
TelemetryService.instance.captureException(
488+
Object.assign(
489+
new ApiProviderError(
490+
error?.message ?? "Unknown error",
491+
this.providerName,
492+
modelId,
493+
"completePrompt",
494+
error?.code,
495+
),
496+
{ status: error?.code },
493497
),
494-
{ status: error?.code },
495498
)
496-
TelemetryService.instance.captureException(errorWithStatus, {
497-
provider: this.providerName,
498-
modelId,
499-
operation: "completePrompt",
500-
errorCode: error?.code,
501-
})
499+
502500
throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`)
503501
}
504502

0 commit comments

Comments
 (0)