fix: filter out 429 rate limit errors from API error telemetry

packages/telemetry/src/PostHogTelemetryClient.ts

@@ -1,7 +1,13 @@
 import { PostHog } from "posthog-node"
 import * as vscode from "vscode"
 
-import { TelemetryEventName, type TelemetryEvent } from "@roo-code/types"
+import {
+	TelemetryEventName,
+	type TelemetryEvent,
+	getErrorStatusCode,
+	getOpenAISdkErrorMessage,
+	shouldReportApiErrorToTelemetry,
+} from "@roo-code/types"
 
 import { BaseTelemetryClient } from "./BaseTelemetryClient"
 
@@ -70,6 +76,20 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 			return
 		}
 
+		// Extract error status code and message for filtering
+		const errorCode = getErrorStatusCode(error)
+		const errorMessage = getOpenAISdkErrorMessage(error) ?? error.message
+
+		// Filter out expected errors (e.g., 429 rate limits)
+		if (!shouldReportApiErrorToTelemetry(errorCode, errorMessage)) {
+			if (this.debug) {
+				console.info(
+					`[PostHogTelemetryClient#captureException] Filtering out expected error: ${errorCode} - ${errorMessage}`,
+				)
+			}
+			return
+		}
+
 		if (this.debug) {
 			console.info(`[PostHogTelemetryClient#captureException] ${error.message}`)
 		}

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

@@ -32,6 +32,7 @@ describe("PostHogTelemetryClient", () => {
 
 		mockPostHogClient = {
 			capture: vi.fn(),
+			captureException: vi.fn(),
 			optIn: vi.fn(),
 			optOut: vi.fn(),
 			shutdown: vi.fn().mockResolvedValue(undefined),
@@ -373,4 +374,100 @@ describe("PostHogTelemetryClient", () => {
 			expect(mockPostHogClient.shutdown).toHaveBeenCalled()
 		})
 	})
+
+	describe("captureException", () => {
+		it("should not capture exceptions when telemetry is disabled", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(false)
+
+			const error = new Error("Test error")
+			client.captureException(error)
+
+			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
+		})
+
+		it("should capture exceptions when telemetry is enabled", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = new Error("Test error")
+			client.captureException(error, { provider: "TestProvider" })
+
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				provider: "TestProvider",
+			})
+		})
+
+		it("should filter out 429 rate limit errors (via status property)", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			// Create an error with status property (like OpenAI SDK errors)
+			const error = Object.assign(new Error("Rate limit exceeded"), { status: 429 })
+			client.captureException(error)
+
+			// Should NOT capture 429 errors
+			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
+		})
+
+		it("should filter out errors with '429' in message", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = new Error("429 Rate limit exceeded: free-models-per-day")
+			client.captureException(error)
+
+			// Should NOT capture errors with 429 in message
+			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
+		})
+
+		it("should filter out errors containing 'rate limit' (case insensitive)", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = new Error("Request failed due to Rate Limit")
+			client.captureException(error)
+
+			// Should NOT capture rate limit errors
+			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
+		})
+
+		it("should capture non-rate-limit errors", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = new Error("Internal server error")
+			client.captureException(error)
+
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+		})
+
+		it("should capture errors with non-429 status codes", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = Object.assign(new Error("Internal server error"), { status: 500 })
+			client.captureException(error)
+
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+		})
+
+		it("should use nested error message from OpenAI SDK error structure", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			// Create an error with nested metadata (like OpenRouter upstream errors)
+			const error = Object.assign(new Error("Request failed"), {
+				status: 429,
+				error: {
+					message: "Error details",
+					metadata: { raw: "Rate limit exceeded: free-models-per-day" },
+				},
+			})
+			client.captureException(error)
+
+			// Should NOT capture - the nested metadata.raw contains rate limit message
+			expect(mockPostHogClient.captureException).not.toHaveBeenCalled()
+		})
+	})
 })

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

@@ -0,0 +1,141 @@
+import { describe, it, expect } from "vitest"
+
+import {
+	getErrorStatusCode,
+	getOpenAISdkErrorMessage,
+	shouldReportApiErrorToTelemetry,
+	EXPECTED_API_ERROR_CODES,
+} from "../telemetry.js"
+
+describe("telemetry error utilities", () => {
+	describe("getErrorStatusCode", () => {
+		it("should return undefined for non-object errors", () => {
+			expect(getErrorStatusCode(null)).toBeUndefined()
+			expect(getErrorStatusCode(undefined)).toBeUndefined()
+			expect(getErrorStatusCode("error string")).toBeUndefined()
+			expect(getErrorStatusCode(42)).toBeUndefined()
+		})
+
+		it("should return undefined for objects without status property", () => {
+			expect(getErrorStatusCode({})).toBeUndefined()
+			expect(getErrorStatusCode({ message: "error" })).toBeUndefined()
+			expect(getErrorStatusCode({ code: 500 })).toBeUndefined()
+		})
+
+		it("should return undefined for objects with non-numeric status", () => {
+			expect(getErrorStatusCode({ status: "500" })).toBeUndefined()
+			expect(getErrorStatusCode({ status: null })).toBeUndefined()
+			expect(getErrorStatusCode({ status: undefined })).toBeUndefined()
+		})
+
+		it("should return status for OpenAI SDK-like errors", () => {
+			const error = { status: 429, message: "Rate limit exceeded" }
+			expect(getErrorStatusCode(error)).toBe(429)
+		})
+
+		it("should return status for errors with additional properties", () => {
+			const error = {
+				status: 500,
+				code: "internal_error",
+				message: "Internal server error",
+				error: { message: "Upstream error" },
+			}
+			expect(getErrorStatusCode(error)).toBe(500)
+		})
+	})
+
+	describe("getOpenAISdkErrorMessage", () => {
+		it("should return undefined for non-OpenAI SDK errors", () => {
+			expect(getOpenAISdkErrorMessage(null)).toBeUndefined()
+			expect(getOpenAISdkErrorMessage(undefined)).toBeUndefined()
+			expect(getOpenAISdkErrorMessage({ message: "error" })).toBeUndefined()
+		})
+
+		it("should return the primary message for simple OpenAI SDK errors", () => {
+			const error = { status: 400, message: "Bad request" }
+			expect(getOpenAISdkErrorMessage(error)).toBe("Bad request")
+		})
+
+		it("should prioritize nested error.message over primary message", () => {
+			const error = {
+				status: 500,
+				message: "Request failed",
+				error: { message: "Upstream provider error" },
+			}
+			expect(getOpenAISdkErrorMessage(error)).toBe("Upstream provider error")
+		})
+
+		it("should prioritize metadata.raw over other messages", () => {
+			const error = {
+				status: 429,
+				message: "Request failed",
+				error: {
+					message: "Error details",
+					metadata: { raw: "Rate limit exceeded: free-models-per-day" },
+				},
+			}
+			expect(getOpenAISdkErrorMessage(error)).toBe("Rate limit exceeded: free-models-per-day")
+		})
+
+		it("should fallback to nested error.message when metadata.raw is undefined", () => {
+			const error = {
+				status: 400,
+				message: "Request failed",
+				error: {
+					message: "Detailed error message",
+					metadata: {},
+				},
+			}
+			expect(getOpenAISdkErrorMessage(error)).toBe("Detailed error message")
+		})
+
+		it("should fallback to primary message when no nested messages exist", () => {
+			const error = {
+				status: 403,
+				message: "Forbidden",
+				error: {},
+			}
+			expect(getOpenAISdkErrorMessage(error)).toBe("Forbidden")
+		})
+	})
+
+	describe("shouldReportApiErrorToTelemetry", () => {
+		it("should return false for expected error codes", () => {
+			for (const code of EXPECTED_API_ERROR_CODES) {
+				expect(shouldReportApiErrorToTelemetry(code)).toBe(false)
+			}
+		})
+
+		it("should return false for 429 rate limit errors", () => {
+			expect(shouldReportApiErrorToTelemetry(429)).toBe(false)
+			expect(shouldReportApiErrorToTelemetry(429, "Rate limit exceeded")).toBe(false)
+		})
+
+		it("should return false for messages starting with 429", () => {
+			expect(shouldReportApiErrorToTelemetry(undefined, "429 Rate limit exceeded")).toBe(false)
+			expect(shouldReportApiErrorToTelemetry(undefined, "429: Too many requests")).toBe(false)
+		})
+
+		it("should return false for messages containing 'rate limit' (case insensitive)", () => {
+			expect(shouldReportApiErrorToTelemetry(undefined, "Rate limit exceeded")).toBe(false)
+			expect(shouldReportApiErrorToTelemetry(undefined, "RATE LIMIT error")).toBe(false)
+			expect(shouldReportApiErrorToTelemetry(undefined, "Request failed due to rate limit")).toBe(false)
+		})
+
+		it("should return true for non-rate-limit errors", () => {
+			expect(shouldReportApiErrorToTelemetry(500)).toBe(true)
+			expect(shouldReportApiErrorToTelemetry(400, "Bad request")).toBe(true)
+			expect(shouldReportApiErrorToTelemetry(401, "Unauthorized")).toBe(true)
+		})
+
+		it("should return true when no error code or message is provided", () => {
+			expect(shouldReportApiErrorToTelemetry()).toBe(true)
+			expect(shouldReportApiErrorToTelemetry(undefined, undefined)).toBe(true)
+		})
+
+		it("should return true for regular error messages without rate limit keywords", () => {
+			expect(shouldReportApiErrorToTelemetry(undefined, "Internal server error")).toBe(true)
+			expect(shouldReportApiErrorToTelemetry(undefined, "Connection timeout")).toBe(true)
+		})
+	})
+})

packages/types/src/telemetry.ts

@@ -276,15 +276,102 @@ export const EXPECTED_API_ERROR_CODES = new Set([
 	429, // Rate limit - expected when hitting API limits
 ])
 
+/**
+ * Patterns in error messages that indicate expected errors (rate limits, etc.)
+ * These are checked when no numeric error code is available.
+ */
+const EXPECTED_ERROR_MESSAGE_PATTERNS = [
+	/^429\b/, // Message starts with "429"
+	/rate limit/i, // Contains "rate limit" (case insensitive)
+]
+
+/**
+ * Interface representing the error structure from OpenAI SDK.
+ * OpenAI SDK errors (APIError, AuthenticationError, RateLimitError, etc.)
+ * have a numeric `status` property and may contain nested error metadata.
+ *
+ * @see https://github.com/openai/openai-node/blob/master/src/error.ts
+ */
+interface OpenAISdkError {
+	/** HTTP status code of the error response */
+	status: number
+	/** Optional error code (may be numeric or string) */
+	code?: number | string
+	/** Primary error message */
+	message: string
+	/** Nested error object containing additional details from the API response */
+	error?: {
+		message?: string
+		metadata?: {
+			/** Raw error message from upstream provider (e.g., OpenRouter upstream errors) */
+			raw?: string
+		}
+	}
+}
+
+/**
+ * Type guard to check if an error object is an OpenAI SDK error.
+ * OpenAI SDK errors (APIError and subclasses) have: status, code, message properties.
+ */
+function isOpenAISdkError(error: unknown): error is OpenAISdkError {
+	return (
+		typeof error === "object" &&
+		error !== null &&
+		"status" in error &&
+		typeof (error as OpenAISdkError).status === "number"
+	)
+}
+
+/**
+ * Extracts the HTTP status code from an error object.
+ * Supports OpenAI SDK errors that have a status property.
+ * @param error - The error to extract status from
+ * @returns The status code if available, undefined otherwise
+ */
+export function getErrorStatusCode(error: unknown): number | undefined {
+	if (isOpenAISdkError(error)) {
+		return error.status
+	}
+	return undefined
+}
+
+/**
+ * Extracts the most descriptive error message from an OpenAI SDK error.
+ * Prioritizes nested metadata (upstream provider errors) over the standard message.
+ * @param error - The error to extract message from
+ * @returns The best available error message, or undefined if not an OpenAI SDK error
+ */
+export function getOpenAISdkErrorMessage(error: unknown): string | undefined {
+	if (isOpenAISdkError(error)) {
+		// Prioritize nested metadata which may contain upstream provider details
+		return error.error?.metadata?.raw || error.error?.message || error.message
+	}
+	return undefined
+}
+
 /**
  * Helper to check if an API error should be reported to telemetry.
- * Filters out expected errors like rate limits.
+ * Filters out expected errors like rate limits by checking both error codes and messages.
  * @param errorCode - The HTTP error code (if available)
+ * @param errorMessage - The error message (if available)
  * @returns true if the error should be reported, false if it should be filtered out
  */
-export function shouldReportApiErrorToTelemetry(errorCode?: number): boolean {
-	if (errorCode === undefined) return true
-	return !EXPECTED_API_ERROR_CODES.has(errorCode)
+export function shouldReportApiErrorToTelemetry(errorCode?: number, errorMessage?: string): boolean {
+	// Check numeric error code
+	if (errorCode !== undefined && EXPECTED_API_ERROR_CODES.has(errorCode)) {
+		return false
+	}
+
+	// Check error message for expected patterns (e.g., "429 Rate limit exceeded")
+	if (errorMessage) {
+		for (const pattern of EXPECTED_ERROR_MESSAGE_PATTERNS) {
+			if (pattern.test(errorMessage)) {
+				return false
+			}
+		}
+	}
+
+	return true
 }
 
 /**