diff --git a/packages/telemetry/src/PostHogTelemetryClient.ts b/packages/telemetry/src/PostHogTelemetryClient.ts index cea945f1580..1b491111bda 100644 --- a/packages/telemetry/src/PostHogTelemetryClient.ts +++ b/packages/telemetry/src/PostHogTelemetryClient.ts @@ -1,7 +1,15 @@ import { PostHog } from "posthog-node" 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" @@ -70,11 +78,37 @@ export class PostHogTelemetryClient extends BaseTelemetryClient { 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) { 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) } /** diff --git a/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts b/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts index 282d1d6c6ab..4e4c5179924 100644 --- a/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts +++ b/packages/telemetry/src/__tests__/PostHogTelemetryClient.test.ts @@ -1,11 +1,11 @@ /* 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 { 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" @@ -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,227 @@ 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 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", + }) + }) + }) }) diff --git a/packages/types/src/__tests__/telemetry.test.ts b/packages/types/src/__tests__/telemetry.test.ts new file mode 100644 index 00000000000..57baf605e02 --- /dev/null +++ b/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) + }) + }) +}) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index eb934a09263..912864a2145 100644 --- a/packages/types/src/telemetry.ts +++ b/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 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. - * 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 } /** @@ -303,3 +390,30 @@ export class ApiProviderError extends Error { 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 { + return { + provider: error.provider, + modelId: error.modelId, + operation: error.operation, + ...(error.errorCode !== undefined && { errorCode: error.errorCode }), + } +} diff --git a/src/api/providers/__tests__/openrouter.spec.ts b/src/api/providers/__tests__/openrouter.spec.ts index 3347d96431e..c3c54d24c24 100644 --- a/src/api/providers/__tests__/openrouter.spec.ts +++ b/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", () => ({})) import { Anthropic } from "@anthropic-ai/sdk" @@ -9,14 +8,12 @@ import OpenAI from "openai" import { OpenRouterHandler } from "../openrouter" import { ApiHandlerOptions } from "../../../shared/api" import { Package } from "../../../shared/package" -import { ApiProviderError } from "@roo-code/types" -// Mock dependencies vitest.mock("openai") vitest.mock("delay", () => ({ default: vitest.fn(() => Promise.resolve()) })) -// Mock TelemetryService const mockCaptureException = vitest.fn() + vitest.mock("@roo-code/telemetry", () => ({ TelemetryService: { instance: { @@ -24,6 +21,7 @@ vitest.mock("@roo-code/telemetry", () => ({ }, }, })) + vitest.mock("../fetchers/modelCache", () => ({ getModels: vitest.fn().mockImplementation(() => { return Promise.resolve({ @@ -294,13 +292,16 @@ describe("OpenRouterHandler", () => { const generator = handler.createMessage("test", []) 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 () => { @@ -313,15 +314,82 @@ describe("OpenRouterHandler", () => { const generator = handler.createMessage("test", []) 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 mockStream = { async *[Symbol.asyncIterator]() { @@ -337,8 +405,16 @@ describe("OpenRouterHandler", () => { const generator = handler.createMessage("test", []) 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 () => { @@ -458,32 +534,104 @@ describe("OpenRouterHandler", () => { await expect(handler.completePrompt("test prompt")).rejects.toThrow("OpenRouter API Error 500: API Error") // 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 () => { 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 = { completions: { create: mockCreate }, } as any 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 mockError = { error: { @@ -501,8 +649,17 @@ describe("OpenRouterHandler", () => { "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, + }), + ) }) }) }) diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index ec9e04c4feb..ffd5e946e49 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -7,7 +7,6 @@ import { OPENROUTER_DEFAULT_PROVIDER_NAME, OPEN_ROUTER_PROMPT_CACHING_MODELS, DEEP_SEEK_DEFAULT_TEMPERATURE, - shouldReportApiErrorToTelemetry, ApiProviderError, } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" @@ -234,8 +233,8 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH modelId, "createMessage", ), - { provider: this.providerName, modelId, operation: "createMessage" }, ) + throw handleOpenAIError(error, this.providerName) } @@ -260,8 +259,9 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH if ("error" in chunk) { const error = chunk.error as { message?: string; code?: number } console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`) - if (shouldReportApiErrorToTelemetry(error?.code)) { - TelemetryService.instance.captureException( + + TelemetryService.instance.captureException( + Object.assign( new ApiProviderError( error?.message ?? "Unknown error", this.providerName, @@ -269,9 +269,10 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH "createMessage", error?.code, ), - { provider: this.providerName, modelId, operation: "createMessage", errorCode: error?.code }, - ) - } + { status: error?.code }, + ), + ) + throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } @@ -463,6 +464,7 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH : undefined let response + try { response = await this.client.chat.completions.create(completionParams, requestOptions) } catch (error) { @@ -473,15 +475,17 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH modelId, "completePrompt", ), - { provider: this.providerName, modelId, operation: "completePrompt" }, ) + throw handleOpenAIError(error, this.providerName) } if ("error" in response) { 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( error?.message ?? "Unknown error", this.providerName, @@ -489,9 +493,10 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH "completePrompt", error?.code, ), - { provider: this.providerName, modelId, operation: "completePrompt", errorCode: error?.code }, - ) - } + { status: error?.code }, + ), + ) + throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) }