diff --git a/src/client.ts b/src/client.ts index f55a6b986..5851f58de 100644 --- a/src/client.ts +++ b/src/client.ts @@ -243,6 +243,62 @@ import { isEmptyObj } from './internal/utils/values'; const WORKLOAD_IDENTITY_API_KEY_PLACEHOLDER = 'workload-identity-auth'; +/** + * Wrap a `Response` so that the given `clearRequestTimeout` callback runs + * exactly once, when the body is either fully read, cancelled, or errors. + * Responses without a body have the timeout cleared synchronously. + * + * This lets the request-timeout timer stay armed across the body-read phase + * (e.g. `await response.json()`), which native `await fetch()` does not cover. + */ +function wrapResponseForRequestTimeout(response: Response, clearRequestTimeout: () => void): Response { + if (!response.body) { + clearRequestTimeout(); + return response; + } + + let cleared = false; + const clearOnce = () => { + if (cleared) return; + cleared = true; + clearRequestTimeout(); + }; + + const originalBody = response.body; + const wrappedBody = new ReadableStream({ + async start(controller) { + const reader = originalBody.getReader(); + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + controller.enqueue(value); + } + controller.close(); + } catch (err) { + controller.error(err); + } finally { + clearOnce(); + try { + reader.releaseLock(); + } catch { + // reader may already be released + } + } + }, + cancel(reason) { + clearOnce(); + return originalBody.cancel(reason); + }, + }); + + return new Response(wrappedBody, { + status: response.status, + statusText: response.statusText, + headers: response.headers, + }); +} + export type ApiKeySetter = () => Promise; export interface ClientOptions { @@ -887,12 +943,22 @@ export class OpenAI { fetchOptions.method = method.toUpperCase(); } + let response: Response; try { // use undefined this binding; fetch errors if bound to something else in browser/cloudflare - return await this.fetch.call(undefined, url, fetchOptions); - } finally { + response = await this.fetch.call(undefined, url, fetchOptions); + } catch (err) { clearTimeout(timeout); + throw err; } + + // Keep the timer armed until the body is fully read, cancelled, or errored. + // The `await fetch()` above resolves as soon as response *headers* arrive, so + // clearing the timeout here would leave subsequent body readers — e.g. + // `await response.json()` in `internal/parse.ts` — without any timeout. + // Servers that flush 200 headers fast and then stall mid-body would cause + // the SDK to hang indefinitely. See openai/openai-node#1825. + return wrapResponseForRequestTimeout(response, () => clearTimeout(timeout)); } private async shouldRetry(response: Response): Promise { diff --git a/tests/index.test.ts b/tests/index.test.ts index f15b518e9..8a6abfaa9 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -544,6 +544,56 @@ describe('default encoder', () => { }); }); +describe('timeout covers body read phase', () => { + // Regression for https://github.com/openai/openai-node/issues/1825 + // The SDK must abort the request if the server sends headers fast but + // stalls while streaming the body. Previously the internal timer was + // cleared as soon as `await fetch()` resolved (at headers arrival), + // leaving `response.json()` unguarded and able to hang forever. + test('rejects when the body read stalls past the configured timeout', async () => { + const CONFIGURED_TIMEOUT_MS = 50; + const BODY_STALL_MS = 2000; + + const testFetch = async ( + _url: string | URL | Request, + { signal }: RequestInit = {}, + ): Promise => { + // Server-style body: headers are returned instantly, but the body + // stream never produces any chunks. We honour the AbortSignal so the + // SDK's retry/timeout path can still short-circuit it. + const body = new ReadableStream({ + start(controller) { + signal?.addEventListener('abort', () => controller.error(new Error('aborted')), { once: true }); + // Safety net so the test never actually hangs longer than + // BODY_STALL_MS, regardless of whether the fix is in place. + setTimeout(() => { + controller.close(); + }, BODY_STALL_MS).unref?.(); + }, + }); + + return new Response(body, { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + + const client = new OpenAI({ + apiKey: 'My API Key', + timeout: CONFIGURED_TIMEOUT_MS, + maxRetries: 0, + fetch: testFetch, + }); + + const started = Date.now(); + await expect(client.request({ path: '/foo', method: 'get' })).rejects.toThrow(); + const elapsed = Date.now() - started; + + // Must reject well before the body stall would naturally complete. + expect(elapsed).toBeLessThan(BODY_STALL_MS); + }); +}); + describe('retries', () => { test('retry on timeout', async () => { let count = 0;