Просмотр исходного кода

fix: filter out 429 rate limit errors from API error telemetry (#9987)

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: cte <[email protected]>
Daniel 3 недель назад
Родитель
Сommit
fda020a418

+ 36 - 2
packages/telemetry/src/PostHogTelemetryClient.ts

@@ -1,7 +1,15 @@
 import { PostHog } from "posthog-node"
 import { PostHog } from "posthog-node"
 import * as vscode from "vscode"
 import * as vscode from "vscode"
 
 
-import { TelemetryEventName, type TelemetryEvent } from "@roo-code/types"
+import {
+	TelemetryEventName,
+	type TelemetryEvent,
+	getErrorStatusCode,
+	getErrorMessage,
+	shouldReportApiErrorToTelemetry,
+	isApiProviderError,
+	extractApiProviderErrorProperties,
+} from "@roo-code/types"
 
 
 import { BaseTelemetryClient } from "./BaseTelemetryClient"
 import { BaseTelemetryClient } from "./BaseTelemetryClient"
 
 
@@ -70,11 +78,37 @@ export class PostHogTelemetryClient extends BaseTelemetryClient {
 			return
 			return
 		}
 		}
 
 
+		// Extract error status code and message for filtering.
+		const errorCode = getErrorStatusCode(error)
+		const errorMessage = getErrorMessage(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) {
 		if (this.debug) {
 			console.info(`[PostHogTelemetryClient#captureException] ${error.message}`)
 			console.info(`[PostHogTelemetryClient#captureException] ${error.message}`)
 		}
 		}
 
 
-		this.client.captureException(error, this.distinctId, additionalProperties)
+		// Auto-extract properties from ApiProviderError and merge with additionalProperties.
+		// Explicit additionalProperties take precedence over auto-extracted properties.
+		let mergedProperties = additionalProperties
+
+		if (isApiProviderError(error)) {
+			const extractedProperties = extractApiProviderErrorProperties(error)
+			mergedProperties = { ...extractedProperties, ...additionalProperties }
+		}
+
+		// Override the error message with the extracted error message.
+		error.message = errorMessage
+
+		this.client.captureException(error, this.distinctId, mergedProperties)
 	}
 	}
 
 
 	/**
 	/**

+ 226 - 2
packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts

@@ -1,11 +1,11 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
 
-// npx vitest run src/__tests__/PostHogTelemetryClient.test.ts
+// pnpm --filter @roo-code/telemetry test src/__tests__/PostHogTelemetryClient.test.ts
 
 
 import * as vscode from "vscode"
 import * as vscode from "vscode"
 import { PostHog } from "posthog-node"
 import { PostHog } from "posthog-node"
 
 
-import { type TelemetryPropertiesProvider, TelemetryEventName } from "@roo-code/types"
+import { type TelemetryPropertiesProvider, TelemetryEventName, ApiProviderError } from "@roo-code/types"
 
 
 import { PostHogTelemetryClient } from "../PostHogTelemetryClient"
 import { PostHogTelemetryClient } from "../PostHogTelemetryClient"
 
 
@@ -32,6 +32,7 @@ describe("PostHogTelemetryClient", () => {
 
 
 		mockPostHogClient = {
 		mockPostHogClient = {
 			capture: vi.fn(),
 			capture: vi.fn(),
+			captureException: vi.fn(),
 			optIn: vi.fn(),
 			optIn: vi.fn(),
 			optOut: vi.fn(),
 			optOut: vi.fn(),
 			shutdown: vi.fn().mockResolvedValue(undefined),
 			shutdown: vi.fn().mockResolvedValue(undefined),
@@ -373,4 +374,227 @@ describe("PostHogTelemetryClient", () => {
 			expect(mockPostHogClient.shutdown).toHaveBeenCalled()
 			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 for filtering", () => {
+			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()
+		})
+
+		it("should modify error.message with extracted message from nested metadata.raw", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			// Create an OpenAI SDK-like error with nested metadata (non-rate-limit error)
+			const error = Object.assign(new Error("Generic request failed"), {
+				status: 500,
+				error: {
+					message: "Nested error message",
+					metadata: { raw: "Upstream provider error: model overloaded" },
+				},
+			})
+
+			client.captureException(error)
+
+			// Verify error message was modified to use metadata.raw (highest priority)
+			expect(error.message).toBe("Upstream provider error: model overloaded")
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+		})
+
+		it("should modify error.message with nested error.message when metadata.raw is not available", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			// Create an OpenAI SDK-like error with nested message but no metadata.raw
+			const error = Object.assign(new Error("Generic request failed"), {
+				status: 500,
+				error: {
+					message: "Upstream provider: connection timeout",
+				},
+			})
+
+			client.captureException(error)
+
+			// Verify error message was modified to use nested error.message
+			expect(error.message).toBe("Upstream provider: connection timeout")
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+		})
+
+		it("should use primary message when no nested error structure exists", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			// Create an OpenAI SDK-like error without nested error object
+			const error = Object.assign(new Error("Primary error message"), {
+				status: 500,
+			})
+
+			client.captureException(error)
+
+			// Verify error message remains the primary message
+			expect(error.message).toBe("Primary error message")
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", undefined)
+		})
+
+		it("should auto-extract properties from ApiProviderError", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
+			client.captureException(error)
+
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				provider: "OpenRouter",
+				modelId: "gpt-4",
+				operation: "createMessage",
+				errorCode: 500,
+			})
+		})
+
+		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", () => {
+			const client = new PostHogTelemetryClient()
+			client.updateTelemetryState(true)
+
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
+			client.captureException(error, { customProperty: "value" })
+
+			expect(mockPostHogClient.captureException).toHaveBeenCalledWith(error, "test-machine-id", {
+				provider: "OpenRouter",
+				modelId: "gpt-4",
+				operation: "createMessage",
+				customProperty: "value",
+			})
+		})
+
+		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", () => {
+			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",
+			})
+		})
+	})
 })
 })

+ 245 - 0
packages/types/src/__tests__/telemetry.test.ts

@@ -0,0 +1,245 @@
+// pnpm --filter @roo-code/types test src/__tests__/telemetry.test.ts
+
+import {
+	getErrorStatusCode,
+	getErrorMessage,
+	shouldReportApiErrorToTelemetry,
+	EXPECTED_API_ERROR_CODES,
+	ApiProviderError,
+	isApiProviderError,
+	extractApiProviderErrorProperties,
+} 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("getErrorMessage", () => {
+		it("should return undefined for non-OpenAI SDK errors", () => {
+			expect(getErrorMessage(null)).toBeUndefined()
+			expect(getErrorMessage(undefined)).toBeUndefined()
+			expect(getErrorMessage({ message: "error" })).toBeUndefined()
+		})
+
+		it("should return the primary message for simple OpenAI SDK errors", () => {
+			const error = { status: 400, message: "Bad request" }
+			expect(getErrorMessage(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(getErrorMessage(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(getErrorMessage(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(getErrorMessage(error)).toBe("Detailed error message")
+		})
+
+		it("should fallback to primary message when no nested messages exist", () => {
+			const error = {
+				status: 403,
+				message: "Forbidden",
+				error: {},
+			}
+			expect(getErrorMessage(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)
+		})
+	})
+
+	describe("ApiProviderError", () => {
+		it("should create an error with correct properties", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
+
+			expect(error.message).toBe("Test error")
+			expect(error.name).toBe("ApiProviderError")
+			expect(error.provider).toBe("OpenRouter")
+			expect(error.modelId).toBe("gpt-4")
+			expect(error.operation).toBe("createMessage")
+			expect(error.errorCode).toBe(500)
+		})
+
+		it("should work without optional errorCode", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
+
+			expect(error.message).toBe("Test error")
+			expect(error.provider).toBe("OpenRouter")
+			expect(error.modelId).toBe("gpt-4")
+			expect(error.operation).toBe("createMessage")
+			expect(error.errorCode).toBeUndefined()
+		})
+
+		it("should be an instance of Error", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
+			expect(error).toBeInstanceOf(Error)
+		})
+	})
+
+	describe("isApiProviderError", () => {
+		it("should return true for ApiProviderError instances", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
+			expect(isApiProviderError(error)).toBe(true)
+		})
+
+		it("should return true for ApiProviderError with errorCode", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 429)
+			expect(isApiProviderError(error)).toBe(true)
+		})
+
+		it("should return false for regular Error instances", () => {
+			const error = new Error("Test error")
+			expect(isApiProviderError(error)).toBe(false)
+		})
+
+		it("should return false for null and undefined", () => {
+			expect(isApiProviderError(null)).toBe(false)
+			expect(isApiProviderError(undefined)).toBe(false)
+		})
+
+		it("should return false for non-error objects", () => {
+			expect(isApiProviderError({})).toBe(false)
+			expect(isApiProviderError({ provider: "test", modelId: "test", operation: "test" })).toBe(false)
+		})
+
+		it("should return false for Error with wrong name", () => {
+			const error = new Error("Test error")
+			error.name = "CustomError"
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any
+			;(error as any).provider = "OpenRouter"
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any
+			;(error as any).modelId = "gpt-4"
+			// eslint-disable-next-line @typescript-eslint/no-explicit-any
+			;(error as any).operation = "createMessage"
+			expect(isApiProviderError(error)).toBe(false)
+		})
+	})
+
+	describe("extractApiProviderErrorProperties", () => {
+		it("should extract all properties from ApiProviderError", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 500)
+			const properties = extractApiProviderErrorProperties(error)
+
+			expect(properties).toEqual({
+				provider: "OpenRouter",
+				modelId: "gpt-4",
+				operation: "createMessage",
+				errorCode: 500,
+			})
+		})
+
+		it("should not include errorCode when undefined", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage")
+			const properties = extractApiProviderErrorProperties(error)
+
+			expect(properties).toEqual({
+				provider: "OpenRouter",
+				modelId: "gpt-4",
+				operation: "createMessage",
+			})
+			expect(properties).not.toHaveProperty("errorCode")
+		})
+
+		it("should include errorCode when it is 0", () => {
+			const error = new ApiProviderError("Test error", "OpenRouter", "gpt-4", "createMessage", 0)
+			const properties = extractApiProviderErrorProperties(error)
+
+			// errorCode of 0 is falsy but !== undefined, so it should be included
+			expect(properties).toHaveProperty("errorCode", 0)
+		})
+	})
+})

+ 118 - 4
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
 	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 getErrorMessage(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.
  * 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 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
  * @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
 }
 }
 
 
 /**
 /**
@@ -303,3 +390,30 @@ export class ApiProviderError extends Error {
 		this.name = "ApiProviderError"
 		this.name = "ApiProviderError"
 	}
 	}
 }
 }
+
+/**
+ * Type guard to check if an error is an ApiProviderError.
+ * Used by telemetry to automatically extract structured properties.
+ */
+export function isApiProviderError(error: unknown): error is ApiProviderError {
+	return (
+		error instanceof Error &&
+		error.name === "ApiProviderError" &&
+		"provider" in error &&
+		"modelId" in error &&
+		"operation" in error
+	)
+}
+
+/**
+ * Extracts properties from an ApiProviderError for telemetry.
+ * Returns the structured properties that can be merged with additionalProperties.
+ */
+export function extractApiProviderErrorProperties(error: ApiProviderError): Record<string, unknown> {
+	return {
+		provider: error.provider,
+		modelId: error.modelId,
+		operation: error.operation,
+		...(error.errorCode !== undefined && { errorCode: error.errorCode }),
+	}
+}

+ 194 - 37
src/api/providers/__tests__/openrouter.spec.ts

@@ -1,6 +1,5 @@
-// npx vitest run src/api/providers/__tests__/openrouter.spec.ts
+// pnpm --filter roo-cline test api/providers/__tests__/openrouter.spec.ts
 
 
-// Mock vscode first to avoid import errors
 vitest.mock("vscode", () => ({}))
 vitest.mock("vscode", () => ({}))
 
 
 import { Anthropic } from "@anthropic-ai/sdk"
 import { Anthropic } from "@anthropic-ai/sdk"
@@ -9,14 +8,12 @@ import OpenAI from "openai"
 import { OpenRouterHandler } from "../openrouter"
 import { OpenRouterHandler } from "../openrouter"
 import { ApiHandlerOptions } from "../../../shared/api"
 import { ApiHandlerOptions } from "../../../shared/api"
 import { Package } from "../../../shared/package"
 import { Package } from "../../../shared/package"
-import { ApiProviderError } from "@roo-code/types"
 
 
-// Mock dependencies
 vitest.mock("openai")
 vitest.mock("openai")
 vitest.mock("delay", () => ({ default: vitest.fn(() => Promise.resolve()) }))
 vitest.mock("delay", () => ({ default: vitest.fn(() => Promise.resolve()) }))
 
 
-// Mock TelemetryService
 const mockCaptureException = vitest.fn()
 const mockCaptureException = vitest.fn()
+
 vitest.mock("@roo-code/telemetry", () => ({
 vitest.mock("@roo-code/telemetry", () => ({
 	TelemetryService: {
 	TelemetryService: {
 		instance: {
 		instance: {
@@ -24,6 +21,7 @@ vitest.mock("@roo-code/telemetry", () => ({
 		},
 		},
 	},
 	},
 }))
 }))
+
 vitest.mock("../fetchers/modelCache", () => ({
 vitest.mock("../fetchers/modelCache", () => ({
 	getModels: vitest.fn().mockImplementation(() => {
 	getModels: vitest.fn().mockImplementation(() => {
 		return Promise.resolve({
 		return Promise.resolve({
@@ -294,13 +292,16 @@ describe("OpenRouterHandler", () => {
 			const generator = handler.createMessage("test", [])
 			const generator = handler.createMessage("test", [])
 			await expect(generator.next()).rejects.toThrow("OpenRouter API Error 500: API Error")
 			await expect(generator.next()).rejects.toThrow("OpenRouter API Error 500: API Error")
 
 
-			// Verify telemetry was captured
-			expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), {
-				provider: "OpenRouter",
-				modelId: mockOptions.openRouterModelId,
-				operation: "createMessage",
-				errorCode: 500,
-			})
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "API Error",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "createMessage",
+					errorCode: 500,
+					status: 500,
+				}),
+			)
 		})
 		})
 
 
 		it("captures telemetry when createMessage throws an exception", async () => {
 		it("captures telemetry when createMessage throws an exception", async () => {
@@ -313,15 +314,82 @@ describe("OpenRouterHandler", () => {
 			const generator = handler.createMessage("test", [])
 			const generator = handler.createMessage("test", [])
 			await expect(generator.next()).rejects.toThrow()
 			await expect(generator.next()).rejects.toThrow()
 
 
-			// Verify telemetry was captured
-			expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), {
-				provider: "OpenRouter",
-				modelId: mockOptions.openRouterModelId,
-				operation: "createMessage",
-			})
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Connection failed",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "createMessage",
+				}),
+			)
+		})
+
+		it("passes SDK exceptions with status 429 to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const error = new Error("Rate limit exceeded: free-models-per-day") as any
+			error.status = 429
+
+			const mockCreate = vitest.fn().mockRejectedValue(error)
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			const generator = handler.createMessage("test", [])
+			await expect(generator.next()).rejects.toThrow("Rate limit exceeded")
+
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Rate limit exceeded: free-models-per-day",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "createMessage",
+				}),
+			)
 		})
 		})
 
 
-		it("does NOT capture telemetry for 429 rate limit errors", async () => {
+		it("passes SDK exceptions with 429 in message to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const error = new Error("429 Rate limit exceeded: free-models-per-day")
+			const mockCreate = vitest.fn().mockRejectedValue(error)
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			const generator = handler.createMessage("test", [])
+			await expect(generator.next()).rejects.toThrow("429 Rate limit exceeded")
+
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "429 Rate limit exceeded: free-models-per-day",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "createMessage",
+				}),
+			)
+		})
+
+		it("passes SDK exceptions containing 'rate limit' to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const error = new Error("Request failed due to rate limit")
+			const mockCreate = vitest.fn().mockRejectedValue(error)
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			const generator = handler.createMessage("test", [])
+			await expect(generator.next()).rejects.toThrow("rate limit")
+
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Request failed due to rate limit",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "createMessage",
+				}),
+			)
+		})
+
+		it("passes 429 rate limit errors from stream to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
 			const handler = new OpenRouterHandler(mockOptions)
 			const handler = new OpenRouterHandler(mockOptions)
 			const mockStream = {
 			const mockStream = {
 				async *[Symbol.asyncIterator]() {
 				async *[Symbol.asyncIterator]() {
@@ -337,8 +405,16 @@ describe("OpenRouterHandler", () => {
 			const generator = handler.createMessage("test", [])
 			const generator = handler.createMessage("test", [])
 			await expect(generator.next()).rejects.toThrow("OpenRouter API Error 429: Rate limit exceeded")
 			await expect(generator.next()).rejects.toThrow("OpenRouter API Error 429: Rate limit exceeded")
 
 
-			// Verify telemetry was NOT captured for 429 errors
-			expect(mockCaptureException).not.toHaveBeenCalled()
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Rate limit exceeded",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "createMessage",
+					errorCode: 429,
+					status: 429,
+				}),
+			)
 		})
 		})
 
 
 		it("yields tool_call_end events when finish_reason is tool_calls", async () => {
 		it("yields tool_call_end events when finish_reason is tool_calls", async () => {
@@ -458,32 +534,104 @@ describe("OpenRouterHandler", () => {
 			await expect(handler.completePrompt("test prompt")).rejects.toThrow("OpenRouter API Error 500: API Error")
 			await expect(handler.completePrompt("test prompt")).rejects.toThrow("OpenRouter API Error 500: API Error")
 
 
 			// Verify telemetry was captured
 			// Verify telemetry was captured
-			expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), {
-				provider: "OpenRouter",
-				modelId: mockOptions.openRouterModelId,
-				operation: "completePrompt",
-				errorCode: 500,
-			})
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "API Error",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "completePrompt",
+					errorCode: 500,
+					status: 500,
+				}),
+			)
 		})
 		})
 
 
 		it("handles unexpected errors and captures telemetry", async () => {
 		it("handles unexpected errors and captures telemetry", async () => {
 			const handler = new OpenRouterHandler(mockOptions)
 			const handler = new OpenRouterHandler(mockOptions)
-			const mockCreate = vitest.fn().mockRejectedValue(new Error("Unexpected error"))
+			const error = new Error("Unexpected error")
+			const mockCreate = vitest.fn().mockRejectedValue(error)
 			;(OpenAI as any).prototype.chat = {
 			;(OpenAI as any).prototype.chat = {
 				completions: { create: mockCreate },
 				completions: { create: mockCreate },
 			} as any
 			} as any
 
 
 			await expect(handler.completePrompt("test prompt")).rejects.toThrow("Unexpected error")
 			await expect(handler.completePrompt("test prompt")).rejects.toThrow("Unexpected error")
 
 
-			// Verify telemetry was captured
-			expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), {
-				provider: "OpenRouter",
-				modelId: mockOptions.openRouterModelId,
-				operation: "completePrompt",
-			})
+			// Verify telemetry was captured (filtering now happens inside PostHogTelemetryClient)
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Unexpected error",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "completePrompt",
+				}),
+			)
+		})
+
+		it("passes SDK exceptions with status 429 to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const error = new Error("Rate limit exceeded: free-models-per-day") as any
+			error.status = 429
+			const mockCreate = vitest.fn().mockRejectedValue(error)
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			await expect(handler.completePrompt("test prompt")).rejects.toThrow("Rate limit exceeded")
+
+			// captureException is called, but PostHogTelemetryClient filters out 429 errors internally
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Rate limit exceeded: free-models-per-day",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "completePrompt",
+				}),
+			)
+		})
+
+		it("passes SDK exceptions with 429 in message to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const error = new Error("429 Rate limit exceeded: free-models-per-day")
+			const mockCreate = vitest.fn().mockRejectedValue(error)
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			await expect(handler.completePrompt("test prompt")).rejects.toThrow("429 Rate limit exceeded")
+
+			// captureException is called, but PostHogTelemetryClient filters out 429 errors internally
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "429 Rate limit exceeded: free-models-per-day",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "completePrompt",
+				}),
+			)
 		})
 		})
 
 
-		it("does NOT capture telemetry for 429 rate limit errors", async () => {
+		it("passes SDK exceptions containing 'rate limit' to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const error = new Error("Request failed due to rate limit")
+			const mockCreate = vitest.fn().mockRejectedValue(error)
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			await expect(handler.completePrompt("test prompt")).rejects.toThrow("rate limit")
+
+			// captureException is called, but PostHogTelemetryClient filters out rate limit errors internally
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Request failed due to rate limit",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "completePrompt",
+				}),
+			)
+		})
+
+		it("passes 429 rate limit errors from response to telemetry (filtering happens in PostHogTelemetryClient)", async () => {
 			const handler = new OpenRouterHandler(mockOptions)
 			const handler = new OpenRouterHandler(mockOptions)
 			const mockError = {
 			const mockError = {
 				error: {
 				error: {
@@ -501,8 +649,17 @@ describe("OpenRouterHandler", () => {
 				"OpenRouter API Error 429: Rate limit exceeded",
 				"OpenRouter API Error 429: Rate limit exceeded",
 			)
 			)
 
 
-			// Verify telemetry was NOT captured for 429 errors
-			expect(mockCaptureException).not.toHaveBeenCalled()
+			// captureException is called, but PostHogTelemetryClient filters out 429 errors internally
+			expect(mockCaptureException).toHaveBeenCalledWith(
+				expect.objectContaining({
+					message: "Rate limit exceeded",
+					provider: "OpenRouter",
+					modelId: mockOptions.openRouterModelId,
+					operation: "completePrompt",
+					errorCode: 429,
+					status: 429,
+				}),
+			)
 		})
 		})
 	})
 	})
 })
 })

+ 18 - 13
src/api/providers/openrouter.ts

@@ -7,7 +7,6 @@ import {
 	OPENROUTER_DEFAULT_PROVIDER_NAME,
 	OPENROUTER_DEFAULT_PROVIDER_NAME,
 	OPEN_ROUTER_PROMPT_CACHING_MODELS,
 	OPEN_ROUTER_PROMPT_CACHING_MODELS,
 	DEEP_SEEK_DEFAULT_TEMPERATURE,
 	DEEP_SEEK_DEFAULT_TEMPERATURE,
-	shouldReportApiErrorToTelemetry,
 	ApiProviderError,
 	ApiProviderError,
 } from "@roo-code/types"
 } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 import { TelemetryService } from "@roo-code/telemetry"
@@ -234,8 +233,8 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
 					modelId,
 					modelId,
 					"createMessage",
 					"createMessage",
 				),
 				),
-				{ provider: this.providerName, modelId, operation: "createMessage" },
 			)
 			)
+
 			throw handleOpenAIError(error, this.providerName)
 			throw handleOpenAIError(error, this.providerName)
 		}
 		}
 
 
@@ -260,8 +259,9 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
 			if ("error" in chunk) {
 			if ("error" in chunk) {
 				const error = chunk.error as { message?: string; code?: number }
 				const error = chunk.error as { message?: string; code?: number }
 				console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`)
 				console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`)
-				if (shouldReportApiErrorToTelemetry(error?.code)) {
-					TelemetryService.instance.captureException(
+
+				TelemetryService.instance.captureException(
+					Object.assign(
 						new ApiProviderError(
 						new ApiProviderError(
 							error?.message ?? "Unknown error",
 							error?.message ?? "Unknown error",
 							this.providerName,
 							this.providerName,
@@ -269,9 +269,10 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
 							"createMessage",
 							"createMessage",
 							error?.code,
 							error?.code,
 						),
 						),
-						{ provider: this.providerName, modelId, operation: "createMessage", errorCode: error?.code },
-					)
-				}
+						{ status: error?.code },
+					),
+				)
+
 				throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`)
 				throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`)
 			}
 			}
 
 
@@ -463,6 +464,7 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
 			: undefined
 			: undefined
 
 
 		let response
 		let response
+
 		try {
 		try {
 			response = await this.client.chat.completions.create(completionParams, requestOptions)
 			response = await this.client.chat.completions.create(completionParams, requestOptions)
 		} catch (error) {
 		} catch (error) {
@@ -473,15 +475,17 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
 					modelId,
 					modelId,
 					"completePrompt",
 					"completePrompt",
 				),
 				),
-				{ provider: this.providerName, modelId, operation: "completePrompt" },
 			)
 			)
+
 			throw handleOpenAIError(error, this.providerName)
 			throw handleOpenAIError(error, this.providerName)
 		}
 		}
 
 
 		if ("error" in response) {
 		if ("error" in response) {
 			const error = response.error as { message?: string; code?: number }
 			const error = response.error as { message?: string; code?: number }
-			if (shouldReportApiErrorToTelemetry(error?.code)) {
-				TelemetryService.instance.captureException(
+			console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`)
+
+			TelemetryService.instance.captureException(
+				Object.assign(
 					new ApiProviderError(
 					new ApiProviderError(
 						error?.message ?? "Unknown error",
 						error?.message ?? "Unknown error",
 						this.providerName,
 						this.providerName,
@@ -489,9 +493,10 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
 						"completePrompt",
 						"completePrompt",
 						error?.code,
 						error?.code,
 					),
 					),
-					{ provider: this.providerName, modelId, operation: "completePrompt", errorCode: error?.code },
-				)
-			}
+					{ status: error?.code },
+				),
+			)
+
 			throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`)
 			throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`)
 		}
 		}