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

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
 }
 
 /**

src/api/providers/__tests__/openrouter.spec.ts

@@ -321,6 +321,50 @@ describe("OpenRouterHandler", () => {
 			})
 		})
 
+		it("does NOT capture telemetry for SDK exceptions with status 429", 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")
+
+			// Verify telemetry was NOT captured for rate limit errors
+			expect(mockCaptureException).not.toHaveBeenCalled()
+		})
+
+		it("does NOT capture telemetry for SDK exceptions with 429 in message (fallback)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const mockCreate = vitest.fn().mockRejectedValue(new Error("429 Rate limit exceeded: free-models-per-day"))
+			;(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")
+
+			// Verify telemetry was NOT captured for rate limit errors (via message parsing)
+			expect(mockCaptureException).not.toHaveBeenCalled()
+		})
+
+		it("does NOT capture telemetry for SDK exceptions containing 'rate limit'", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const mockCreate = vitest.fn().mockRejectedValue(new Error("Request failed due to rate limit"))
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			const generator = handler.createMessage("test", [])
+			await expect(generator.next()).rejects.toThrow("rate limit")
+
+			// Verify telemetry was NOT captured for rate limit errors
+			expect(mockCaptureException).not.toHaveBeenCalled()
+		})
+
 		it("does NOT capture telemetry for 429 rate limit errors", async () => {
 			const handler = new OpenRouterHandler(mockOptions)
 			const mockStream = {
@@ -483,6 +527,47 @@ describe("OpenRouterHandler", () => {
 			})
 		})
 
+		it("does NOT capture telemetry for SDK exceptions with status 429", 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")
+
+			// Verify telemetry was NOT captured for rate limit errors
+			expect(mockCaptureException).not.toHaveBeenCalled()
+		})
+
+		it("does NOT capture telemetry for SDK exceptions with 429 in message (fallback)", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const mockCreate = vitest.fn().mockRejectedValue(new Error("429 Rate limit exceeded: free-models-per-day"))
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			await expect(handler.completePrompt("test prompt")).rejects.toThrow("429 Rate limit exceeded")
+
+			// Verify telemetry was NOT captured for rate limit errors (via message parsing)
+			expect(mockCaptureException).not.toHaveBeenCalled()
+		})
+
+		it("does NOT capture telemetry for SDK exceptions containing 'rate limit'", async () => {
+			const handler = new OpenRouterHandler(mockOptions)
+			const mockCreate = vitest.fn().mockRejectedValue(new Error("Request failed due to rate limit"))
+			;(OpenAI as any).prototype.chat = {
+				completions: { create: mockCreate },
+			} as any
+
+			await expect(handler.completePrompt("test prompt")).rejects.toThrow("rate limit")
+
+			// Verify telemetry was NOT captured for rate limit errors
+			expect(mockCaptureException).not.toHaveBeenCalled()
+		})
+
 		it("does NOT capture telemetry for 429 rate limit errors", async () => {
 			const handler = new OpenRouterHandler(mockOptions)
 			const mockError = {