From 899c4c08261051e3cc28070ba3c1bb434471f050 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 31 Jul 2026 12:27:23 -0400 Subject: [PATCH] fix(node-client-sdk): adding docs to clarify FDv2 datasystem will ignore `initialConnectionMode` --- .../__tests__/NodeClientFDv2.test.ts | 31 ++++++++ .../sdk/node-client/__tests__/options.test.ts | 75 +++++++++++++++++++ packages/sdk/node-client/src/NodeOptions.ts | 6 ++ packages/sdk/node-client/src/options.ts | 15 ++++ 4 files changed, 127 insertions(+) diff --git a/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts b/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts index 479b844bec..045ecfea30 100644 --- a/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts +++ b/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts @@ -161,6 +161,37 @@ it('getConnectionMode reflects FDv2 manual polling mode at construction', () => expect(client.isOffline()).toBe(false); }); +it('warns that initialConnectionMode is ignored when dataSystem is also configured', () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + initialConnectionMode: 'offline', + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('initialConnectionMode')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('dataSystem')); + + // The warning is diagnostic only: the FDv2 data system still owns mode resolution, + // so the FDv1-style initialConnectionMode does not take effect. + expect(client.getConnectionMode()).toBe('streaming'); + expect(client.isOffline()).toBe(false); +}); + +it('does not warn about initialConnectionMode when only dataSystem is configured', () => { + createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + + expect(logger.warn).not.toHaveBeenCalled(); +}); + // ------ buildQueryParams coverage ------ it('buildQueryParams returns [] in FDv2 mobile key mode', () => { diff --git a/packages/sdk/node-client/__tests__/options.test.ts b/packages/sdk/node-client/__tests__/options.test.ts index d98e38fcef..44398e0e1d 100644 --- a/packages/sdk/node-client/__tests__/options.test.ts +++ b/packages/sdk/node-client/__tests__/options.test.ts @@ -135,6 +135,81 @@ it('does not warn when only localStoragePath is set', () => { expect(logger.warn).not.toHaveBeenCalled(); }); +it('warns when both dataSystem and initialConnectionMode are set', () => { + const out = validateOptions({ dataSystem: {}, initialConnectionMode: 'offline' }, logger); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('initialConnectionMode')); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('dataSystem')); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('dataSystem.automaticModeSwitching.initialConnectionMode'), + ); + // Warning only: the validated value passes through unchanged. NodeClient ignores it + // on the FDv2 path. + expect(out.initialConnectionMode).toBe('offline'); +}); + +it('warns when dataSystem is set alongside an explicit streaming initialConnectionMode', () => { + validateOptions({ dataSystem: {}, initialConnectionMode: 'streaming' }, logger); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('initialConnectionMode')); +}); + +it('warns when initialConnectionMode is set alongside a manual-mode dataSystem', () => { + validateOptions( + { + dataSystem: { automaticModeSwitching: { type: 'manual', initialConnectionMode: 'polling' } }, + initialConnectionMode: 'offline', + }, + logger, + ); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('initialConnectionMode')); +}); + +it('does not warn when dataSystem is set without initialConnectionMode', () => { + const out = validateOptions({ dataSystem: {} }, logger); + + // The default is applied but the user did not opt in, so no warning is appropriate. + expect(out.initialConnectionMode).toBe('streaming'); + expect(logger.warn).not.toHaveBeenCalled(); +}); + +it('does not warn when initialConnectionMode is set without dataSystem', () => { + const out = validateOptions({ initialConnectionMode: 'offline' }, logger); + + expect(out.initialConnectionMode).toBe('offline'); + expect(logger.warn).not.toHaveBeenCalled(); +}); + +it('does not warn when dataSystem is null', () => { + const out = validateOptions({ dataSystem: null as any, initialConnectionMode: 'offline' }, logger); + + // null dataSystem never activates FDv2 (Configuration's validator skips nullish values), + // so initialConnectionMode is honored and warning here would be wrong. + expect(logger.warn).not.toHaveBeenCalledWith(expect.stringContaining('dataSystem')); + expect(out.initialConnectionMode).toBe('offline'); +}); + +it('does not warn when dataSystem is a non-object value', () => { + validateOptions({ dataSystem: 'streaming' as any, initialConnectionMode: 'offline' }, logger); + + // A non-object dataSystem fails Configuration's own type check and falls back to FDv1 + // there too, so this mirrors the null case above. + expect(logger.warn).not.toHaveBeenCalledWith( + expect.stringContaining('Both "dataSystem" and "initialConnectionMode"'), + ); +}); + +it('does not warn when initialConnectionMode is null', () => { + validateOptions({ dataSystem: {}, initialConnectionMode: null as any }, logger); + + // The dataSystem/initialConnectionMode conflict check excludes null explicitly, matching + // how a caller would clear the option to opt back into the FDv2 default. + expect(logger.warn).not.toHaveBeenCalledWith( + expect.stringContaining('Both "dataSystem" and "initialConnectionMode"'), + ); +}); + it('defaults useMobileKey to false when omitted', () => { const validated = validateOptions({}, logger); expect(validated.useMobileKey).toBe(false); diff --git a/packages/sdk/node-client/src/NodeOptions.ts b/packages/sdk/node-client/src/NodeOptions.ts index 9998b96a28..d8cf9b1857 100644 --- a/packages/sdk/node-client/src/NodeOptions.ts +++ b/packages/sdk/node-client/src/NodeOptions.ts @@ -48,6 +48,12 @@ export interface NodeOptions extends LDOptionsBase { * Possible values are offline, streaming, or polling. See {@link ConnectionMode} for more information. * * Defaults to streaming. + * + * This option applies only when the FDv2 data system is not configured. When `dataSystem` is + * set, the FDv2 data system owns the initial connection mode and this option is ignored; a + * warning is logged in that case. Use + * `dataSystem.automaticModeSwitching.initialConnectionMode` (with `automaticModeSwitching.type` + * set to `'manual'`) to set the initial connection mode for the FDv2 data system. */ initialConnectionMode?: ConnectionMode; diff --git a/packages/sdk/node-client/src/options.ts b/packages/sdk/node-client/src/options.ts index 80547074e7..9c43b986fb 100644 --- a/packages/sdk/node-client/src/options.ts +++ b/packages/sdk/node-client/src/options.ts @@ -115,6 +115,21 @@ export default function validateOptions(opts: NodeOptions, logger: LDLogger): Va ); } + const dataSystemConfigured = + typeof opts.dataSystem === 'object' && opts.dataSystem !== null && !Array.isArray(opts.dataSystem); + if ( + dataSystemConfigured && + opts.initialConnectionMode !== undefined && + opts.initialConnectionMode !== null + ) { + logger.warn( + 'Both "dataSystem" and "initialConnectionMode" are set. "initialConnectionMode" applies ' + + 'only when the FDv2 data system is not configured and will be ignored. Use ' + + '"dataSystem.automaticModeSwitching.initialConnectionMode" (with automaticModeSwitching.type ' + + 'set to "manual") to set the initial connection mode for the FDv2 data system.', + ); + } + if (output.tlsParams?.rejectUnauthorized === false) { logger.warn( 'TLS certificate verification is disabled via tlsParams.rejectUnauthorized=false. ' +