Skip to content

Commit 79bb535

Browse files
daniel-lxsdaniel-lxs
committed
Fix: filter out 429 rate limit errors from telemetry
- Add SdkErrorWithStatus interface matching OpenAI SDK error structure - Add isSdkErrorWithStatus type guard to properly extract error.status - Add getErrorStatusCode helper function for type-safe status extraction - Update shouldReportApiErrorToTelemetry to check both error codes and messages - Add message pattern matching for '429' prefix and 'rate limit' text - Filter out expected errors (429, rate limits) from telemetry reporting - Add comprehensive tests for all error scenarios
1 parent 36ef603 commit 79bb535

File tree

3 files changed

+168
-24
lines changed

3 files changed

+168
-24
lines changed

‎packages/types/src/telemetry.ts‎

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,15 +276,75 @@ export const EXPECTED_API_ERROR_CODES = new Set([
276276
429, // Rate limit - expected when hitting API limits
277277
])
278278

279+
/**
280+
* Patterns in error messages that indicate expected errors (rate limits, etc.)
281+
* These are checked when no numeric error code is available.
282+
*/
283+
const EXPECTED_ERROR_MESSAGE_PATTERNS = [
284+
/^429\b/, // Message starts with "429"
285+
/rate limit/i, // Contains "rate limit" (case insensitive)
286+
]
287+
288+
/**
289+
* Interface for SDK errors that have HTTP status information.
290+
* OpenAI SDK errors (APIError, AuthenticationError, RateLimitError, etc.)
291+
* all extend this interface with a numeric status property.
292+
*/
293+
interface SdkErrorWithStatus {
294+
status: number
295+
code?: number | string
296+
message: string
297+
}
298+
299+
/**
300+
* Type guard to check if an error object is an SDK error with status property.
301+
* OpenAI SDK errors (APIError and subclasses) have: status, code, message properties.
302+
*/
303+
function isSdkErrorWithStatus(error: unknown): error is SdkErrorWithStatus {
304+
return (
305+
typeof error === "object" &&
306+
error !== null &&
307+
"status" in error &&
308+
typeof (error as SdkErrorWithStatus).status === "number"
309+
)
310+
}
311+
312+
/**
313+
* Extracts the HTTP status code from an error object.
314+
* Supports SDK errors that have a status property (e.g., OpenAI APIError).
315+
* @param error - The error to extract status from
316+
* @returns The status code if available, undefined otherwise
317+
*/
318+
export function getErrorStatusCode(error: unknown): number | undefined {
319+
if (isSdkErrorWithStatus(error)) {
320+
return error.status
321+
}
322+
return undefined
323+
}
324+
279325
/**
280326
* Helper to check if an API error should be reported to telemetry.
281-
* Filters out expected errors like rate limits.
327+
* Filters out expected errors like rate limits by checking both error codes and messages.
282328
* @param errorCode - The HTTP error code (if available)
329+
* @param errorMessage - The error message (if available)
283330
* @returns true if the error should be reported, false if it should be filtered out
284331
*/
285-
export function shouldReportApiErrorToTelemetry(errorCode?: number): boolean {
286-
if (errorCode === undefined) return true
287-
return !EXPECTED_API_ERROR_CODES.has(errorCode)
332+
export function shouldReportApiErrorToTelemetry(errorCode?: number, errorMessage?: string): boolean {
333+
// Check numeric error code
334+
if (errorCode !== undefined && EXPECTED_API_ERROR_CODES.has(errorCode)) {
335+
return false
336+
}
337+
338+
// Check error message for expected patterns (e.g., "429 Rate limit exceeded")
339+
if (errorMessage) {
340+
for (const pattern of EXPECTED_ERROR_MESSAGE_PATTERNS) {
341+
if (pattern.test(errorMessage)) {
342+
return false
343+
}
344+
}
345+
}
346+
347+
return true
288348
}
289349

290350
/**

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

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,50 @@ describe("OpenRouterHandler", () => {
321321
})
322322
})
323323

324+
it("does NOT capture telemetry for SDK exceptions with status 429", async () => {
325+
const handler = new OpenRouterHandler(mockOptions)
326+
const error = new Error("Rate limit exceeded: free-models-per-day") as any
327+
error.status = 429
328+
const mockCreate = vitest.fn().mockRejectedValue(error)
329+
;(OpenAI as any).prototype.chat = {
330+
completions: { create: mockCreate },
331+
} as any
332+
333+
const generator = handler.createMessage("test", [])
334+
await expect(generator.next()).rejects.toThrow("Rate limit exceeded")
335+
336+
// Verify telemetry was NOT captured for rate limit errors
337+
expect(mockCaptureException).not.toHaveBeenCalled()
338+
})
339+
340+
it("does NOT capture telemetry for SDK exceptions with 429 in message (fallback)", async () => {
341+
const handler = new OpenRouterHandler(mockOptions)
342+
const mockCreate = vitest.fn().mockRejectedValue(new Error("429 Rate limit exceeded: free-models-per-day"))
343+
;(OpenAI as any).prototype.chat = {
344+
completions: { create: mockCreate },
345+
} as any
346+
347+
const generator = handler.createMessage("test", [])
348+
await expect(generator.next()).rejects.toThrow("429 Rate limit exceeded")
349+
350+
// Verify telemetry was NOT captured for rate limit errors (via message parsing)
351+
expect(mockCaptureException).not.toHaveBeenCalled()
352+
})
353+
354+
it("does NOT capture telemetry for SDK exceptions containing 'rate limit'", async () => {
355+
const handler = new OpenRouterHandler(mockOptions)
356+
const mockCreate = vitest.fn().mockRejectedValue(new Error("Request failed due to rate limit"))
357+
;(OpenAI as any).prototype.chat = {
358+
completions: { create: mockCreate },
359+
} as any
360+
361+
const generator = handler.createMessage("test", [])
362+
await expect(generator.next()).rejects.toThrow("rate limit")
363+
364+
// Verify telemetry was NOT captured for rate limit errors
365+
expect(mockCaptureException).not.toHaveBeenCalled()
366+
})
367+
324368
it("does NOT capture telemetry for 429 rate limit errors", async () => {
325369
const handler = new OpenRouterHandler(mockOptions)
326370
const mockStream = {
@@ -483,6 +527,47 @@ describe("OpenRouterHandler", () => {
483527
})
484528
})
485529

530+
it("does NOT capture telemetry for SDK exceptions with status 429", async () => {
531+
const handler = new OpenRouterHandler(mockOptions)
532+
const error = new Error("Rate limit exceeded: free-models-per-day") as any
533+
error.status = 429
534+
const mockCreate = vitest.fn().mockRejectedValue(error)
535+
;(OpenAI as any).prototype.chat = {
536+
completions: { create: mockCreate },
537+
} as any
538+
539+
await expect(handler.completePrompt("test prompt")).rejects.toThrow("Rate limit exceeded")
540+
541+
// Verify telemetry was NOT captured for rate limit errors
542+
expect(mockCaptureException).not.toHaveBeenCalled()
543+
})
544+
545+
it("does NOT capture telemetry for SDK exceptions with 429 in message (fallback)", async () => {
546+
const handler = new OpenRouterHandler(mockOptions)
547+
const mockCreate = vitest.fn().mockRejectedValue(new Error("429 Rate limit exceeded: free-models-per-day"))
548+
;(OpenAI as any).prototype.chat = {
549+
completions: { create: mockCreate },
550+
} as any
551+
552+
await expect(handler.completePrompt("test prompt")).rejects.toThrow("429 Rate limit exceeded")
553+
554+
// Verify telemetry was NOT captured for rate limit errors (via message parsing)
555+
expect(mockCaptureException).not.toHaveBeenCalled()
556+
})
557+
558+
it("does NOT capture telemetry for SDK exceptions containing 'rate limit'", async () => {
559+
const handler = new OpenRouterHandler(mockOptions)
560+
const mockCreate = vitest.fn().mockRejectedValue(new Error("Request failed due to rate limit"))
561+
;(OpenAI as any).prototype.chat = {
562+
completions: { create: mockCreate },
563+
} as any
564+
565+
await expect(handler.completePrompt("test prompt")).rejects.toThrow("rate limit")
566+
567+
// Verify telemetry was NOT captured for rate limit errors
568+
expect(mockCaptureException).not.toHaveBeenCalled()
569+
})
570+
486571
it("does NOT capture telemetry for 429 rate limit errors", async () => {
487572
const handler = new OpenRouterHandler(mockOptions)
488573
const mockError = {

‎src/api/providers/openrouter.ts‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
OPEN_ROUTER_PROMPT_CACHING_MODELS,
99
DEEP_SEEK_DEFAULT_TEMPERATURE,
1010
shouldReportApiErrorToTelemetry,
11+
getErrorStatusCode,
1112
ApiProviderError,
1213
} from "@roo-code/types"
1314
import { TelemetryService } from "@roo-code/telemetry"
@@ -227,15 +228,14 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
227228
try {
228229
stream = await this.client.chat.completions.create(completionParams, requestOptions)
229230
} catch (error) {
230-
TelemetryService.instance.captureException(
231-
new ApiProviderError(
232-
error instanceof Error ? error.message : String(error),
233-
this.providerName,
234-
modelId,
235-
"createMessage",
236-
),
237-
{ provider: this.providerName, modelId, operation: "createMessage" },
238-
)
231+
const errorMessage = error instanceof Error ? error.message : String(error)
232+
const errorStatus = getErrorStatusCode(error)
233+
if (shouldReportApiErrorToTelemetry(errorStatus, errorMessage)) {
234+
TelemetryService.instance.captureException(
235+
new ApiProviderError(errorMessage, this.providerName, modelId, "createMessage", errorStatus),
236+
{ provider: this.providerName, modelId, operation: "createMessage", errorCode: errorStatus },
237+
)
238+
}
239239
throw handleOpenAIError(error, this.providerName)
240240
}
241241

@@ -260,7 +260,7 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
260260
if ("error" in chunk) {
261261
const error = chunk.error as { message?: string; code?: number }
262262
console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`)
263-
if (shouldReportApiErrorToTelemetry(error?.code)) {
263+
if (shouldReportApiErrorToTelemetry(error?.code, error?.message)) {
264264
TelemetryService.instance.captureException(
265265
new ApiProviderError(
266266
error?.message ?? "Unknown error",
@@ -466,21 +466,20 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
466466
try {
467467
response = await this.client.chat.completions.create(completionParams, requestOptions)
468468
} catch (error) {
469-
TelemetryService.instance.captureException(
470-
new ApiProviderError(
471-
error instanceof Error ? error.message : String(error),
472-
this.providerName,
473-
modelId,
474-
"completePrompt",
475-
),
476-
{ provider: this.providerName, modelId, operation: "completePrompt" },
477-
)
469+
const errorMessage = error instanceof Error ? error.message : String(error)
470+
const errorStatus = getErrorStatusCode(error)
471+
if (shouldReportApiErrorToTelemetry(errorStatus, errorMessage)) {
472+
TelemetryService.instance.captureException(
473+
new ApiProviderError(errorMessage, this.providerName, modelId, "completePrompt", errorStatus),
474+
{ provider: this.providerName, modelId, operation: "completePrompt", errorCode: errorStatus },
475+
)
476+
}
478477
throw handleOpenAIError(error, this.providerName)
479478
}
480479

481480
if ("error" in response) {
482481
const error = response.error as { message?: string; code?: number }
483-
if (shouldReportApiErrorToTelemetry(error?.code)) {
482+
if (shouldReportApiErrorToTelemetry(error?.code, error?.message)) {
484483
TelemetryService.instance.captureException(
485484
new ApiProviderError(
486485
error?.message ?? "Unknown error",

0 commit comments

Comments
 (0)