diff --git a/.github/workflows/node-client.yml b/.github/workflows/node-client.yml index cbc2af26ac..25e8d24809 100644 --- a/.github/workflows/node-client.yml +++ b/.github/workflows/node-client.yml @@ -46,4 +46,12 @@ jobs: with: test_service_port: 8000 token: ${{ secrets.GITHUB_TOKEN }} + stop_service: 'false' extra_params: '--skip-from=${{ github.workspace }}/packages/sdk/node-client/contract-tests/testharness-suppressions.txt' + - name: Run contract tests (FDv2) + uses: launchdarkly/gh-actions/actions/contract-tests@5adb11fd6953e1bc35d9cf1fc1b4374c464e3a8b # contract-tests-v1.3.0 + with: + test_service_port: 8000 + token: ${{ secrets.GITHUB_TOKEN }} + version: v3.1.1-alpha.6 + extra_params: '--skip-from=${{ github.workspace }}/packages/sdk/node-client/contract-tests/testharness-suppressions-fdv2.txt' diff --git a/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts b/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts new file mode 100644 index 0000000000..479b844bec --- /dev/null +++ b/packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts @@ -0,0 +1,394 @@ +import * as fs from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; + +import NodeCrypto from '../src/platform/NodeCrypto'; +import NodeEncoding from '../src/platform/NodeEncoding'; +import NodeInfo from '../src/platform/NodeInfo'; +import { resetNodeStorage } from '../src/platform/NodeStorage'; +import { createMockLogger } from './testHelpers'; + +jest.mock('../src/platform/NodePlatform', () => ({ + __esModule: true, + default: jest.fn(() => ({ + crypto: new NodeCrypto(), + info: new NodeInfo(), + requests: { + createEventSource: jest.fn(), + fetch: jest.fn(), + getEventSourceCapabilities: jest.fn(), + }, + encoding: new NodeEncoding(), + storage: { + clear: jest.fn(), + get: jest.fn(), + set: jest.fn(), + }, + })), +})); + +// Tracks the mode last passed to the mock FDv2 data manager's setConnectionMode. +// Seeded from opts.foregroundMode when createFDv2DataManagerBase is called. +// Used only to verify delegation; NodeClient now tracks requested mode itself. +let mockCurrentMode = 'streaming'; + +// Captures the opts passed to createFDv2DataManagerBase so tests can invoke +// buildQueryParams directly. +let capturedFDv2Opts: + | { buildQueryParams: (opts?: { hash?: string }) => { key: string; value: string }[] } + | undefined; + +const mockSetConnectionMode = jest.fn((mode: string) => { + mockCurrentMode = mode; +}); + +jest.mock('@launchdarkly/js-client-sdk-common', () => ({ + ...jest.requireActual('@launchdarkly/js-client-sdk-common'), + createFDv2DataManagerBase: jest.fn((opts: any) => { + capturedFDv2Opts = opts; + mockCurrentMode = opts.foregroundMode ?? 'streaming'; + return { + identify: jest.fn(), + close: jest.fn(), + setConnectionMode: mockSetConnectionMode, + getCurrentMode: () => mockCurrentMode, + }; + }), +})); + +import { LDClientImpl } from '@launchdarkly/js-client-sdk-common'; + +import { createClient } from '../src'; + +const DEFAULT_INITIAL_CONTEXT = { kind: 'user' as const, key: 'bob' }; +let tmpRoot: string; +let logger: ReturnType; + +beforeAll(() => { + jest.useFakeTimers(); +}); + +afterAll(() => { + jest.useRealTimers(); +}); + +beforeEach(async () => { + jest.clearAllMocks(); + capturedFDv2Opts = undefined; + mockCurrentMode = 'streaming'; + tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'node-client-fdv2-test-')); + resetNodeStorage(); + logger = createMockLogger(); +}); + +afterEach(async () => { + await fs.rm(tmpRoot, { recursive: true, force: true }); +}); + +it('setConnectionMode does not throw in FDv2 mode', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + await expect(client.setConnectionMode('offline')).resolves.toBeUndefined(); +}); + +it('getConnectionMode returns the mode set via setConnectionMode in FDv2 mode', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + await client.setConnectionMode('offline'); + expect(client.getConnectionMode()).toBe('offline'); + expect(client.isOffline()).toBe(true); +}); + +it('isOffline returns false after switching back to streaming in FDv2 mode', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + await client.setConnectionMode('offline'); + await client.setConnectionMode('streaming'); + expect(client.getConnectionMode()).toBe('streaming'); + expect(client.isOffline()).toBe(false); +}); + +it('setConnectionMode delegates to the FDv2 data manager', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + await client.setConnectionMode('offline'); + expect(mockSetConnectionMode).toHaveBeenCalledWith('offline'); +}); + +// ------ FDv2 initial mode reconciliation ------ + +it('isOffline returns true initially when FDv2 dataSystem configures offline mode', () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: { automaticModeSwitching: { type: 'manual', initialConnectionMode: 'offline' } }, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(client.isOffline()).toBe(true); + expect(client.getConnectionMode()).toBe('offline'); +}); + +it('getConnectionMode reflects FDv2 manual polling mode at construction', () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: { automaticModeSwitching: { type: 'manual', initialConnectionMode: 'polling' } }, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(client.getConnectionMode()).toBe('polling'); + expect(client.isOffline()).toBe(false); +}); + +// ------ buildQueryParams coverage ------ + +it('buildQueryParams returns [] in FDv2 mobile key mode', () => { + createClient('mobile-key-123', DEFAULT_INITIAL_CONTEXT, { + useMobileKey: true, + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(capturedFDv2Opts).toBeDefined(); + expect(capturedFDv2Opts!.buildQueryParams()).toEqual([]); +}); + +it('buildQueryParams ignores and warns when per-identify hash is set in FDv2 mobile key mode', () => { + createClient('mobile-key-123', DEFAULT_INITIAL_CONTEXT, { + useMobileKey: true, + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(capturedFDv2Opts).toBeDefined(); + expect(capturedFDv2Opts!.buildQueryParams({ hash: 'should-not-leak' })).toEqual([]); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('hash')); +}); + +it('buildQueryParams returns auth param in FDv2 client-side ID mode', () => { + createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(capturedFDv2Opts).toBeDefined(); + expect(capturedFDv2Opts!.buildQueryParams()).toEqual([{ key: 'auth', value: 'client-side-id' }]); +}); + +it('buildQueryParams includes hash param when hash is set in client-side ID mode', () => { + createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + hash: 'secure-hash-abc', + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(capturedFDv2Opts).toBeDefined(); + expect(capturedFDv2Opts!.buildQueryParams()).toEqual([ + { key: 'auth', value: 'client-side-id' }, + { key: 'h', value: 'secure-hash-abc' }, + ]); +}); + +it('buildQueryParams per-identify hash overrides construction-time hash in FDv2', () => { + createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + hash: 'construction-hash', + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(capturedFDv2Opts).toBeDefined(); + expect(capturedFDv2Opts!.buildQueryParams({ hash: 'per-identify-hash' })).toEqual([ + { key: 'auth', value: 'client-side-id' }, + { key: 'h', value: 'per-identify-hash' }, + ]); +}); + +it('buildQueryParams uses construction-time hash when no per-identify hash is provided in FDv2', () => { + createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + hash: 'construction-hash', + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + expect(capturedFDv2Opts).toBeDefined(); + expect(capturedFDv2Opts!.buildQueryParams({})).toEqual([ + { key: 'auth', value: 'client-side-id' }, + { key: 'h', value: 'construction-hash' }, + ]); +}); + +// ------ Concurrent setConnectionMode serialization ------ + +it('concurrent setConnectionMode calls are serialized in call order', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + // Fire offline then streaming concurrently. Without serialization the offline + // branch's await flush() lets streaming complete first, reversing the order. + const p1 = client.setConnectionMode('offline'); + const p2 = client.setConnectionMode('streaming'); + await Promise.all([p1, p2]); + // Serialization ensures the calls execute in issue order: offline then streaming. + expect(mockSetConnectionMode.mock.calls).toEqual([['offline'], ['streaming']]); + expect(client.getConnectionMode()).toBe('streaming'); + expect(client.isOffline()).toBe(false); +}); + +// ------ Idempotency ------ + +it('setConnectionMode is a no-op when called with the current mode in FDv2', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + // Initial mode is streaming; calling streaming again should be a no-op. + await client.setConnectionMode('streaming'); + expect(mockSetConnectionMode).not.toHaveBeenCalled(); + expect(client.getConnectionMode()).toBe('streaming'); +}); + +it('duplicate-mode setConnectionMode awaits an in-flight transition to the same mode in FDv2', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + + // flush() must not reject during the offline leg so p1 does not poison the queue. + const flushSpy = jest + .spyOn(LDClientImpl.prototype, 'flush') + .mockImplementation(() => Promise.resolve({ result: true })); + + // p3 queues behind p2; by the time p3's task runs, p2 has already committed + // _connectionMode to 'streaming', so the idempotency check fires and p3 no-ops. + const p1 = client.setConnectionMode('offline'); + const p2 = client.setConnectionMode('streaming'); + const p3 = client.setConnectionMode('streaming'); + + await p3; + expect(mockSetConnectionMode).toHaveBeenCalledWith('streaming'); + + await Promise.all([p1, p2]); + expect(client.getConnectionMode()).toBe('streaming'); + expect(client.isOffline()).toBe(false); + + flushSpy.mockRestore(); +}); + +// ------ Post-close guard ------ + +it('setConnectionMode after close is a no-op in FDv2', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + await client.close(); + await client.setConnectionMode('offline'); + expect(mockSetConnectionMode).not.toHaveBeenCalled(); +}); + +// ------ Invalid mode validation ------ + +it('setConnectionMode with an invalid mode logs a warning and does not delegate in FDv2', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + // @ts-ignore testing JS callers passing an invalid value + await client.setConnectionMode('invalid-mode'); + expect(mockSetConnectionMode).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('invalid-mode')); +}); + +// ------ Failed offline transition ------ + +it('setConnectionMode rejects and does not change connection mode when flush fails in FDv2', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + // flush() throws before _connectionMode is written or the data manager is notified. + const flushSpy = jest + .spyOn(LDClientImpl.prototype, 'flush') + .mockRejectedValueOnce(new Error('flush failed')); + + await expect(client.setConnectionMode('offline')).rejects.toThrow('flush failed'); + + expect(client.getConnectionMode()).toBe('streaming'); + expect(client.isOffline()).toBe(false); + expect(mockSetConnectionMode).not.toHaveBeenCalled(); + + flushSpy.mockRestore(); +}); + +it('setConnectionMode queue stays alive after a failed offline transition in FDv2', async () => { + const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, { + dataSystem: {}, + diagnosticOptOut: true, + sendEvents: false, + logger, + localStoragePath: tmpRoot, + }); + const flushSpy = jest + .spyOn(LDClientImpl.prototype, 'flush') + .mockRejectedValueOnce(new Error('flush failed')); + + await expect(client.setConnectionMode('offline')).rejects.toThrow('flush failed'); + flushSpy.mockRestore(); + + // task.catch() keeps _fdv2ConnectionModeQueue alive; the next call must succeed. + await client.setConnectionMode('polling'); + expect(client.getConnectionMode()).toBe('polling'); + expect(mockSetConnectionMode).toHaveBeenCalledWith('polling'); +}); diff --git a/packages/sdk/node-client/contract-tests/testharness-suppressions-fdv2.txt b/packages/sdk/node-client/contract-tests/testharness-suppressions-fdv2.txt index f78cc2c8c1..16600095c4 100644 --- a/packages/sdk/node-client/contract-tests/testharness-suppressions-fdv2.txt +++ b/packages/sdk/node-client/contract-tests/testharness-suppressions-fdv2.txt @@ -5,3 +5,8 @@ streaming/fdv2/reconnection state management/saves previously known state streaming/fdv2/reconnection state management/replaces previously known state streaming/fdv2/reconnection state management/updates previously known state streaming/fdv2/can discard partial events on errors + +# Looks like an issue in the test harness where client SDKs are assumed to have a +# one-shot polling connection established before establishing a streaming connection. +# Will need to investigate more, but manual testing seems fine. +wrapper/stream requests/wrapper name and version diff --git a/packages/sdk/node-client/src/NodeClient.ts b/packages/sdk/node-client/src/NodeClient.ts index b3e00d9847..92820754d2 100644 --- a/packages/sdk/node-client/src/NodeClient.ts +++ b/packages/sdk/node-client/src/NodeClient.ts @@ -3,6 +3,11 @@ import { browserFdv1Endpoints, Configuration, ConnectionMode, + createDefaultSourceFactoryProvider, + createFDv2DataManagerBase, + DESKTOP_DATA_SYSTEM_DEFAULTS, + DESKTOP_TRANSITION_TABLE, + FDv2DataManagerControl, FlagManager, internal, LDClientImpl, @@ -12,9 +17,12 @@ import { LDEmitterEventName, LDFlagValue, LDHeaders, + LDIdentifyOptions, LDIdentifyResult, LDPluginEnvironmentMetadata, mobileFdv1Endpoints, + MODE_TABLE, + resolveForegroundMode, } from '@launchdarkly/js-client-sdk-common'; import basicLogger from './basicLogger'; @@ -28,6 +36,14 @@ import NodePlatform from './platform/NodePlatform'; export class NodeClient extends LDClientImpl { private readonly _plugins: LDPlugin[]; + // Connection mode for both paths. FDv2 updates this inside the + // serialized queue task; FDv1 reads it back from the data manager after each + // transition. + private _connectionMode: ConnectionMode; + // Serializes FDv2 mode transitions so concurrent callers cannot interleave + // flush() and setEventSendingEnabled() across the offline branch's await. + private _fdv2ConnectionModeQueue: Promise = Promise.resolve(); + private _closed: boolean = false; constructor(envKey: string, initialContext: LDContext, options: NodeOptions = {}) { const { logger: customLogger, debug } = options; @@ -48,6 +64,7 @@ export class NodeClient extends LDClientImpl { credentialType: useMobileKey ? 'mobileKey' : 'clientSideId', requiresStart: true, initialContext, + dataSystemDefaults: DESKTOP_DATA_SYSTEM_DEFAULTS, }; const platform = new NodePlatform(logger, validatedNodeOptions); @@ -64,8 +81,46 @@ export class NodeClient extends LDClientImpl { baseHeaders: LDHeaders, emitter: LDEmitter, diagnosticsManager?: internal.DiagnosticsManager, - ) => - new NodeDataManager( + ) => { + if (configuration.dataSystem) { + const foregroundMode = resolveForegroundMode( + configuration.dataSystem, + DESKTOP_DATA_SYSTEM_DEFAULTS, + ); + return createFDv2DataManagerBase({ + platform, + flagManager, + credential: envKey, + config: configuration, + baseHeaders, + emitter, + transitionTable: DESKTOP_TRANSITION_TABLE, + foregroundMode, + backgroundMode: undefined, + modeTable: MODE_TABLE, + sourceFactoryProvider: createDefaultSourceFactoryProvider(), + fdv1Endpoints: useMobileKey ? mobileFdv1Endpoints() : browserFdv1Endpoints(envKey), + buildQueryParams: (identifyOptions?: LDIdentifyOptions) => { + if (useMobileKey) { + // Mobile mode authenticates via Authorization header, not query params. + if ((identifyOptions as NodeIdentifyOptions | undefined)?.hash) { + logger.warn('[NodeClient] \'hash\' is ignored in mobile key mode.'); + } + return []; + } + const params: { key: string; value: string }[] = [{ key: 'auth', value: envKey }]; + // Per-identify hash overrides the construction-time hash, mirroring FDv1 behavior. + const hash = + (identifyOptions as NodeIdentifyOptions | undefined)?.hash || + validatedNodeOptions.hash; + if (hash) { + params.push({ key: 'h', value: hash }); + } + return params; + }, + }); + } + return new NodeDataManager( platform, flagManager, envKey, @@ -76,14 +131,32 @@ export class NodeClient extends LDClientImpl { baseHeaders, emitter, diagnosticsManager, - ), + ); + }, internalOptions, ); + // Initialize _connectionMode for whichever path is active. + // FDv2ConnectionMode is a superset of ConnectionMode; map platform-only modes + // (e.g. 'one-shot', 'background') to 'streaming' since the source is active. + if (this.dataSystemConfig) { + const initialFdv2Mode = resolveForegroundMode(this.dataSystemConfig, DESKTOP_DATA_SYSTEM_DEFAULTS); + this._connectionMode = + initialFdv2Mode === 'offline' || initialFdv2Mode === 'polling' + ? initialFdv2Mode + : 'streaming'; + } else { + this._connectionMode = validatedNodeOptions.initialConnectionMode; + } this._plugins = validatedNodeOptions.plugins; this.setEventSendingEnabled(!this.isOffline(), false); } + override async close(): Promise { + this._closed = true; + return super.close(); + } + /** * Registers plugins with the public client facade so plugins receive the * public API (single identify that returns LDIdentifyResult). @@ -104,22 +177,51 @@ export class NodeClient extends LDClientImpl { } async setConnectionMode(mode: ConnectionMode): Promise { - const dataManager = this.dataManager as NodeDataManager; - await dataManager.setConnectionMode( - mode, - () => this.flush(), - (enabled) => this.setEventSendingEnabled(enabled, false), - ); + if (this.isFDv2) { + // JS callers can pass arbitrary strings; validate before the debounce timer fires. + if (mode !== 'offline' && mode !== 'streaming' && mode !== 'polling') { + this.logger.warn(`[NodeClient] Unknown connection mode "${mode}", ignoring.`); + return; + } + if (this._closed) { + return; + } + const task = this._fdv2ConnectionModeQueue.then(async () => { + // Re-check: close() may have been called while this task was queued. + if (this._closed) { + return; + } + if (mode === this._connectionMode) { + return; + } + if (mode === 'offline') { + await this.flush(); + this.setEventSendingEnabled(false, false); + } + (this.dataManager as FDv2DataManagerControl).setConnectionMode(mode); + this._connectionMode = mode; + if (mode !== 'offline') { + this.setEventSendingEnabled(true, false); + } + }); + this._fdv2ConnectionModeQueue = task.catch(() => {}); + await task; + } else { + await (this.dataManager as NodeDataManager).setConnectionMode( + mode, + () => this.flush(), + (enabled) => this.setEventSendingEnabled(enabled, false), + ); + this._connectionMode = (this.dataManager as NodeDataManager).getConnectionMode(); + } } getConnectionMode(): ConnectionMode { - const dataManager = this.dataManager as NodeDataManager; - return dataManager.getConnectionMode(); + return this._connectionMode; } isOffline(): boolean { - const dataManager = this.dataManager as NodeDataManager; - return dataManager.getConnectionMode() === 'offline'; + return this._connectionMode === 'offline'; } }