diff --git a/README.md b/README.md index 34b08b45d..0224c0714 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,12 @@ await workos.get('/organizations', { maxRetries: 0 }); Set `maxRetries: 0` to disable automatic retries entirely. +The request timeout (`timeout`, 60 seconds by default) covers each attempt from +the request being sent to the response body being fully read, so a server that +returns headers promptly but stalls the body still times out. A timeout while +reading a successful response body is reported as a 408 but is not +automatically retried, because the server has already applied the request. + ### Access token issuer validation Session helpers (`authenticateWithSessionCookie`, `loadSealedSession(...).authenticate()`) diff --git a/setup-jest.ts b/setup-jest.ts index 019680cb6..95a06c3d2 100644 --- a/setup-jest.ts +++ b/setup-jest.ts @@ -1,6 +1,13 @@ -import { enableFetchMocks } from 'jest-fetch-mock'; import { webcrypto } from 'crypto'; +// jest-fetch-mock replaces the global fetch the moment it is loaded. Keep the +// runtime's own implementation reachable for tests that need real HTTP +// streaming and abort behaviour (see fetch-client.spec.ts). +const NATIVE_FETCH_KEY = '__workosNativeFetch'; +(globalThis as Record)[NATIVE_FETCH_KEY] = globalThis.fetch; + +const { enableFetchMocks } = require('jest-fetch-mock'); + enableFetchMocks(); // Make Node's crypto.webcrypto available as global.crypto for tests diff --git a/src/common/net/fetch-client.spec.ts b/src/common/net/fetch-client.spec.ts index d318f26a2..3e94962d4 100644 --- a/src/common/net/fetch-client.spec.ts +++ b/src/common/net/fetch-client.spec.ts @@ -3,6 +3,8 @@ import { fetchOnce, fetchURL } from '../../common/utils/test-utils'; import { FetchHttpClient } from './fetch-client'; import { HttpClientError } from './http-client'; import { ParseError } from '../exceptions/parse-error'; +import http from 'node:http'; +import { AddressInfo } from 'node:net'; const fetchClient = new FetchHttpClient('https://test.workos.com', { headers: { @@ -658,3 +660,553 @@ describe('FetchHttpClient with timeout', () => { expect(result).toBeDefined(); }); }); + +describe('request timeout covers the response body (GH-1679)', () => { + // `fetch` in this file is the jest-fetch-mock import. + type FetchFn = typeof globalThis.fetch; + + type FakeAttempt = { + signal: AbortSignal; + response: any; + failBody: (error: Error) => void; + readonly textCalls: number; + }; + + type FakeAttemptPlan = { + status?: number; + headers?: Record; + /** `'pending'` to stall until the signal aborts, else the body. */ + body?: 'pending' | string; + /** Delay before the headers resolve. */ + headersDelayMs?: number; + }; + + const abortError = () => { + const error = new Error('The operation was aborted'); + error.name = 'AbortError'; + return error; + }; + + /** + * A fetch double that honours the abort signal the way real + * implementations do: an abort rejects the pending headers or the pending + * body read with an `AbortError`. + */ + function createFakeFetch(plan: FakeAttemptPlan[]) { + const attempts: FakeAttempt[] = []; + + const fetchFn = jest.fn((_url: string, init: RequestInit) => { + const step = plan[Math.min(attempts.length, plan.length - 1)]; + const signal = init.signal as AbortSignal; + const status = step.status ?? 200; + const headers = new Headers({ + 'content-type': 'application/json', + 'x-request-id': 'req_1679', + ...step.headers, + }); + + let settleBody!: { + resolve: (body: string) => void; + reject: (error: Error) => void; + }; + const bodyPromise = new Promise((resolve, reject) => { + settleBody = { resolve, reject }; + }); + bodyPromise.catch(() => undefined); + signal.addEventListener('abort', () => settleBody.reject(abortError())); + + let textCalls = 0; + const response = { + ok: status < 400, + status, + statusText: status < 400 ? 'OK' : 'Error', + headers, + text: () => { + textCalls++; + return bodyPromise; + }, + }; + + if (step.body !== 'pending') { + settleBody.resolve(step.body ?? ''); + } + + attempts.push({ + signal, + response, + failBody: settleBody.reject, + get textCalls() { + return textCalls; + }, + }); + + return new Promise((resolve, reject) => { + signal.addEventListener('abort', () => reject(abortError())); + if (step.headersDelayMs) { + setTimeout(() => resolve(response), step.headersDelayMs); + } else { + resolve(response); + } + }); + }); + + return { fetchFn: fetchFn as unknown as FetchFn, attempts }; + } + + function createClient( + plan: FakeAttemptPlan[], + options: { maxRetries?: number; timeout?: number } = {}, + ) { + const fake = createFakeFetch(plan); + const client = new FetchHttpClient( + 'https://api.example.com', + { timeout: 100, maxRetries: 0, ...options }, + fake.fetchFn, + ); + return { client, ...fake }; + } + + function settledFlag(promise: Promise) { + const state = { settled: false }; + promise.then( + () => (state.settled = true), + () => (state.settled = true), + ); + return state; + } + + const timeout408 = { + message: 'Request timeout after 100ms', + response: { status: 408, data: { error: 'Request timeout' } }, + }; + + describe('with deterministic timers', () => { + beforeEach(() => jest.useFakeTimers()); + afterEach(() => jest.useRealTimers()); + + it('keeps the deadline armed after the headers arrive and fails a stalled JSON body with a 408', async () => { + const { client, attempts } = createClient([{ body: 'pending' }]); + + const res = await client.get('/users', {}); + const read = res.toJSON(); + read.catch(() => undefined); + const state = settledFlag(read); + + await jest.advanceTimersByTimeAsync(99); + expect(state.settled).toBe(false); + expect(attempts[0].signal.aborted).toBe(false); + + await jest.advanceTimersByTimeAsync(1); + await expect(read).rejects.toThrow(HttpClientError); + await expect(read).rejects.toMatchObject(timeout408); + const error = await read.catch((e) => e); + expect(error.response.headers.get('x-request-id')).toBe('req_1679'); + expect(attempts[0].signal.aborted).toBe(true); + expect(attempts[0].textCalls).toBe(1); + expect(jest.getTimerCount()).toBe(0); + }); + + it('charges the wait for headers against the same deadline instead of restarting it', async () => { + const { client } = createClient([ + { body: 'pending', headersDelayMs: 80 }, + ]); + + const request = client.get('/users', {}); + await jest.advanceTimersByTimeAsync(80); + const res = await request; + + const read = res.toJSON(); + read.catch(() => undefined); + const state = settledFlag(read); + + await jest.advanceTimersByTimeAsync(19); + expect(state.settled).toBe(false); + + await jest.advanceTimersByTimeAsync(1); + await expect(read).rejects.toMatchObject(timeout408); + }); + + it('does not retry a request whose successful body times out: the server has already applied it', async () => { + const { client, fetchFn, attempts } = createClient( + [{ body: 'pending' }], + { maxRetries: 2 }, + ); + + const res = await client.patch('/users/123', { name: 'x' }, {}); + const read = res.toJSON(); + read.catch(() => undefined); + + await jest.advanceTimersByTimeAsync(100); + await expect(read).rejects.toMatchObject(timeout408); + + await jest.advanceTimersByTimeAsync(20_000); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(attempts).toHaveLength(1); + }); + + it('reads a stalled error body inside the attempt so Retry-After applies, with a fresh deadline per attempt', async () => { + const { client, fetchFn, attempts } = createClient( + [ + { + status: 503, + headers: { 'x-request-id': 'req_first', 'retry-after': '1' }, + body: 'pending', + }, + { status: 200, body: '{"ok":true}' }, + ], + { maxRetries: 1 }, + ); + + const request = client.post('/users', { name: 'x' }, {}); + request.catch(() => undefined); + + await jest.advanceTimersByTimeAsync(100); + expect(attempts[0].signal.aborted).toBe(true); + expect(attempts[0].textCalls).toBe(1); + expect(fetchFn).toHaveBeenCalledTimes(1); + + // Retry-After from the aborted attempt's headers is honoured. + await jest.advanceTimersByTimeAsync(999); + expect(fetchFn).toHaveBeenCalledTimes(1); + await jest.advanceTimersByTimeAsync(1); + expect(fetchFn).toHaveBeenCalledTimes(2); + + const res = await request; + await expect(res.toJSON()).resolves.toEqual({ ok: true }); + + const calls = (fetchFn as unknown as jest.Mock).mock.calls; + expect(calls[0][1].headers['Idempotency-Key']).toBe( + calls[1][1].headers['Idempotency-Key'], + ); + + await jest.advanceTimersByTimeAsync(10_000); + expect(attempts[1].signal.aborted).toBe(false); + expect(jest.getTimerCount()).toBe(0); + }); + + it('surfaces a stalled error body as a 408 carrying the response headers once retries are exhausted', async () => { + const { client, attempts } = createClient([ + { + status: 422, + headers: { 'x-request-id': 'req_422' }, + body: 'pending', + }, + ]); + + const request = client.get('/users', {}); + request.catch(() => undefined); + await jest.advanceTimersByTimeAsync(100); + + await expect(request).rejects.toMatchObject(timeout408); + const error = await request.catch((e) => e); + expect(error.response.headers.get('x-request-id')).toBe('req_422'); + expect(attempts[0].signal.aborted).toBe(true); + }); + + it('keeps a timeout before the headers on its existing path, including retries', async () => { + const { client, fetchFn, attempts } = createClient( + [{ body: 'pending', headersDelayMs: 500 }], + { maxRetries: 1 }, + ); + + const request = client.get('/users', {}); + request.catch(() => undefined); + + await jest.advanceTimersByTimeAsync(100); + expect(attempts[0].signal.aborted).toBe(true); + expect(fetchFn).toHaveBeenCalledTimes(1); + + // Backoff (max 1687.5ms at attempt 2) then the second attempt's deadline. + await jest.advanceTimersByTimeAsync(2000); + expect(fetchFn).toHaveBeenCalledTimes(2); + expect(attempts[1].signal.aborted).toBe(true); + + await expect(request).rejects.toMatchObject(timeout408); + const error = await request.catch((e) => e); + expect(error.response.headers.get('x-request-id')).toBeNull(); + }); + + it('still reports a complete but malformed JSON body as a ParseError and releases the deadline', async () => { + const { client, attempts } = createClient([{ body: '{ invalid' }]); + + const res = await client.get('/users', {}); + const error = await res.toJSON().catch((e) => e); + + expect(error).toBeInstanceOf(ParseError); + expect(error.rawBody).toBe('{ invalid'); + expect(error.rawStatus).toBe(200); + expect(error.requestID).toBe('req_1679'); + expect(jest.getTimerCount()).toBe(0); + + await jest.advanceTimersByTimeAsync(10_000); + expect(attempts[0].signal.aborted).toBe(false); + }); + + it('propagates a body failure that is not a timeout unchanged and does not retry it', async () => { + const { client, fetchFn, attempts } = createClient( + [{ body: 'pending' }], + { + maxRetries: 2, + }, + ); + + const res = await client.get('/users', {}); + const read = res.toJSON(); + read.catch(() => undefined); + attempts[0].failBody(new TypeError('terminated')); + + await expect(read).rejects.toThrow(TypeError); + await expect(read).rejects.toThrow('terminated'); + await expect(read).rejects.not.toBeInstanceOf(HttpClientError); + expect(jest.getTimerCount()).toBe(0); + + await jest.advanceTimersByTimeAsync(10_000); + expect(fetchFn).toHaveBeenCalledTimes(1); + expect(attempts[0].signal.aborted).toBe(false); + }); + + it('delivers a body that completes in time, releases the deadline and keeps the original response reachable', async () => { + const { client, attempts } = createClient([{ body: '{"ok":true}' }]); + + const res = await client.get('/users', {}); + await expect(res.toJSON()).resolves.toEqual({ ok: true }); + expect(jest.getTimerCount()).toBe(0); + expect(res.getRawResponse()).toBe(attempts[0].response); + + await jest.advanceTimersByTimeAsync(10_000); + expect(attempts[0].signal.aborted).toBe(false); + }); + + it('drains a non-JSON body inside the deadline and still reports it as null', async () => { + const { client, attempts } = createClient([ + { headers: { 'content-type': 'text/plain' }, body: 'hello' }, + ]); + + const res = await client.get('/users', {}); + await expect(res.toJSON()).resolves.toBeNull(); + expect(attempts[0].textCalls).toBe(1); + expect(jest.getTimerCount()).toBe(0); + }); + + it('fails toJSON() with a 408 for a stalled non-JSON body instead of resolving to null early', async () => { + const { client, attempts } = createClient([ + { headers: { 'content-type': 'text/plain' }, body: 'pending' }, + ]); + + const res = await client.get('/users', {}); + const read = res.toJSON(); + read.catch(() => undefined); + const state = settledFlag(read); + + await jest.advanceTimersByTimeAsync(99); + expect(state.settled).toBe(false); + + await jest.advanceTimersByTimeAsync(1); + await expect(read).rejects.toMatchObject(timeout408); + expect(attempts[0].signal.aborted).toBe(true); + }); + + it('bounds a stalled body that nobody reads without failing the call', async () => { + const { client, attempts } = createClient([{ body: 'pending' }]); + + await client.delete('/users/123', {}); + expect(attempts[0].textCalls).toBe(1); + expect(jest.getTimerCount()).toBe(1); + + await jest.advanceTimersByTimeAsync(100); + expect(attempts[0].signal.aborted).toBe(true); + expect(jest.getTimerCount()).toBe(0); + }); + }); + + // Captured in setup-jest.ts before jest-fetch-mock replaces the global. + const nativeFetch = (globalThis as Record) + .__workosNativeFetch as FetchFn; + // jest-fetch-mock registers a mock for the 'node-fetch' module id. + const nodeFetch = jest.requireActual('node-fetch') as FetchFn; + + describe.each([ + ['native fetch', nativeFetch], + ['node-fetch', nodeFetch], + ])('over real HTTP with %s', (_name, fetchImpl) => { + let server: http.Server; + let baseURL: string; + let handler: (req: http.IncomingMessage, res: http.ServerResponse) => void; + let requestCount: number; + const openResponses: http.ServerResponse[] = []; + + const sleep = (ms: number) => + new Promise((resolve) => setTimeout(resolve, ms)); + + function stallBody(status: number, head = '{"ok":') { + handler = (_req, res) => { + res.writeHead(status, { + 'content-type': 'application/json', + 'x-request-id': 'req_real', + 'retry-after': '0', + }); + res.write(head); + // Hold the body open until the test tears the connection down. + }; + } + + function respond(status: number, body: string) { + handler = (_req, res) => { + res.writeHead(status, { + 'content-type': 'application/json', + 'x-request-id': 'req_real', + 'retry-after': '0', + }); + res.end(body); + }; + } + + function createClient(maxRetries = 0) { + const signals: AbortSignal[] = []; + const rawResponses: Response[] = []; + const fetchFn: FetchFn = async (url, init) => { + signals.push(init!.signal as AbortSignal); + const response = await fetchImpl(url, init); + rawResponses.push(response); + return response; + }; + const client = new FetchHttpClient( + baseURL, + { timeout: 100, maxRetries }, + fetchFn, + ); + return { client, signals, rawResponses }; + } + + beforeAll(async () => { + server = http.createServer((req, res) => { + requestCount++; + openResponses.push(res); + handler(req, res); + }); + await new Promise((resolve) => + server.listen(0, '127.0.0.1', resolve), + ); + const { port } = server.address() as AddressInfo; + baseURL = `http://127.0.0.1:${port}`; + }); + + afterAll(async () => { + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + }); + + beforeEach(() => { + requestCount = 0; + }); + + afterEach(() => { + for (const res of openResponses) { + res.destroy(); + } + openResponses.length = 0; + }); + + it('times out a successful JSON body that stalls after the headers', async () => { + stallBody(200); + const { client, signals } = createClient(); + + const res = await client.get('/users', {}); + expect(res.getStatusCode()).toBe(200); + + const error = await res.toJSON().catch((e) => e); + expect(error).toBeInstanceOf(HttpClientError); + expect(error).toMatchObject(timeout408); + expect(error.response.headers.get('x-request-id')).toBe('req_real'); + expect(signals[0].aborted).toBe(true); + expect(requestCount).toBe(1); + }); + + it('does not retry a successful body that times out, even when retries are enabled', async () => { + stallBody(200); + const { client } = createClient(2); + + const res = await client.patch('/users/123', { name: 'x' }, {}); + await expect(res.toJSON()).rejects.toMatchObject(timeout408); + + // Retry-After is 0, so a retry would already have been sent. + await sleep(50); + expect(requestCount).toBe(1); + }); + + it('times out a stalled error body inside the attempt and retries it with a fresh deadline', async () => { + // First attempt stalls its 503 body; the retry gets a clean 200. + handler = (_req, res) => { + if (requestCount === 1) { + res.writeHead(503, { + 'content-type': 'application/json', + 'retry-after': '0', + }); + res.write('{"error":'); + return; + } + res.writeHead(200, { 'content-type': 'application/json' }); + res.end('{"ok":true}'); + }; + const { client, signals } = createClient(1); + + const res = await client.get('/users', {}); + await expect(res.toJSON()).resolves.toEqual({ ok: true }); + expect(requestCount).toBe(2); + expect(signals[0].aborted).toBe(true); + expect(signals[1].aborted).toBe(false); + }); + + it('surfaces a stalled error body as a 408 carrying the response headers when not retried', async () => { + stallBody(422); + const { client, signals } = createClient(); + + const error = await client.get('/users', {}).catch((e) => e); + expect(error).toBeInstanceOf(HttpClientError); + expect(error).toMatchObject(timeout408); + expect(error.response.headers.get('x-request-id')).toBe('req_real'); + expect(signals[0].aborted).toBe(true); + }); + + it('delivers a body that completes in time and disarms the deadline', async () => { + respond(200, '{"ok":true}'); + const { client, signals } = createClient(); + + const res = await client.get('/users', {}); + await expect(res.toJSON()).resolves.toEqual({ ok: true }); + + await sleep(150); + expect(signals[0].aborted).toBe(false); + }); + + it('reports a connection dropped mid-body before the deadline as its own error', async () => { + handler = (_req, res) => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.write('{"ok":'); + setTimeout(() => res.destroy(), 10); + }; + const { client } = createClient(); + + const res = await client.get('/users', {}); + const error = await res.toJSON().catch((e) => e); + // The implementation's own error (undici raises it in Jest's outer + // realm, so no instanceof Error here), not a timeout or parse error. + expect(typeof error.message).toBe('string'); + expect(error).not.toBeInstanceOf(HttpClientError); + expect(error).not.toBeInstanceOf(ParseError); + expect(error.name).not.toBe('AbortError'); + }); + + it("hands back the implementation's own response, with the body read by the SDK", async () => { + respond(200, '{}'); + const { client, rawResponses, signals } = createClient(); + + const res = await client.delete('/users/123', {}); + expect(res.getRawResponse()).toBe(rawResponses[0]); + expect(rawResponses[0].bodyUsed).toBe(true); + + await sleep(150); + expect(signals[0].aborted).toBe(false); + }); + }); +}); diff --git a/src/common/net/fetch-client.ts b/src/common/net/fetch-client.ts index 42586b382..1b76c4f41 100644 --- a/src/common/net/fetch-client.ts +++ b/src/common/net/fetch-client.ts @@ -191,11 +191,24 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { const timeout = this.options?.timeout ?? DEFAULT_FETCH_TIMEOUT; // Default 60 seconds const abortController = new AbortController(); const timeoutId = setTimeout(() => { - abortController?.abort(); + abortController.abort(); }, timeout); + // Pass the response headers once they are known so a timeout while + // reading the body still carries the request ID and Retry-After. + const timeoutError = (responseHeaders?: Headers) => + new HttpClientError({ + message: `Request timeout after ${timeout}ms`, + response: { + status: 408, + headers: responseHeaders ?? new Headers(), + data: { error: 'Request timeout' }, + }, + }); + // Set once the headers arrive. + let res: Response | undefined; try { - const res = await this._fetchFn(url, { + res = await this._fetchFn(url, { method, headers: { Accept: 'application/json, text/plain, */*', @@ -205,17 +218,15 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { 'User-Agent': (userAgent || 'workos-node').toString(), }, body: requestBody, - signal: abortController?.signal, + signal: abortController.signal, }); - // Clear timeout if request completed successfully - if (timeoutId) { - clearTimeout(timeoutId); - } - if (!res.ok) { const requestID = res.headers.get('X-Request-ID') ?? ''; + // Read the error body under the same deadline, inside the attempt, + // so a stalled error response is retried like any other timeout. const rawBody = await res.text(); + clearTimeout(timeoutId); let responseJson: any; @@ -243,23 +254,37 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { }, }); } - return new FetchHttpClientResponse(res); + + // The deadline also covers a successful body (GH-1679), but the server + // has already applied this request, so the read happens outside the + // retry boundary: a stall surfaces from toJSON() as a 408 and is not + // retried. The body is read regardless of whether anyone awaits it, so + // an ignored response is drained and its deadline cleared. + const response = res; + const rawBody = (async () => { + try { + return await response.text(); + } catch (error) { + // Checked on our own signal rather than the error's type: the + // AbortError a fetch implementation raises for an aborted body may + // come from another realm. + throw abortController.signal.aborted + ? timeoutError(response.headers) + : error; + } finally { + clearTimeout(timeoutId); + } + })(); + rawBody.catch(() => undefined); + + return new FetchHttpClientResponse(res, rawBody); } catch (error) { // Clear timeout if request failed - if (timeoutId) { - clearTimeout(timeoutId); - } + clearTimeout(timeoutId); // Handle timeout errors - if (error instanceof Error && error.name === 'AbortError') { - throw new HttpClientError({ - message: `Request timeout after ${timeout}ms`, - response: { - status: 408, - headers: new Headers(), - data: { error: 'Request timeout' }, - }, - }); + if (abortController.signal.aborted) { + throw timeoutError(res?.headers); } throw error; @@ -414,13 +439,15 @@ export class FetchHttpClientResponse implements HttpClientResponseInterface { _res: Response; + private readonly _rawBody: Promise; - constructor(res: Response) { + constructor(res: Response, rawBody: Promise) { super( res.status, FetchHttpClientResponse._transformHeadersToObject(res.headers), ); this._res = res; + this._rawBody = rawBody; } getRawResponse(): Response { @@ -428,6 +455,10 @@ export class FetchHttpClientResponse } async toJSON(): Promise { + // Awaited before the content-type check so a stalled non-JSON body still + // surfaces its timeout instead of resolving to null early. + const rawBody = await this._rawBody; + const contentType = this._res.headers.get('content-type'); const isJsonResponse = contentType?.includes('application/json'); @@ -435,8 +466,6 @@ export class FetchHttpClientResponse return null; } - const rawBody = await this._res.text(); - try { return JSON.parse(rawBody); } catch (error) { diff --git a/src/workos.spec.ts b/src/workos.spec.ts index 708864185..2e61a1107 100644 --- a/src/workos.spec.ts +++ b/src/workos.spec.ts @@ -665,6 +665,103 @@ describe('WorkOS', () => { }); }); + describe('when the request times out while the response body is streaming (GH-1679)', () => { + const abortError = () => { + const error = new Error('Aborted'); + error.name = 'AbortError'; + return error; + }; + + /** + * Resolves headers immediately; the body read stalls until the abort + * signal fires, as real Fetch implementations behave. + */ + function stalledBodyFetch({ text }: { text?: () => Promise } = {}) { + const signals: AbortSignal[] = []; + const fetchFn = jest.fn(async (_url: any, init: any) => { + const signal = init.signal as AbortSignal; + signals.push(signal); + return { + ok: true, + status: 200, + headers: new Headers({ + 'X-Request-ID': 'req_body', + 'content-type': 'application/json', + }), + text: + text ?? + (() => + new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(abortError())); + })), + }; + }); + return { fetchFn, signals }; + } + + it.each([ + ['get', (workos: WorkOS) => workos.get('/path')], + ['post', (workos: WorkOS) => workos.post('/path', {})], + ['put', (workos: WorkOS) => workos.put('/path', {})], + ['patch', (workos: WorkOS) => workos.patch('/path', {})], + ])( + '%s surfaces the same 408 OauthException as a timeout before the headers, without retrying', + async (_name, call) => { + const { fetchFn, signals } = stalledBodyFetch(); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + timeout: 20, + fetchFn: fetchFn as any, + }); + + await expect(call(workos)).rejects.toMatchObject({ + name: 'OauthException', + status: 408, + requestID: 'req_body', + message: 'Error: Request timeout', + }); + expect(signals[0].aborted).toBe(true); + expect(fetchFn).toHaveBeenCalledTimes(1); + }, + ); + + it('propagates a body read failure that is not a timeout unchanged', async () => { + const transportError = new TypeError('terminated'); + const { fetchFn } = stalledBodyFetch({ + text: () => Promise.reject(transportError), + }); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + timeout: 20, + fetchFn: fetchFn as any, + }); + + await expect(workos.post('/path', {})).rejects.toBe(transportError); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['delete', (workos: WorkOS) => workos.delete('/path')], + [ + 'deleteWithBody', + (workos: WorkOS) => workos.deleteWithBody('/path', { id: 'x' }), + ], + ])( + '%s resolves once the response arrives, without requiring a body, and leaves a stalled body bounded by the deadline', + async (_name, call) => { + const { fetchFn, signals } = stalledBodyFetch(); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + timeout: 20, + fetchFn: fetchFn as any, + }); + + await expect(call(workos)).resolves.toBeUndefined(); + expect(signals[0].aborted).toBe(false); + + await new Promise((resolve) => setTimeout(resolve, 60)); + expect(signals[0].aborted).toBe(true); + }, + ); + }); + describe('when in a worker environment', () => { it('uses the worker client', () => { const workos = new WorkOSWorker('sk_test_key'); diff --git a/src/workos.ts b/src/workos.ts index 7f0d568df..72154aada 100644 --- a/src/workos.ts +++ b/src/workos.ts @@ -257,7 +257,7 @@ export class WorkOS { throw error; } - return { data: await res.toJSON() }; + return { data: await this.readResponseJSON(path, res) }; } async get( @@ -291,7 +291,7 @@ export class WorkOS { throw error; } - return { data: await res.toJSON() }; + return { data: await this.readResponseJSON(path, res) }; } async put( @@ -323,7 +323,7 @@ export class WorkOS { throw error; } - return { data: await res.toJSON() }; + return { data: await this.readResponseJSON(path, res) }; } async patch( @@ -355,7 +355,7 @@ export class WorkOS { throw error; } - return { data: await res.toJSON() }; + return { data: await this.readResponseJSON(path, res) }; } async delete( @@ -390,6 +390,28 @@ export class WorkOS { } } + /** + * Consume a successful response body. The request deadline covers the body, + * so a stall while it is still streaming surfaces from the transport as a + * 408 `HttpClientError` and is translated like a timeout before the headers. + * Everything else (a `ParseError` for malformed JSON, a network failure + * mid-body) propagates unchanged. + */ + private async readResponseJSON( + path: string, + response: HttpClientResponseInterface, + ): Promise { + try { + return await response.toJSON(); + } catch (error) { + if (error instanceof HttpClientError) { + this.handleHttpError({ path, error }); + } + + throw error; + } + } + emitWarning(warning: string) { // tslint:disable-next-line:no-console console.warn(`WorkOS: ${warning}`);