chore: fix bot comments

fix: Restore FDv2 connection mode when an offline transition fails

fix: Await in-flight FDv2 transition on duplicate-mode setConnectionMode

Author: joker23

packages/sdk/node-client/__tests__/NodeClientFDv2.test.ts

@@ -56,6 +56,8 @@ jest.mock('@launchdarkly/js-client-sdk-common', () => ({
   }),
 }));
 
+import { LDClientImpl } from '@launchdarkly/js-client-sdk-common';
+
 import { createClient } from '../src';
 
 const DEFAULT_INITIAL_CONTEXT = { kind: 'user' as const, key: 'bob' };
@@ -285,6 +287,43 @@ it('setConnectionMode is a no-op when called with the current mode in FDv2', asy
   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,
+  });
+
+  // Make the offline transition's flush() defer to a later microtask so the
+  // offline -> streaming queue is still draining when the duplicate streaming
+  // call is issued. This exposes the early-return race deterministically.
+  const flushSpy = jest
+    .spyOn(LDClientImpl.prototype, 'flush')
+    .mockImplementation(() => Promise.resolve({ result: true }));
+
+  // Sequence: initial mode is 'streaming'. Go offline, then streaming, then
+  // streaming again (the duplicate). The third call's _connectionMode is already
+  // 'streaming' (set synchronously by the second call), so the idempotency guard
+  // fires. Without the await fix, p3 resolves before the second call's queued
+  // task has delegated 'streaming' to the data manager.
+  const p1 = client.setConnectionMode('offline');
+  const p2 = client.setConnectionMode('streaming');
+  const p3 = client.setConnectionMode('streaming');
+
+  // Awaiting only the duplicate call must guarantee the queued streaming
+  // transition has completed (mockSetConnectionMode called with '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 () => {
@@ -315,3 +354,51 @@ it('setConnectionMode with an invalid mode logs a warning and does not delegate
   expect(mockSetConnectionMode).not.toHaveBeenCalled();
   expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('invalid-mode'));
 });
+
+// ------ Mode rollback on failed transition ------
+
+it('restores connection mode when an FDv2 offline transition task throws', async () => {
+  const client = createClient('client-side-id', DEFAULT_INITIAL_CONTEXT, {
+    dataSystem: {},
+    diagnosticOptOut: true,
+    sendEvents: false,
+    logger,
+    localStoragePath: tmpRoot,
+  });
+  // Force the queued offline transition task to throw by making flush() reject.
+  const flushSpy = jest
+    .spyOn(LDClientImpl.prototype, 'flush')
+    .mockRejectedValueOnce(new Error('flush failed'));
+
+  await expect(client.setConnectionMode('offline')).rejects.toThrow('flush failed');
+
+  // The synchronously-set 'offline' must be rolled back to the prior mode so
+  // isOffline() does not lie while the data manager is still streaming.
+  expect(client.getConnectionMode()).toBe('streaming');
+  expect(client.isOffline()).toBe(false);
+  // The data manager was never told to go offline.
+  expect(mockSetConnectionMode).not.toHaveBeenCalled();
+
+  flushSpy.mockRestore();
+});
+
+it('keeps the FDv2 transition queue alive for subsequent calls after a failed transition', 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();
+
+  // A subsequent transition must still work; the queue was not poisoned.
+  await client.setConnectionMode('polling');
+  expect(client.getConnectionMode()).toBe('polling');
+  expect(mockSetConnectionMode).toHaveBeenCalledWith('polling');
+});

packages/sdk/node-client/src/NodeClient.ts

@@ -190,24 +190,47 @@ export class NodeClient extends LDClientImpl {
         return;
       }
       if (mode === this._connectionMode) {
+        // The requested mode matches the optimistically-set _connectionMode, so
+        // no new transition is queued. But _connectionMode is set synchronously
+        // before the queued task runs, so an earlier call's transition to this
+        // same mode may still be in flight. Await the queue tail (without
+        // appending to it) so the caller's promise resolves only after that
+        // transition has actually completed. If nothing is in flight,
+        // _fdv2ConnectionModeQueue is already resolved, so this is a no-op.
+        await this._fdv2ConnectionModeQueue;
         return;
       }
       // FDv2 data manager: serialize transitions so concurrent calls cannot
       // reorder flush/setEventSendingEnabled around the offline await.
+      // Capture the prior mode so a failed transition can be rolled back -
+      // otherwise isOffline()/getConnectionMode() would report a state the
+      // data manager never actually entered.
+      const previousMode = this._connectionMode;
       this._connectionMode = mode;
       const task = this._fdv2ConnectionModeQueue.then(async () => {
         // Re-check after any preceding tasks have run — close() may have fired
         // while this task was queued, matching the FDv1 NodeDataManager guard.
         if (this._closed) {
           return;
         }
-        if (mode === 'offline') {
-          await this.flush();
-          this.setEventSendingEnabled(false, false);
-        }
-        (this.dataManager as FDv2DataManagerControl).setConnectionMode(mode);
-        if (mode !== 'offline') {
-          this.setEventSendingEnabled(true, false);
+        try {
+          if (mode === 'offline') {
+            await this.flush();
+            this.setEventSendingEnabled(false, false);
+          }
+          (this.dataManager as FDv2DataManagerControl).setConnectionMode(mode);
+          if (mode !== 'offline') {
+            this.setEventSendingEnabled(true, false);
+          }
+        } catch (e) {
+          // The transition did not complete; restore the requested-mode marker
+          // so isOffline()/getConnectionMode() stay consistent with the data
+          // manager. Only roll back if no later queued task has already moved
+          // us on from this transition's target mode.
+          if (this._connectionMode === mode) {
+            this._connectionMode = previousMode;
+          }
+          throw e;
         }
       });
       this._fdv2ConnectionModeQueue = task.catch(() => {});