From d7b725dc252c42f100d17a67766faf5ab7abf96e Mon Sep 17 00:00:00 2001 From: Stanley Phu Date: Wed, 4 Nov 2026 22:45:27 -0800 Subject: [PATCH 1/2] fix(feature-flags): Report poll timeouts and cancel abandoned requests A poll that hit requestTimeoutMs emitted 'error' with "Poll aborted" instead of the timeout: the timeout aborted the poll's controller before rejecting, and the abort listener's rejection won the race. The poll also never handed its abort signal to the HTTP request, so a timed-out poll, or one in flight at close(), left its request open until the HTTP client's own 60-second timeout. That timeout surfaces as a 408, which the client retries, so abandoned requests were re-sent after a minute, including after close(). The timeout now aborts the poll with a timeout error as the reason, and the poll passes its signal to the request. To carry it, workos.get and the HTTP client accept a `signal` option: aborting cancels the request, rejects with the signal's reason, and is never retried. Co-Authored-By: Claude Opus 5.5 --- .../interfaces/get-options.interface.ts | 2 + .../interfaces/http-client.interface.ts | 2 + src/common/net/fetch-client.spec.ts | 45 ++++++++++++++ src/common/net/fetch-client.ts | 60 +++++++++++++------ src/feature-flags/runtime-client.spec.ts | 47 +++++++++++++++ src/feature-flags/runtime-client.ts | 35 ++++------- src/workos.spec.ts | 26 ++++++++ src/workos.ts | 2 + 8 files changed, 180 insertions(+), 39 deletions(-) diff --git a/src/common/interfaces/get-options.interface.ts b/src/common/interfaces/get-options.interface.ts index 494fba798..3212565d6 100644 --- a/src/common/interfaces/get-options.interface.ts +++ b/src/common/interfaces/get-options.interface.ts @@ -6,4 +6,6 @@ export interface GetOptions { skipApiKeyCheck?: boolean; /** Maximum number of retries for this request, overriding the client-wide `maxRetries`. */ maxRetries?: number; + /** Aborts the request, rejecting with the signal's reason. An aborted request is not retried. */ + signal?: AbortSignal; } diff --git a/src/common/interfaces/http-client.interface.ts b/src/common/interfaces/http-client.interface.ts index be6d7decd..ff1a8302e 100644 --- a/src/common/interfaces/http-client.interface.ts +++ b/src/common/interfaces/http-client.interface.ts @@ -7,6 +7,8 @@ export type RequestOptions = { * `maxRetries`. Set to `0` to disable retries for this request. */ maxRetries?: number; + /** Aborts the request. An aborted request is not retried. */ + signal?: AbortSignal; }; export type ResponseHeaderValue = string | string[]; export type ResponseHeaders = Record; diff --git a/src/common/net/fetch-client.spec.ts b/src/common/net/fetch-client.spec.ts index a4c283483..6becd994c 100644 --- a/src/common/net/fetch-client.spec.ts +++ b/src/common/net/fetch-client.spec.ts @@ -661,6 +661,51 @@ describe('FetchHttpClient with timeout', () => { }); }); +describe('FetchHttpClient with an abort signal', () => { + beforeEach(() => fetch.resetMocks()); + + it('aborts the request in flight and fails with the signal reason instead of retrying a timeout', async () => { + const mockFetch = jest.fn(); + mockFetch.mockImplementation( + (_, options) => + new Promise((_, reject) => { + options.signal.addEventListener('abort', () => { + const error = new Error('AbortError'); + error.name = 'AbortError'; + reject(error); + }); + }), + ); + const client = new FetchHttpClient( + 'https://api.example.com', + {}, + mockFetch, + ); + const controller = new AbortController(); + const reason = new Error('Stop'); + + const request = client.get('/users', { signal: controller.signal }); + controller.abort(reason); + + await expect(request).rejects.toBe(reason); + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(mockFetch.mock.calls[0][1].signal.aborted).toBe(true); + }); + + it('sends no further attempt once the signal aborts between retries', async () => { + fetchOnce({}, { status: 500 }); + const controller = new AbortController(); + const reason = new Error('Stop'); + const mockSleep = jest.spyOn(fetchClient, 'sleep'); + mockSleep.mockImplementation(async () => controller.abort(reason)); + + await expect( + fetchClient.get('/organizations', { signal: controller.signal }), + ).rejects.toBe(reason); + expect(fetch.mock.calls.length).toBe(1); + }); +}); + describe('request timeout covers the response body (GH-1679)', () => { // `fetch` in this file is the jest-fetch-mock import. type FetchFn = typeof globalThis.fetch; diff --git a/src/common/net/fetch-client.ts b/src/common/net/fetch-client.ts index 1b76c4f41..774c782aa 100644 --- a/src/common/net/fetch-client.ts +++ b/src/common/net/fetch-client.ts @@ -56,6 +56,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { null, options.headers, options.maxRetries, + options.signal, ); } @@ -79,6 +80,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { ...options.headers, }, options.maxRetries, + options.signal, ); } @@ -102,6 +104,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { ...options.headers, }, options.maxRetries, + options.signal, ); } @@ -125,6 +128,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { ...options.headers, }, options.maxRetries, + options.signal, ); } @@ -144,6 +148,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { null, options.headers, options.maxRetries, + options.signal, ); } @@ -167,6 +172,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { ...options.headers, }, options.maxRetries, + options.signal, ); } @@ -175,7 +181,10 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { method: string, body?: any, headers?: RequestHeaders, + signal?: AbortSignal, ): Promise { + signal?.throwIfAborted(); + // For methods which expect payloads, we should always pass a body value // even when it is empty. Without this, some JS runtimes (eg. Deno) will // inject a second Content-Length header. @@ -193,17 +202,26 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { const timeoutId = setTimeout(() => { abortController.abort(); }, timeout); + const abortRequest = () => abortController.abort(); + signal?.addEventListener('abort', abortRequest, { once: true }); + const release = () => { + clearTimeout(timeoutId); + signal?.removeEventListener('abort', abortRequest); + }; // 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' }, - }, - }); + // reading the body still carries the request ID and Retry-After. A + // request the caller aborted fails with the caller's reason instead. + const abortError = (responseHeaders?: Headers) => + signal?.aborted + ? signal.reason + : 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; @@ -226,7 +244,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { // 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); + release(); let responseJson: any; @@ -269,10 +287,10 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { // AbortError a fetch implementation raises for an aborted body may // come from another realm. throw abortController.signal.aborted - ? timeoutError(response.headers) + ? abortError(response.headers) : error; } finally { - clearTimeout(timeoutId); + release(); } })(); rawBody.catch(() => undefined); @@ -280,11 +298,11 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { return new FetchHttpClientResponse(res, rawBody); } catch (error) { // Clear timeout if request failed - clearTimeout(timeoutId); + release(); - // Handle timeout errors + // Handle timeouts and aborts if (abortController.signal.aborted) { - throw timeoutError(res?.headers); + throw abortError(res?.headers); } throw error; @@ -297,6 +315,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { body?: any, headers?: RequestHeaders, maxRetries?: number, + signal?: AbortSignal, ): Promise { const maxRetryAttempts = maxRetries ?? this.MAX_RETRY_ATTEMPTS; @@ -318,12 +337,19 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { let requestError: any = null; try { - response = await this.fetchRequest(url, method, body, requestHeaders); + response = await this.fetchRequest( + url, + method, + body, + requestHeaders, + signal, + ); } catch (e) { requestError = e; } if ( + !signal?.aborted && this.shouldRetryRequest(requestError, retryAttempts, maxRetryAttempts) ) { retryAttempts++; diff --git a/src/feature-flags/runtime-client.spec.ts b/src/feature-flags/runtime-client.spec.ts index 6b4d2a82c..9fa63cf1b 100644 --- a/src/feature-flags/runtime-client.spec.ts +++ b/src/feature-flags/runtime-client.spec.ts @@ -148,6 +148,53 @@ describe('FeatureFlagsRuntimeClient', () => { // Only the initial poll should have happened expect(fetch.mock.calls.length).toBe(1); }); + + it('aborts the in-flight poll request', async () => { + fetch.mockResponseOnce(() => new Promise(() => {})); + const client = workos.featureFlags.createRuntimeClient(); + await jest.advanceTimersByTimeAsync(0); + const signal = fetch.mock.calls[0][1]?.signal; + expect(signal?.aborted).toBe(false); + + client.close(); + + expect(signal?.aborted).toBe(true); + }); + }); + + describe('requestTimeoutMs', () => { + it('emits a timeout error when a poll takes too long', async () => { + fetch.mockResponseOnce(() => new Promise(() => {})); + const client = workos.featureFlags.createRuntimeClient({ + requestTimeoutMs: 50, + }); + const errors: Error[] = []; + client.on('error', (err) => errors.push(err)); + + await jest.advanceTimersByTimeAsync(50); + + expect(errors).toHaveLength(1); + expect(errors[0].message).toBe('Request timed out'); + + client.close(); + }); + + it('aborts the request of a poll that takes too long', async () => { + fetch.mockResponseOnce(() => new Promise(() => {})); + const client = workos.featureFlags.createRuntimeClient({ + requestTimeoutMs: 50, + }); + client.on('error', () => {}); + + await jest.advanceTimersByTimeAsync(49); + const signal = fetch.mock.calls[0][1]?.signal; + expect(signal?.aborted).toBe(false); + + await jest.advanceTimersByTimeAsync(1); + expect(signal?.aborted).toBe(true); + + client.close(); + }); }); describe('isEnabled', () => { diff --git a/src/feature-flags/runtime-client.ts b/src/feature-flags/runtime-client.ts index cd2a99c03..d5f0365fa 100644 --- a/src/feature-flags/runtime-client.ts +++ b/src/feature-flags/runtime-client.ts @@ -233,38 +233,29 @@ export class FeatureFlagsRuntimeClient extends EventEmitter this.pollAbortController = new AbortController(); const { signal } = this.pollAbortController; - let timeoutId: ReturnType; + const timeoutId = setTimeout(() => { + this.pollAbortController?.abort(new Error('Request timed out')); + }, this.requestTimeoutMs); const fetchPromise = this.workos - .get('/sdk/feature-flags?payload_version=2') + .get('/sdk/feature-flags?payload_version=2', { signal }) .then(({ data }) => data); - const timeoutPromise = new Promise((_, reject) => { - timeoutId = setTimeout(() => { - this.pollAbortController?.abort(); - reject(new Error('Request timed out')); - }, this.requestTimeoutMs); - }); - + // Settles the poll as soon as it is aborted, even while the HTTP client + // is between retries. const abortPromise = new Promise((_, reject) => { if (signal.aborted) { - reject(new Error('Poll aborted')); + reject(signal.reason); return; } - signal.addEventListener( - 'abort', - () => reject(new Error('Poll aborted')), - { - once: true, - }, - ); + signal.addEventListener('abort', () => reject(signal.reason), { + once: true, + }); }); - return Promise.race([fetchPromise, timeoutPromise, abortPromise]).finally( - () => { - clearTimeout(timeoutId); - }, - ); + return Promise.race([fetchPromise, abortPromise]).finally(() => { + clearTimeout(timeoutId); + }); } private scheduleNextPoll(): void { diff --git a/src/workos.spec.ts b/src/workos.spec.ts index 2e61a1107..b2d3eddb6 100644 --- a/src/workos.spec.ts +++ b/src/workos.spec.ts @@ -614,6 +614,32 @@ describe('WorkOS', () => { expect(error.requestID).toBe('a-request-id'); }); }); + + describe('when the signal aborts', () => { + it('aborts the request and rejects with the signal reason', async () => { + const pendingFetch = jest.fn( + (_url: any, init: any) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const abortError = new Error('Aborted'); + abortError.name = 'AbortError'; + reject(abortError); + }); + }) as any, + ); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + fetchFn: pendingFetch as any, + }); + const controller = new AbortController(); + const reason = new Error('Stop'); + + const request = workos.get('/path', { signal: controller.signal }); + controller.abort(reason); + + await expect(request).rejects.toBe(reason); + expect(pendingFetch.mock.calls[0][1].signal.aborted).toBe(true); + }); + }); }); describe('put', () => { diff --git a/src/workos.ts b/src/workos.ts index 780e51f43..d869a7763 100644 --- a/src/workos.ts +++ b/src/workos.ts @@ -284,8 +284,10 @@ export class WorkOS { params: options.query, headers: requestHeaders, maxRetries: options.maxRetries, + signal: options.signal, }); } catch (error) { + options.signal?.throwIfAborted(); this.handleHttpError({ path, error }); throw error; From 31b860e2f860647a6958e7d2725ab85b32ab9fa8 Mon Sep 17 00:00:00 2001 From: Stanley Phu Date: Thu, 5 Nov 2026 00:24:07 -0800 Subject: [PATCH 2/2] fix(feature-flags): Reject requests aborted with a null reason `controller.abort(null)` sets the signal's reason to null, which the retry loop mistook for success, so `workos.get` failed with a TypeError instead of rejecting with the caller's reason. Co-Authored-By: Claude Opus 5.5 --- src/common/net/fetch-client.spec.ts | 26 ++++++++++++++++++++++++++ src/common/net/fetch-client.ts | 5 ++++- src/workos.spec.ts | 22 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/common/net/fetch-client.spec.ts b/src/common/net/fetch-client.spec.ts index 6becd994c..cb8dd28bd 100644 --- a/src/common/net/fetch-client.spec.ts +++ b/src/common/net/fetch-client.spec.ts @@ -692,6 +692,32 @@ describe('FetchHttpClient with an abort signal', () => { expect(mockFetch.mock.calls[0][1].signal.aborted).toBe(true); }); + it('fails with a null signal reason instead of returning no response', async () => { + const mockFetch = jest.fn(); + mockFetch.mockImplementation( + (_, options) => + new Promise((_, reject) => { + options.signal.addEventListener('abort', () => { + const error = new Error('AbortError'); + error.name = 'AbortError'; + reject(error); + }); + }), + ); + const client = new FetchHttpClient( + 'https://api.example.com', + {}, + mockFetch, + ); + const controller = new AbortController(); + + const request = client.get('/users', { signal: controller.signal }); + controller.abort(null); + + await expect(request).rejects.toBeNull(); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + it('sends no further attempt once the signal aborts between retries', async () => { fetchOnce({}, { status: 500 }); const controller = new AbortController(); diff --git a/src/common/net/fetch-client.ts b/src/common/net/fetch-client.ts index 774c782aa..f13998cfe 100644 --- a/src/common/net/fetch-client.ts +++ b/src/common/net/fetch-client.ts @@ -335,6 +335,8 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { const makeRequest = async (): Promise => { let requestError: any = null; + // Tracked apart from the error: an abort can reject with a null reason. + let failed = false; try { response = await this.fetchRequest( @@ -346,6 +348,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { ); } catch (e) { requestError = e; + failed = true; } if ( @@ -360,7 +363,7 @@ export class FetchHttpClient extends HttpClient implements HttpClientInterface { return makeRequest(); } - if (requestError != null) { + if (failed) { throw requestError; } diff --git a/src/workos.spec.ts b/src/workos.spec.ts index b2d3eddb6..4a7781259 100644 --- a/src/workos.spec.ts +++ b/src/workos.spec.ts @@ -639,6 +639,28 @@ describe('WorkOS', () => { await expect(request).rejects.toBe(reason); expect(pendingFetch.mock.calls[0][1].signal.aborted).toBe(true); }); + + it('rejects with a null signal reason', async () => { + const pendingFetch = jest.fn( + (_url: any, init: any) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const abortError = new Error('Aborted'); + abortError.name = 'AbortError'; + reject(abortError); + }); + }) as any, + ); + const workos = new WorkOS('sk_test_Sz3IQjepeSWaI4cMS4ms4sMuU', { + fetchFn: pendingFetch as any, + }); + const controller = new AbortController(); + + const request = workos.get('/path', { signal: controller.signal }); + controller.abort(null); + + await expect(request).rejects.toBeNull(); + }); }); });