Skip to content

Commit c4aa363

Browse files
committed
docs: fixing comments
1 parent e0655ea commit c4aa363

2 files changed

Lines changed: 16 additions & 29 deletions

File tree

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -296,24 +296,17 @@ it('duplicate-mode setConnectionMode awaits an in-flight transition to the same
296296
localStoragePath: tmpRoot,
297297
});
298298

299-
// Make the offline transition's flush() defer to a later microtask so the
300-
// offline -> streaming queue is still draining when the duplicate streaming
301-
// call is issued. This exposes the early-return race deterministically.
299+
// Keeps the queue draining when p3 is issued, making the race deterministic.
302300
const flushSpy = jest
303301
.spyOn(LDClientImpl.prototype, 'flush')
304302
.mockImplementation(() => Promise.resolve({ result: true }));
305303

306-
// Sequence: initial mode is 'streaming'. Go offline, then streaming, then
307-
// streaming again (the duplicate). The third call's _connectionMode is already
308-
// 'streaming' (set synchronously by the second call), so the idempotency guard
309-
// fires. Without the await fix, p3 resolves before the second call's queued
310-
// task has delegated 'streaming' to the data manager.
304+
// p3 hits the idempotency guard because p2 sets _connectionMode synchronously,
305+
// before p2's queued task runs. Without the await, p3 resolves too early.
311306
const p1 = client.setConnectionMode('offline');
312307
const p2 = client.setConnectionMode('streaming');
313308
const p3 = client.setConnectionMode('streaming');
314309

315-
// Awaiting only the duplicate call must guarantee the queued streaming
316-
// transition has completed (mockSetConnectionMode called with 'streaming').
317310
await p3;
318311
expect(mockSetConnectionMode).toHaveBeenCalledWith('streaming');
319312

@@ -365,15 +358,16 @@ it('restores connection mode when an FDv2 offline transition task throws', async
365358
logger,
366359
localStoragePath: tmpRoot,
367360
});
368-
// Force the queued offline transition task to throw by making flush() reject.
361+
// flush() is the first async step, so a reject here means setConnectionMode
362+
// on the data manager is never reached.
369363
const flushSpy = jest
370364
.spyOn(LDClientImpl.prototype, 'flush')
371365
.mockRejectedValueOnce(new Error('flush failed'));
372366

373367
await expect(client.setConnectionMode('offline')).rejects.toThrow('flush failed');
374368

375-
// The synchronously-set 'offline' must be rolled back to the prior mode so
376-
// isOffline() does not lie while the data manager is still streaming.
369+
// The synchronously-set 'offline' is rolled back: the data manager is still
370+
// streaming, so isOffline() reflects actual state.
377371
expect(client.getConnectionMode()).toBe('streaming');
378372
expect(client.isOffline()).toBe(false);
379373
// The data manager was never told to go offline.

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

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -190,26 +190,20 @@ export class NodeClient extends LDClientImpl {
190190
return;
191191
}
192192
if (mode === this._connectionMode) {
193-
// The requested mode matches the optimistically-set _connectionMode, so
194-
// no new transition is queued. But _connectionMode is set synchronously
195-
// before the queued task runs, so an earlier call's transition to this
196-
// same mode may still be in flight. Await the queue tail (without
197-
// appending to it) so the caller's promise resolves only after that
198-
// transition has actually completed. If nothing is in flight,
199-
// _fdv2ConnectionModeQueue is already resolved, so this is a no-op.
193+
// Await without appending: an earlier call may still be transitioning to
194+
// this same mode, and we must not resolve until it has finished.
200195
await this._fdv2ConnectionModeQueue;
201196
return;
202197
}
203-
// FDv2 data manager: serialize transitions so concurrent calls cannot
204-
// reorder flush/setEventSendingEnabled around the offline await.
205-
// Capture the prior mode so a failed transition can be rolled back -
198+
// Without serialization, concurrent calls can interleave flush() and
199+
// setEventSendingEnabled() across an offline await, corrupting order.
200+
// Capture the prior mode so a failed transition can be rolled back:
206201
// otherwise isOffline()/getConnectionMode() would report a state the
207202
// data manager never actually entered.
208203
const previousMode = this._connectionMode;
209204
this._connectionMode = mode;
210205
const task = this._fdv2ConnectionModeQueue.then(async () => {
211-
// Re-check after any preceding tasks have run — close() may have fired
212-
// while this task was queued, matching the FDv1 NodeDataManager guard.
206+
// Re-check: close() may have been called while this task was queued.
213207
if (this._closed) {
214208
return;
215209
}
@@ -223,10 +217,9 @@ export class NodeClient extends LDClientImpl {
223217
this.setEventSendingEnabled(true, false);
224218
}
225219
} catch (e) {
226-
// The transition did not complete; restore the requested-mode marker
227-
// so isOffline()/getConnectionMode() stay consistent with the data
228-
// manager. Only roll back if no later queued task has already moved
229-
// us on from this transition's target mode.
220+
// Only roll back if no later queued task has already moved past this
221+
// mode; otherwise isOffline()/getConnectionMode() would drift from
222+
// the state the data manager actually reached.
230223
if (this._connectionMode === mode) {
231224
this._connectionMode = previousMode;
232225
}

0 commit comments

Comments
 (0)