Skip to content

Commit 4934ef6

Browse files
committed
More progress
1 parent e894bee commit 4934ef6

File tree

5 files changed

+176
-98
lines changed

5 files changed

+176
-98
lines changed

packages/telemetry/src/PostHogTelemetryClient.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
TelemetryEventName,
66
type TelemetryEvent,
77
getErrorStatusCode,
8-
getOpenAISdkErrorMessage,
8+
getErrorMessage,
99
shouldReportApiErrorToTelemetry,
1010
isApiProviderError,
1111
extractApiProviderErrorProperties,
@@ -78,9 +78,9 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
7878
return
7979
}
8080

81-
// Extract error status code and message for filtering
81+
// Extract error status code and message for filtering.
8282
const errorCode = getErrorStatusCode(error)
83-
const errorMessage = getOpenAISdkErrorMessage(error) ?? error.message
83+
const errorMessage = getErrorMessage(error) ?? error.message
8484

8585
// Filter out expected errors (e.g., 429 rate limits)
8686
if (!shouldReportApiErrorToTelemetry(errorCode, errorMessage)) {
@@ -105,6 +105,9 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
105105
mergedProperties = { ...extractedProperties, ...additionalProperties }
106106
}
107107

108+
// Override the error message with the extracted error message.
109+
error.message = errorMessage
110+
108111
this.client.captureException(error, this.distinctId, mergedProperties)
109112
}
110113

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

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
22

3-
// npx vitest run src/__tests__/PostHogTelemetryClient.test.ts
3+
// pnpm --filter @roo-code/telemetry test src/__tests__/PostHogTelemetryClient.test.ts
44

55
import * as vscode from "vscode"
66
import { PostHog } from "posthog-node"
@@ -452,7 +452,7 @@ describe("PostHogTelemetryClient", () => {
452452
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
453453
})
454454

455-
it("should use nested error message from OpenAI SDK error structure", () => {
455+
it("should use nested error message from OpenAI SDK error structure for filtering", () => {
456456
const client = new PostHogTelemetryClient()
457457
client.updateTelemetryState(true)
458458

@@ -470,6 +470,61 @@ describe("PostHogTelemetryClient", () => {
470470
expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
471471
})
472472

473+
it("should modify error.message with extracted message from nested metadata.raw", () => {
474+
const client = new PostHogTelemetryClient()
475+
client.updateTelemetryState(true)
476+
477+
// Create an OpenAI SDK-like error with nested metadata (non-rate-limit error)
478+
const error = Object.assign(new Error("Generic request failed"), {
479+
status: 500,
480+
error: {
481+
message: "Nested error message",
482+
metadata: { raw: "Upstream provider error: model overloaded" },
483+
},
484+
})
485+
486+
client.captureException(error)
487+
488+
// Verify error message was modified to use metadata.raw (highest priority)
489+
expect(error.message).toBe("Upstream provider error: model overloaded")
490+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
491+
})
492+
493+
it("should modify error.message with nested error.message when metadata.raw is not available", () => {
494+
const client = new PostHogTelemetryClient()
495+
client.updateTelemetryState(true)
496+
497+
// Create an OpenAI SDK-like error with nested message but no metadata.raw
498+
const error = Object.assign(new Error("Generic request failed"), {
499+
status: 500,
500+
error: {
501+
message: "Upstream provider: connection timeout",
502+
},
503+
})
504+
505+
client.captureException(error)
506+
507+
// Verify error message was modified to use nested error.message
508+
expect(error.message).toBe("Upstream provider: connection timeout")
509+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
510+
})
511+
512+
it("should use primary message when no nested error structure exists", () => {
513+
const client = new PostHogTelemetryClient()
514+
client.updateTelemetryState(true)
515+
516+
// Create an OpenAI SDK-like error without nested error object
517+
const error = Object.assign(new Error("Primary error message"), {
518+
status: 500,
519+
})
520+
521+
client.captureException(error)
522+
523+
// Verify error message remains the primary message
524+
expect(error.message).toBe("Primary error message")
525+
expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
526+
})
527+
473528
it("should auto-extract properties from ApiProviderError", () => {
474529
const client = new PostHogTelemetryClient()
475530
client.updateTelemetryState(true)

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
// pnpm --filter @roo-code/types test src/__tests__/telemetry.test.ts
2+
13
import {
24
getErrorStatusCode,
3-
getOpenAISdkErrorMessage,
5+
getErrorMessage,
46
shouldReportApiErrorToTelemetry,
57
EXPECTED_API_ERROR_CODES,
68
ApiProviderError,
@@ -45,16 +47,16 @@ describe("telemetry error utilities", () => {
4547
})
4648
})
4749

48-
describe("getOpenAISdkErrorMessage", () => {
50+
describe("getErrorMessage", () => {
4951
it("should return undefined for non-OpenAI SDK errors", () => {
50-
expect(getOpenAISdkErrorMessage(null)).toBeUndefined()
51-
expect(getOpenAISdkErrorMessage(undefined)).toBeUndefined()
52-
expect(getOpenAISdkErrorMessage({ message: "error" })).toBeUndefined()
52+
expect(getErrorMessage(null)).toBeUndefined()
53+
expect(getErrorMessage(undefined)).toBeUndefined()
54+
expect(getErrorMessage({ message: "error" })).toBeUndefined()
5355
})
5456

5557
it("should return the primary message for simple OpenAI SDK errors", () => {
5658
const error = { status: 400, message: "Bad request" }
57-
expect(getOpenAISdkErrorMessage(error)).toBe("Bad request")
59+
expect(getErrorMessage(error)).toBe("Bad request")
5860
})
5961

6062
it("should prioritize nested error.message over primary message", () => {
@@ -63,7 +65,7 @@ describe("telemetry error utilities", () => {
6365
message: "Request failed",
6466
error: { message: "Upstream provider error" },
6567
}
66-
expect(getOpenAISdkErrorMessage(error)).toBe("Upstream provider error")
68+
expect(getErrorMessage(error)).toBe("Upstream provider error")
6769
})
6870

6971
it("should prioritize metadata.raw over other messages", () => {
@@ -75,7 +77,7 @@ describe("telemetry error utilities", () => {
7577
metadata: { raw: "Rate limit exceeded: free-models-per-day" },
7678
},
7779
}
78-
expect(getOpenAISdkErrorMessage(error)).toBe("Rate limit exceeded: free-models-per-day")
80+
expect(getErrorMessage(error)).toBe("Rate limit exceeded: free-models-per-day")
7981
})
8082

8183
it("should fallback to nested error.message when metadata.raw is undefined", () => {
@@ -87,7 +89,7 @@ describe("telemetry error utilities", () => {
8789
metadata: {},
8890
},
8991
}
90-
expect(getOpenAISdkErrorMessage(error)).toBe("Detailed error message")
92+
expect(getErrorMessage(error)).toBe("Detailed error message")
9193
})
9294

9395
it("should fallback to primary message when no nested messages exist", () => {
@@ -96,7 +98,7 @@ describe("telemetry error utilities", () => {
9698
message: "Forbidden",
9799
error: {},
98100
}
99-
expect(getOpenAISdkErrorMessage(error)).toBe("Forbidden")
101+
expect(getErrorMessage(error)).toBe("Forbidden")
100102
})
101103
})
102104

packages/types/src/telemetry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ export function getErrorStatusCode(error: unknown): number | undefined {
341341
* @param error - The error to extract message from
342342
* @returns The best available error message, or undefined if not an OpenAI SDK error
343343
*/
344-
export function getOpenAISdkErrorMessage(error: unknown): string | undefined {
344+
export function getErrorMessage(error: unknown): string | undefined {
345345
if (isOpenAISdkError(error)) {
346346
// Prioritize nested metadata which may contain upstream provider details
347347
return error.error?.metadata?.raw || error.error?.message || error.message

0 commit comments

Comments
 (0)