From 4226d5ef52d05a115590b91e6631954565dc2bf9 Mon Sep 17 00:00:00 2001 From: Sunita Prajapati Date: Mon, 27 Oct 2025 20:10:22 +0530 Subject: [PATCH 1/2] fix: fixed race condition causing events to be lost during initialization - added unit test cases - fixed the race condition --- packages/core/src/__tests__/analytics.test.ts | 131 ++++++++++++++++++ packages/core/src/analytics.ts | 3 +- 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/packages/core/src/__tests__/analytics.test.ts b/packages/core/src/__tests__/analytics.test.ts index afe3c6b9..1a2a75f8 100644 --- a/packages/core/src/__tests__/analytics.test.ts +++ b/packages/core/src/__tests__/analytics.test.ts @@ -249,4 +249,135 @@ describe('SegmentClient', () => { expect(client.getFlushPolicies().length).toBe(policies.length); }); }); + + describe('Initialization order - race condition fix', () => { + /*jshint -W069 */ + /* eslint-disable dot-notation */ + it('sets isReady to true before executing onReady to prevent events being lost', async () => { + // This test verifies that the race condition fix works: + // isReady is set to true BEFORE onReady() executes, + // so events tracked during onReady() go directly to the queue + // instead of being incorrectly saved as pending events. + + client = new SegmentClient(clientArgs); + + // Track the value of isReady when onReady is called + let isReadyValueInOnReady: boolean | undefined; + + // Mock onReady to capture the isReady state + const originalOnReady = client['onReady'].bind(client); + client['onReady'] = jest.fn(async () => { + // Capture isReady value at the start of onReady + isReadyValueInOnReady = client['isReady'].value; + // Call the original onReady + return originalOnReady(); + }); + + // Initialize the client + await client.init(); + + // Verify that isReady was true when onReady was called + // This is the key fix - isReady is set BEFORE onReady runs + expect(isReadyValueInOnReady).toBe(true); + + // Verify onReady was called + expect(client['onReady']).toHaveBeenCalledTimes(1); + }); + + it('ensures correct operation order: isReady -> onReady -> processing', async () => { + client = new SegmentClient(clientArgs); + + // Track the order of operations + const operationOrder: string[] = []; + + // Mock isReady setter + const isReadyDescriptor = Object.getOwnPropertyDescriptor( + client['isReady'], + 'value' + ); + Object.defineProperty(client['isReady'], 'value', { + ...isReadyDescriptor, + set: function (value: boolean) { + if (value === true) { + operationOrder.push('isReady-set-true'); + } + isReadyDescriptor?.set?.call(this, value); + }, + }); + + // Mock onReady to track when it's called + const originalOnReady = client['onReady'].bind(client); + client['onReady'] = jest.fn(async () => { + operationOrder.push('onReady-start'); + await originalOnReady(); + operationOrder.push('onReady-end'); + }); + + // Initialize the client + await client.init(); + + // Verify the correct order: isReady is set true BEFORE onReady starts + // The expected order should be: + // 1. isReady-set-true + // 2. onReady-start + // 3. onReady-end + expect(operationOrder).toEqual([ + 'isReady-set-true', + 'onReady-start', + 'onReady-end', + ]); + }); + + it('does not drop events tracked during onReady processing', async () => { + // This test verifies that events tracked during onReady() processing + // are not lost when the fix is applied (isReady set before onReady) + + client = new SegmentClient(clientArgs); + + // Track how many events are added as pending + const eventsAddedAsPending: string[] = []; + const originalAddPending = client['store'].pendingEvents.add.bind( + client['store'].pendingEvents + ); + /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/strict-boolean-expressions, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ + client['store'].pendingEvents.add = jest.fn(async (event: any) => { + const eventName: string = event.event || event.type; + // Only count track events we explicitly send (not auto-tracked events) + if (eventName?.includes('Event')) { + eventsAddedAsPending.push(eventName); + } + return originalAddPending(event); + }); + /* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/strict-boolean-expressions, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ + + // Mock onReady to track events during its execution + const originalOnReady = client['onReady'].bind(client); + client['onReady'] = jest.fn(async () => { + // Track events DURING onReady processing + // With the fix: these go directly to processing (NOT pending) + // Without fix: these become pending and never get sent + await client.track('Event During OnReady 1'); + await client.track('Event During OnReady 2'); + + // Call original onReady to process initial pending events + await originalOnReady(); + }); + + // Track an event before initialization (this SHOULD always be pending) + await client.track('Event Before Init'); + + // Initialize the client + await client.init(); + + // CRITICAL ASSERTION: + // With the fix (isReady = true BEFORE onReady): + // - Only "Event Before Init" is added as pending (count = 1) + // - Events during onReady go directly to processing + // Without the fix (isReady = true AFTER onReady): + // - All 3 events are added as pending (count = 3) + // - Events during onReady become stuck pending events + + expect(eventsAddedAsPending).toEqual(['Event Before Init']); + }); + }); }); diff --git a/packages/core/src/analytics.ts b/packages/core/src/analytics.ts index 9f4ec731..22942f75 100644 --- a/packages/core/src/analytics.ts +++ b/packages/core/src/analytics.ts @@ -307,9 +307,8 @@ export class SegmentClient { // check if the app was opened from a deep link this.trackDeepLinks(), ]); - - await this.onReady(); this.isReady.value = true; + await this.onReady(); } catch (error) { this.reportInternalError( new SegmentError( From a85f29f241a24ab7ec359dcd71965613ba0476f3 Mon Sep 17 00:00:00 2001 From: Sunita Prajapati Date: Thu, 6 Nov 2025 13:35:35 +0530 Subject: [PATCH 2/2] fix: race condition to prevent loss of events during initialisation - refactored onReady() - removed unused tests which were written for wrong fix --- packages/core/src/__tests__/analytics.test.ts | 131 ------------------ .../src/__tests__/methods/process.test.ts | 2 + packages/core/src/analytics.ts | 19 +-- 3 files changed, 13 insertions(+), 139 deletions(-) diff --git a/packages/core/src/__tests__/analytics.test.ts b/packages/core/src/__tests__/analytics.test.ts index 1a2a75f8..afe3c6b9 100644 --- a/packages/core/src/__tests__/analytics.test.ts +++ b/packages/core/src/__tests__/analytics.test.ts @@ -249,135 +249,4 @@ describe('SegmentClient', () => { expect(client.getFlushPolicies().length).toBe(policies.length); }); }); - - describe('Initialization order - race condition fix', () => { - /*jshint -W069 */ - /* eslint-disable dot-notation */ - it('sets isReady to true before executing onReady to prevent events being lost', async () => { - // This test verifies that the race condition fix works: - // isReady is set to true BEFORE onReady() executes, - // so events tracked during onReady() go directly to the queue - // instead of being incorrectly saved as pending events. - - client = new SegmentClient(clientArgs); - - // Track the value of isReady when onReady is called - let isReadyValueInOnReady: boolean | undefined; - - // Mock onReady to capture the isReady state - const originalOnReady = client['onReady'].bind(client); - client['onReady'] = jest.fn(async () => { - // Capture isReady value at the start of onReady - isReadyValueInOnReady = client['isReady'].value; - // Call the original onReady - return originalOnReady(); - }); - - // Initialize the client - await client.init(); - - // Verify that isReady was true when onReady was called - // This is the key fix - isReady is set BEFORE onReady runs - expect(isReadyValueInOnReady).toBe(true); - - // Verify onReady was called - expect(client['onReady']).toHaveBeenCalledTimes(1); - }); - - it('ensures correct operation order: isReady -> onReady -> processing', async () => { - client = new SegmentClient(clientArgs); - - // Track the order of operations - const operationOrder: string[] = []; - - // Mock isReady setter - const isReadyDescriptor = Object.getOwnPropertyDescriptor( - client['isReady'], - 'value' - ); - Object.defineProperty(client['isReady'], 'value', { - ...isReadyDescriptor, - set: function (value: boolean) { - if (value === true) { - operationOrder.push('isReady-set-true'); - } - isReadyDescriptor?.set?.call(this, value); - }, - }); - - // Mock onReady to track when it's called - const originalOnReady = client['onReady'].bind(client); - client['onReady'] = jest.fn(async () => { - operationOrder.push('onReady-start'); - await originalOnReady(); - operationOrder.push('onReady-end'); - }); - - // Initialize the client - await client.init(); - - // Verify the correct order: isReady is set true BEFORE onReady starts - // The expected order should be: - // 1. isReady-set-true - // 2. onReady-start - // 3. onReady-end - expect(operationOrder).toEqual([ - 'isReady-set-true', - 'onReady-start', - 'onReady-end', - ]); - }); - - it('does not drop events tracked during onReady processing', async () => { - // This test verifies that events tracked during onReady() processing - // are not lost when the fix is applied (isReady set before onReady) - - client = new SegmentClient(clientArgs); - - // Track how many events are added as pending - const eventsAddedAsPending: string[] = []; - const originalAddPending = client['store'].pendingEvents.add.bind( - client['store'].pendingEvents - ); - /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/strict-boolean-expressions, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ - client['store'].pendingEvents.add = jest.fn(async (event: any) => { - const eventName: string = event.event || event.type; - // Only count track events we explicitly send (not auto-tracked events) - if (eventName?.includes('Event')) { - eventsAddedAsPending.push(eventName); - } - return originalAddPending(event); - }); - /* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/strict-boolean-expressions, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment */ - - // Mock onReady to track events during its execution - const originalOnReady = client['onReady'].bind(client); - client['onReady'] = jest.fn(async () => { - // Track events DURING onReady processing - // With the fix: these go directly to processing (NOT pending) - // Without fix: these become pending and never get sent - await client.track('Event During OnReady 1'); - await client.track('Event During OnReady 2'); - - // Call original onReady to process initial pending events - await originalOnReady(); - }); - - // Track an event before initialization (this SHOULD always be pending) - await client.track('Event Before Init'); - - // Initialize the client - await client.init(); - - // CRITICAL ASSERTION: - // With the fix (isReady = true BEFORE onReady): - // - Only "Event Before Init" is added as pending (count = 1) - // - Events during onReady go directly to processing - // Without the fix (isReady = true AFTER onReady): - // - All 3 events are added as pending (count = 3) - // - Events during onReady become stuck pending events - - expect(eventsAddedAsPending).toEqual(['Event Before Init']); - }); - }); }); diff --git a/packages/core/src/__tests__/methods/process.test.ts b/packages/core/src/__tests__/methods/process.test.ts index 1b8beb58..69f22287 100644 --- a/packages/core/src/__tests__/methods/process.test.ts +++ b/packages/core/src/__tests__/methods/process.test.ts @@ -68,6 +68,8 @@ describe('process', () => { jest.spyOn(client.isReady, 'value', 'get').mockReturnValue(true); // @ts-ignore await client.onReady(); + // @ts-ignore + await client.processPendingEvents(); expectedEvent = { ...expectedEvent, context: { ...store.context.get() }, diff --git a/packages/core/src/analytics.ts b/packages/core/src/analytics.ts index 22942f75..1ded23b6 100644 --- a/packages/core/src/analytics.ts +++ b/packages/core/src/analytics.ts @@ -307,8 +307,13 @@ export class SegmentClient { // check if the app was opened from a deep link this.trackDeepLinks(), ]); - this.isReady.value = true; await this.onReady(); + this.isReady.value = true; + + // Process all pending events + await this.processPendingEvents(); + // Trigger manual flush + this.flushPolicyExecuter.manualFlush(); } catch (error) { this.reportInternalError( new SegmentError( @@ -560,15 +565,13 @@ export class SegmentClient { // Start flush policies // This should be done before any pending events are added to the queue so that any policies that rely on events queued can trigger accordingly this.setupFlushPolicies(); - - // Send all events in the queue + } + private async processPendingEvents() { const pending = await this.store.pendingEvents.get(true); - for (const e of pending) { - await this.startTimelineProcessing(e); - await this.store.pendingEvents.remove(e); + for (const event of pending) { + await this.startTimelineProcessing(event); + await this.store.pendingEvents.remove(event); } - - this.flushPolicyExecuter.manualFlush(); } async flush(): Promise {