From ad520dd1ddabd3188bf57bfd79d9e6a0d3241b59 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 23 Jun 2026 20:48:46 +0200 Subject: [PATCH] validate maxRetries --- src/client.ts | 17 +++++++---- src/internal/utils/values.ts | 10 +++++++ tests/index.test.ts | 56 ++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/client.ts b/src/client.ts index ac082a5e13..2afbde209f 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3,7 +3,12 @@ import type { RequestInit, RequestInfo, BodyInit } from './internal/builtin-types'; import type { HTTPMethod, PromiseOrValue, MergedRequestInit, FinalizedRequestInit } from './internal/types'; import { uuid4 } from './internal/utils/uuid'; -import { validatePositiveInteger, isAbsoluteURL, safeJSON } from './internal/utils/values'; +import { + validatePositiveInteger, + validateNonNegativeInteger, + isAbsoluteURL, + safeJSON, +} from './internal/utils/values'; import { sleep } from './internal/utils/sleep'; export type { Logger, LogLevel } from './internal/utils/log'; import { castToError, isAbortError } from './internal/errors'; @@ -453,7 +458,7 @@ export class OpenAI { parseLogLevel(readEnv('OPENAI_LOG'), "process.env['OPENAI_LOG']", this) ?? defaultLogLevel; this.fetchOptions = options.fetchOptions; - this.maxRetries = options.maxRetries ?? 2; + this.maxRetries = validateNonNegativeInteger('maxRetries', options.maxRetries ?? 2); this.fetch = options.fetch ?? Shims.getDefaultFetch(); this.#encoder = Opts.FallbackEncoder; @@ -706,7 +711,7 @@ export class OpenAI { retryOfRequestLogID: string | undefined, ): Promise { const options = await optionsInput; - const maxRetries = options.maxRetries ?? this.maxRetries; + const maxRetries = validateNonNegativeInteger('maxRetries', options.maxRetries ?? this.maxRetries); if (retriesRemaining == null) { retriesRemaining = maxRetries; } @@ -756,7 +761,7 @@ export class OpenAI { const isTimeout = isAbortError(response) || /timed? ?out/i.test(String(response) + ('cause' in response ? String(response.cause) : '')); - if (retriesRemaining) { + if (retriesRemaining > 0) { loggerFor(this).info( `[${requestLogID}] connection ${isTimeout ? 'timed out' : 'failed'} - ${retryMessage}`, ); @@ -828,7 +833,7 @@ export class OpenAI { } const shouldRetry = await this.shouldRetry(response); - if (retriesRemaining && shouldRetry) { + if (retriesRemaining > 0 && shouldRetry) { const retryMessage = `retrying, ${retriesRemaining} attempts remaining`; // We don't need the body of this response. @@ -1029,7 +1034,7 @@ export class OpenAI { // If the API asks us to wait a certain amount of time, just do what it // says, but otherwise calculate a default if (timeoutMillis === undefined) { - const maxRetries = options.maxRetries ?? this.maxRetries; + const maxRetries = validateNonNegativeInteger('maxRetries', options.maxRetries ?? this.maxRetries); timeoutMillis = this.calculateDefaultRetryTimeoutMillis(retriesRemaining, maxRetries); } await sleep(timeoutMillis); diff --git a/src/internal/utils/values.ts b/src/internal/utils/values.ts index 284ff5cdef..d6f64f6682 100644 --- a/src/internal/utils/values.ts +++ b/src/internal/utils/values.ts @@ -55,6 +55,16 @@ export const validatePositiveInteger = (name: string, n: unknown): number => { return n; }; +export const validateNonNegativeInteger = (name: string, n: unknown): number => { + if (typeof n !== 'number' || !Number.isInteger(n)) { + throw new OpenAIError(`${name} must be an integer`); + } + if (n < 0) { + throw new OpenAIError(`${name} must be a non-negative integer`); + } + return n; +}; + export const coerceInteger = (value: unknown): number => { if (typeof value === 'number') return Math.round(value); if (typeof value === 'string') return parseInt(value, 10); diff --git a/tests/index.test.ts b/tests/index.test.ts index 028eccb17a..b70eb5ad70 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -464,6 +464,22 @@ describe('instantiate client', () => { expect(client2.maxRetries).toEqual(2); }); + test.each([ + { maxRetries: -1, message: 'maxRetries must be a non-negative integer' }, + { maxRetries: 1.5, message: 'maxRetries must be an integer' }, + { maxRetries: Infinity, message: 'maxRetries must be an integer' }, + { maxRetries: NaN, message: 'maxRetries must be an integer' }, + ])('throws for invalid maxRetries option: $maxRetries', ({ maxRetries, message }) => { + expect( + () => + new OpenAI({ + maxRetries, + apiKey: 'My API Key', + adminAPIKey: 'My Admin API Key', + }), + ).toThrow(message); + }); + describe('withOptions', () => { test('creates a new client with overridden options', async () => { const client = new OpenAI({ @@ -653,6 +669,46 @@ describe('default encoder', () => { }); describe('retries', () => { + test('does not retry if maxRetries is zero', async () => { + let count = 0; + const testFetch = async (): Promise => { + count++; + return new Response(undefined, { status: 429 }); + }; + + const client = new OpenAI({ + apiKey: 'My API Key', + adminAPIKey: 'My Admin API Key', + fetch: testFetch, + maxRetries: 0, + }); + + await expect(client.request({ path: '/foo', method: 'get' })).rejects.toThrow(); + expect(count).toEqual(1); + }); + + test.each([ + { maxRetries: -1, message: 'maxRetries must be a non-negative integer' }, + { maxRetries: 1.5, message: 'maxRetries must be an integer' }, + { maxRetries: Infinity, message: 'maxRetries must be an integer' }, + { maxRetries: NaN, message: 'maxRetries must be an integer' }, + ])('throws for invalid request maxRetries: $maxRetries', async ({ maxRetries, message }) => { + let count = 0; + const testFetch = async (): Promise => { + count++; + return new Response(JSON.stringify({ a: 1 }), { headers: { 'Content-Type': 'application/json' } }); + }; + + const client = new OpenAI({ + apiKey: 'My API Key', + adminAPIKey: 'My Admin API Key', + fetch: testFetch, + }); + + await expect(client.request({ path: '/foo', method: 'get', maxRetries })).rejects.toThrow(message); + expect(count).toEqual(0); + }); + test('retry on timeout', async () => { let count = 0; const testFetch = async (