Skip to content

Commit e921962

Browse files
committed
fix: prevent event loss during initialization by setting isReady before onReady
Fixed a race condition where events tracked during the onReady() processing were incorrectly categorized as pending and never sent to Segment. The bug occurred when: 1. App starts tracking events before initialization 2. During init, onReady() begins processing pending events (takes seconds) 3. New events arrive while onReady() is still running 4. These events check isReady (still false) and get saved as "pending" 5. Since onReady() already ran, these new pending events are never processed The fix simply swaps two lines to set isReady=true BEFORE calling onReady(), ensuring any events that arrive during processing go directly to the queue. Added tests to verify the initialization order and prevent regression.
1 parent 7123479 commit e921962

File tree

2 files changed

+139
-1
lines changed

2 files changed

+139
-1
lines changed

packages/core/src/__tests__/analytics.test.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,4 +249,141 @@ describe('SegmentClient', () => {
249249
expect(client.getFlushPolicies().length).toBe(policies.length);
250250
});
251251
});
252+
253+
describe('Initialization order - race condition fix', () => {
254+
/*jshint -W069 */
255+
/* eslint-disable dot-notation */
256+
it('sets isReady to true before executing onReady to prevent events being lost', async () => {
257+
// This test verifies that the race condition fix works:
258+
// isReady is set to true BEFORE onReady() executes,
259+
// so events tracked during onReady() go directly to the queue
260+
// instead of being incorrectly saved as pending events.
261+
262+
client = new SegmentClient(clientArgs);
263+
264+
// Track the value of isReady when onReady is called
265+
let isReadyValueInOnReady: boolean | undefined;
266+
267+
// Mock onReady to capture the isReady state
268+
const originalOnReady = client['onReady'].bind(client);
269+
client['onReady'] = jest.fn(async () => {
270+
// Capture isReady value at the start of onReady
271+
isReadyValueInOnReady = client['isReady'].value;
272+
// Call the original onReady
273+
return originalOnReady();
274+
});
275+
276+
// Initialize the client
277+
await client.init();
278+
279+
// Verify that isReady was true when onReady was called
280+
// This is the key fix - isReady is set BEFORE onReady runs
281+
expect(isReadyValueInOnReady).toBe(true);
282+
283+
// Verify onReady was called
284+
expect(client['onReady']).toHaveBeenCalledTimes(1);
285+
});
286+
287+
it('ensures correct operation order: isReady -> onReady -> processing', async () => {
288+
client = new SegmentClient(clientArgs);
289+
290+
// Track the order of operations
291+
const operationOrder: string[] = [];
292+
293+
// Mock isReady setter
294+
const isReadyDescriptor = Object.getOwnPropertyDescriptor(
295+
client['isReady'],
296+
'value'
297+
);
298+
Object.defineProperty(client['isReady'], 'value', {
299+
...isReadyDescriptor,
300+
set: function (value: boolean) {
301+
if (value === true) {
302+
operationOrder.push('isReady-set-true');
303+
}
304+
isReadyDescriptor?.set?.call(this, value);
305+
},
306+
});
307+
308+
// Mock onReady to track when it's called
309+
const originalOnReady = client['onReady'].bind(client);
310+
client['onReady'] = jest.fn(async () => {
311+
operationOrder.push('onReady-start');
312+
await originalOnReady();
313+
operationOrder.push('onReady-end');
314+
});
315+
316+
// Initialize the client
317+
await client.init();
318+
319+
// Verify the correct order: isReady is set true BEFORE onReady starts
320+
// The expected order should be:
321+
// 1. isReady-set-true
322+
// 2. onReady-start
323+
// 3. onReady-end
324+
expect(operationOrder).toEqual([
325+
'isReady-set-true',
326+
'onReady-start',
327+
'onReady-end',
328+
]);
329+
});
330+
331+
it('does not drop events tracked during onReady processing', async () => {
332+
// This test verifies that events tracked during onReady() processing
333+
// are not lost when the fix is applied (isReady set before onReady)
334+
335+
client = new SegmentClient(clientArgs);
336+
337+
// Track how many events are added as pending
338+
const eventsAddedAsPending: string[] = [];
339+
const originalAddPending = client['store'].pendingEvents.add.bind(
340+
client['store'].pendingEvents
341+
);
342+
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/strict-boolean-expressions, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */
343+
client['store'].pendingEvents.add = jest.fn(async (event: any) => {
344+
const eventName: string = event.event || event.type;
345+
// Only count track events we explicitly send (not auto-tracked events)
346+
if (eventName?.includes('Event')) {
347+
eventsAddedAsPending.push(eventName);
348+
}
349+
return originalAddPending(event);
350+
});
351+
/* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/strict-boolean-expressions, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */
352+
353+
// Mock onReady to track events during its execution
354+
const originalOnReady = client['onReady'].bind(client);
355+
client['onReady'] = jest.fn(async () => {
356+
// Track events DURING onReady processing
357+
// With the fix: these go directly to processing (NOT pending)
358+
// Without fix: these become pending and never get sent
359+
await client.track('Event During OnReady 1');
360+
await client.track('Event During OnReady 2');
361+
362+
// Call original onReady to process initial pending events
363+
await originalOnReady();
364+
});
365+
366+
// Track an event before initialization (this SHOULD always be pending)
367+
await client.track('Event Before Init');
368+
369+
// Initialize the client
370+
await client.init();
371+
372+
// CRITICAL ASSERTION:
373+
// With the fix (isReady = true BEFORE onReady):
374+
// - Only "Event Before Init" is added as pending (count = 1)
375+
// - Events during onReady go directly to processing
376+
// Without the fix (isReady = true AFTER onReady):
377+
// - All 3 events are added as pending (count = 3)
378+
// - Events during onReady become stuck pending events
379+
380+
expect(eventsAddedAsPending).toEqual(['Event Before Init']);
381+
382+
// Double-check: events during onReady should NOT be in pending
383+
expect(eventsAddedAsPending).not.toContain('Event During OnReady 1');
384+
expect(eventsAddedAsPending).not.toContain('Event During OnReady 2');
385+
});
386+
});
387+
/*jshint +W069 */
388+
/* eslint-enable dot-notation */
252389
});

packages/core/src/analytics.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,9 @@ export class SegmentClient {
308308
this.trackDeepLinks(),
309309
]);
310310

311-
await this.onReady();
311+
// Set ready BEFORE processing pending events to prevent race condition
312312
this.isReady.value = true;
313+
await this.onReady();
313314
} catch (error) {
314315
this.reportInternalError(
315316
new SegmentError(

0 commit comments

Comments
 (0)