From 037606ac68372584109363a9c876aa5bd0661ef2 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 22 Jun 2026 12:55:09 -0400 Subject: [PATCH] fix: Wire timeout option through to flag polling and event delivery requests The timeout configuration was stored in configuration but never threaded through the datamanger so all connections ran without any timeout regardless of configuration. This commit will fix this for the node server sdk (we still need to assess this for client side sdks and see if it is a problem for other server side sdks). In addition, this commit will also set the timeout default to 10s which is consistent with most other server side sdks (eg python, ruby, c++, etc). --- .../__tests__/platform/NodeRequests.test.ts | 32 +++++++++++++++++++ .../server-node/src/platform/NodeRequests.ts | 4 +++ .../__tests__/data_sources/Requestor.test.ts | 21 +++++++++--- .../createDiagnosticsInitConfig.test.ts | 4 +-- .../__tests__/options/Configuration.test.ts | 6 ++-- .../sdk-server/src/data_sources/Requestor.ts | 3 ++ .../sdk-server/src/options/Configuration.ts | 4 +-- 7 files changed, 63 insertions(+), 11 deletions(-) diff --git a/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts b/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts index 608c9cdc29..dec5a91ad6 100644 --- a/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts +++ b/packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts @@ -181,6 +181,38 @@ describe('given a default instance of NodeRequests', () => { }); }); +describe('given a request to a server that never responds', () => { + let hangingServer: http.Server; + let hangingPort: number; + + beforeAll( + async () => + new Promise((resolveListening) => { + hangingServer = http.createServer((_req, _res) => { + // Intentionally never respond — simulates a hung upstream. + }); + hangingServer.listen(0, () => { + hangingPort = (hangingServer.address() as { port: number }).port; + resolveListening(); + }); + }), + ); + + afterAll( + async () => + new Promise((resolveClose) => { + hangingServer.close(() => resolveClose()); + }), + ); + + it('rejects with "Request timed out" when the timeout elapses', async () => { + const requests = new NodeRequests(); + await expect( + requests.fetch(`http://localhost:${hangingPort}`, { timeout: 50 }), + ).rejects.toThrow('Request timed out'); + }); +}); + describe('given an instance of NodeRequests with enableEventCompression turned on', () => { const requests = new NodeRequests(undefined, undefined, undefined, true); it('can make a basic post with compressBodyIfPossible enabled', async () => { diff --git a/packages/sdk/server-node/src/platform/NodeRequests.ts b/packages/sdk/server-node/src/platform/NodeRequests.ts index 4a8962acab..c224dbfe6c 100644 --- a/packages/sdk/server-node/src/platform/NodeRequests.ts +++ b/packages/sdk/server-node/src/platform/NodeRequests.ts @@ -166,6 +166,10 @@ export default class NodeRequests implements platform.Requests { reject(err); }); + req.on('timeout', () => { + req.destroy(new Error('Request timed out')); + }); + req.end(); }); } diff --git a/packages/shared/sdk-server/__tests__/data_sources/Requestor.test.ts b/packages/shared/sdk-server/__tests__/data_sources/Requestor.test.ts index 967b6d2b82..725c2af469 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/Requestor.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/Requestor.test.ts @@ -16,12 +16,15 @@ describe('given a requestor', () => { let requestor: Requestor; let requestsMade: Array<{ url: string; options: Options }>; + let requests: Requests; let testHeaders: Record; let testStatus = 200; let testResponse: string | undefined; let throwThis: string | undefined; + const baseHeaders = { authorization: 'sdkKey' }; + function resetRequestState() { requestsMade = []; testHeaders = {}; @@ -33,7 +36,7 @@ describe('given a requestor', () => { beforeEach(() => { resetRequestState(); - const requests: Requests = { + requests = { async fetch(url: string, options?: Options): Promise { return new Promise((a, r) => { if (throwThis) { @@ -80,9 +83,7 @@ describe('given a requestor', () => { }, }; - requestor = new Requestor(new Configuration({}), requests, { - authorization: 'sdkKey', - }); + requestor = new Requestor(new Configuration({}), requests, baseHeaders); }); it('gets data', (done) => { @@ -213,4 +214,16 @@ describe('given a requestor', () => { expect(res.err).toBeInstanceOf(LDFlagDeliveryFallbackError); expect(res.fallbackToFDv1).toBe(true); }); + + it('passes the configured timeout (converted to milliseconds) in the request options', async () => { + testResponse = 'a response'; + const localRequestor = new Requestor(new Configuration({ timeout: 10 }), requests, baseHeaders); + const res = await promisify<{ err: any; body: any }>((cb) => { + localRequestor.requestAllData((err, body) => cb({ err, body })); + }); + expect(res.err).toBeUndefined(); + expect(requestsMade.length).toBe(1); + // timeout: 10 (seconds) must be converted to milliseconds before being passed to fetch. + expect(requestsMade[0].options.timeout).toBe(10000); + }); }); diff --git a/packages/shared/sdk-server/__tests__/diagnostics/createDiagnosticsInitConfig.test.ts b/packages/shared/sdk-server/__tests__/diagnostics/createDiagnosticsInitConfig.test.ts index f0e3d21ada..e157838d14 100644 --- a/packages/shared/sdk-server/__tests__/diagnostics/createDiagnosticsInitConfig.test.ts +++ b/packages/shared/sdk-server/__tests__/diagnostics/createDiagnosticsInitConfig.test.ts @@ -18,7 +18,7 @@ describe.each([ {}, { allAttributesPrivate: false, - connectTimeoutMillis: 5000, + connectTimeoutMillis: 10000, customBaseURI: false, customEventsURI: false, customStreamURI: false, @@ -29,7 +29,7 @@ describe.each([ offline: false, pollingIntervalMillis: 30000, reconnectTimeMillis: 1000, - socketTimeoutMillis: 5000, + socketTimeoutMillis: 10000, streamingDisabled: false, contextKeysCapacity: 1000, contextKeysFlushIntervalMillis: 300000, diff --git a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts index 48b8f51682..72b3951463 100644 --- a/packages/shared/sdk-server/__tests__/options/Configuration.test.ts +++ b/packages/shared/sdk-server/__tests__/options/Configuration.test.ts @@ -36,7 +36,7 @@ describe.each([undefined, null, 'potat0', 17, [], {}])('constructed without opti expect(config.stream).toBe(true); expect(config.streamInitialReconnectDelay).toEqual(1); expect(config.tags.value).toBeUndefined(); - expect(config.timeout).toEqual(5); + expect(config.timeout).toEqual(10); expect(config.tlsParams).toBeUndefined(); expect(config.useLdd).toBe(false); expect(config.wrapperName).toBeUndefined(); @@ -138,9 +138,9 @@ describe('when setting different options', () => { }); it.each([ - [0, 0, []], + [0, 1, [{ level: LogLevel.Warn, matches: /Config option "timeout" had invalid value/ }]], [6, 6, []], - ['potato', 5, [{ level: LogLevel.Warn, matches: /Config option "timeout" should be of type/ }]], + ['potato', 10, [{ level: LogLevel.Warn, matches: /Config option "timeout" should be of type/ }]], ])('allow setting timeout and validates timeout', (value, expected, logs) => { // @ts-ignore const config = new Configuration(withLogger({ timeout: value })); diff --git a/packages/shared/sdk-server/src/data_sources/Requestor.ts b/packages/shared/sdk-server/src/data_sources/Requestor.ts index d15e96b831..f1611cb389 100644 --- a/packages/shared/sdk-server/src/data_sources/Requestor.ts +++ b/packages/shared/sdk-server/src/data_sources/Requestor.ts @@ -20,6 +20,7 @@ import Configuration from '../options/Configuration'; export default class Requestor implements LDFeatureRequestor { private readonly _headers: Record; private readonly _serviceEndpoints: ServiceEndpoints; + private readonly _timeoutMs: number; private readonly _eTagCache: Record< string, { @@ -38,6 +39,7 @@ export default class Requestor implements LDFeatureRequestor { ) { this._headers = { ...baseHeaders }; this._serviceEndpoints = serviceEndpointsOverride ?? config.serviceEndpoints; + this._timeoutMs = config.timeout * 1000; } /** @@ -81,6 +83,7 @@ export default class Requestor implements LDFeatureRequestor { const options: Options = { method: 'GET', headers: this._headers, + timeout: this._timeoutMs, }; const uri = getPollingUri(this._serviceEndpoints, this._path, queryParams); diff --git a/packages/shared/sdk-server/src/options/Configuration.ts b/packages/shared/sdk-server/src/options/Configuration.ts index 6297b9e445..a28d635cda 100644 --- a/packages/shared/sdk-server/src/options/Configuration.ts +++ b/packages/shared/sdk-server/src/options/Configuration.ts @@ -48,7 +48,7 @@ const validations: Record = { baseUri: TypeValidators.String, streamUri: TypeValidators.String, eventsUri: TypeValidators.String, - timeout: TypeValidators.Number, + timeout: TypeValidators.numberWithMin(1), capacity: TypeValidators.Number, logger: TypeValidators.Object, featureStore: TypeValidators.ObjectOrFactory, @@ -112,7 +112,7 @@ export const defaultValues: ValidatedOptions = { stream: true, streamInitialReconnectDelay: DEFAULT_STREAM_RECONNECT_DELAY, sendEvents: true, - timeout: 5, + timeout: 10, capacity: 10000, flushInterval: 5, pollInterval: DEFAULT_POLL_INTERVAL,