Skip to content

Commit 027ee32

Browse files
committed
chore: pr comments
1 parent 01a2a59 commit 027ee32

3 files changed

Lines changed: 71 additions & 5 deletions

File tree

packages/sdk/node-client/__tests__/NodeClient.dataSource.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,3 +406,64 @@ it('does not read cached flags when bootstrap is provided', async () => {
406406

407407
await client.close();
408408
});
409+
410+
it('rejects identify rather than hanging when the mode flips to offline mid-identify', async () => {
411+
// Gate the cached-flag read so we can flip the connection mode while identify is parked on
412+
// the await -- reproducing the race where _setupConnection later sees connectionMode==='offline'.
413+
let releaseGet: () => void = () => {};
414+
const getGate = new Promise<void>((resolve) => {
415+
releaseGet = resolve;
416+
});
417+
418+
const fakePlatform = makeMockPlatform({
419+
requests: {
420+
fetch: jest.fn(),
421+
createEventSource: jest.fn(() => ({
422+
addEventListener: jest.fn(),
423+
close: jest.fn(),
424+
onclose: jest.fn(),
425+
onerror: jest.fn(),
426+
onopen: jest.fn(),
427+
onretrying: jest.fn(),
428+
})),
429+
getEventSourceCapabilities: () => ({ readTimeout: true, headers: true, customMethod: true }),
430+
},
431+
});
432+
(fakePlatform as any).storage = {
433+
get: jest.fn(async () => {
434+
await getGate;
435+
return null;
436+
}),
437+
set: jest.fn(async () => {}),
438+
clear: jest.fn(async () => {}),
439+
};
440+
NodePlatformMock.mockImplementationOnce(() => fakePlatform);
441+
442+
const client = createClient(
443+
'client-side-id',
444+
{ kind: 'user', key: 'bob' },
445+
{
446+
initialConnectionMode: 'streaming',
447+
sendEvents: false,
448+
diagnosticOptOut: true,
449+
logger,
450+
},
451+
);
452+
453+
// Bootstrap on start so the first identify skips the (gated) cache read.
454+
await client.start({ bootstrap: bootstrapData });
455+
456+
// A second identify without bootstrap routes through the cache path and parks on the gated get.
457+
const identifyPromise = client.identify({ kind: 'user', key: 'alice' }, { timeout: 2 });
458+
await new Promise((resolve) => setTimeout(resolve, 10));
459+
460+
// Flip to offline while identify is parked, then release the cache read.
461+
await client.setConnectionMode('offline');
462+
releaseGet();
463+
464+
const result = await identifyPromise;
465+
// With the fix the identify settles immediately as an error; the bug would hang to timeout.
466+
expect(result.status).toBe('error');
467+
468+
await client.close();
469+
});

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,12 @@ export class NodeClient extends LDClientImpl {
105105
async setConnectionMode(mode: ConnectionMode): Promise<void> {
106106
const task = this._connectionModeQueue.then(async () => {
107107
const dataManager = this.dataManager as NodeDataManager;
108-
if (mode === 'offline') {
109-
// Disable analytics, then drain any queued events before tearing down the data source.
110-
this.setEventSendingEnabled(false, false);
111-
await this.flush();
112-
}
113108
try {
109+
if (mode === 'offline') {
110+
// Disable analytics, then drain any queued events before tearing down the data source.
111+
this.setEventSendingEnabled(false, false);
112+
await this.flush();
113+
}
114114
await dataManager.setConnectionMode(mode);
115115
} finally {
116116
// Read the mode back so event-sending always matches the mode that actually took

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,11 @@ export default class NodeDataManager extends BaseDataManager {
181181
`${logTag} _setupConnection called with unsupported connectionMode '${this.connectionMode}'.`,
182182
);
183183
this.updateProcessor = undefined;
184+
// The mode may have changed to 'offline' while identify was awaiting the cache; reject
185+
// rather than leave the identify promise to hang until its timeout.
186+
identifyReject?.(
187+
new Error(`Connection mode changed to '${this.connectionMode}' during identify.`),
188+
);
184189
return;
185190
}
186191
this.updateProcessor!.start();

0 commit comments

Comments
 (0)