Skip to content

Commit 99a4ca7

Browse files
authored
[FSSDK-11879] flush events without closing client on page unload (#1087)
Currently, in browser, when the page is unloaded, the sdk instance is being closed in order to flush pending events. But when the page is loaded from [bfcache](https://developer.mozilla.org/en-US/docs/Glossary/bfcache), the sdk instance stays closed as bfcache restores the Javascript heap as well, which causes further events to be not processed. This PR updates the code to flush events on page unload without closing the sdk instance.
1 parent ccd6adb commit 99a4ca7

15 files changed

+230
-23
lines changed

lib/client_factory.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export type OptimizelyFactoryConfig = Config & {
3131
requestHandler: RequestHandler;
3232
}
3333

34-
export const getOptimizelyInstance = (config: OptimizelyFactoryConfig): Client => {
34+
export const getOptimizelyInstance = (config: OptimizelyFactoryConfig): Optimizely => {
3535
const {
3636
clientEngine,
3737
clientVersion,

lib/event_processor/batch_event_processor.spec.ts

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -875,7 +875,7 @@ describe('BatchEventProcessor', async () => {
875875
});
876876

877877
describe('retryFailedEvents', () => {
878-
it('should disptach only failed events from the store and not dispatch queued events', async () => {
878+
it('should dispatch only failed events from the store and not dispatch queued events', async () => {
879879
const eventDispatcher = getMockDispatcher();
880880
const mockDispatch: MockInstance<typeof eventDispatcher.dispatchEvent> = eventDispatcher.dispatchEvent;
881881
mockDispatch.mockResolvedValue({});
@@ -921,7 +921,7 @@ describe('BatchEventProcessor', async () => {
921921
]));
922922
});
923923

924-
it('should disptach only failed events from the store and not dispatch events that are being dispatched', async () => {
924+
it('should dispatch only failed events from the store and not dispatch events that are being dispatched', async () => {
925925
const eventDispatcher = getMockDispatcher();
926926
const mockDispatch: MockInstance<typeof eventDispatcher.dispatchEvent> = eventDispatcher.dispatchEvent;
927927
const mockResult1 = resolvablePromise();
@@ -977,7 +977,7 @@ describe('BatchEventProcessor', async () => {
977977
]));
978978
});
979979

980-
it('should disptach events in correct batch size and separate events with differnt contexts in separate batch', async () => {
980+
it('should dispatch events in correct batch size and separate events with differnt contexts in separate batch', async () => {
981981
const eventDispatcher = getMockDispatcher();
982982
const mockDispatch: MockInstance<typeof eventDispatcher.dispatchEvent> = eventDispatcher.dispatchEvent;
983983
mockDispatch.mockResolvedValue({});
@@ -1023,7 +1023,7 @@ describe('BatchEventProcessor', async () => {
10231023
});
10241024

10251025
describe('when failedEventRepeater is fired', () => {
1026-
it('should disptach only failed events from the store and not dispatch queued events', async () => {
1026+
it('should dispatch only failed events from the store and not dispatch queued events', async () => {
10271027
const eventDispatcher = getMockDispatcher();
10281028
const mockDispatch: MockInstance<typeof eventDispatcher.dispatchEvent> = eventDispatcher.dispatchEvent;
10291029
mockDispatch.mockResolvedValue({});
@@ -1071,7 +1071,7 @@ describe('BatchEventProcessor', async () => {
10711071
]));
10721072
});
10731073

1074-
it('should disptach only failed events from the store and not dispatch events that are being dispatched', async () => {
1074+
it('should dispatch only failed events from the store and not dispatch events that are being dispatched', async () => {
10751075
const eventDispatcher = getMockDispatcher();
10761076
const mockDispatch: MockInstance<typeof eventDispatcher.dispatchEvent> = eventDispatcher.dispatchEvent;
10771077
const mockResult1 = resolvablePromise();
@@ -1129,7 +1129,7 @@ describe('BatchEventProcessor', async () => {
11291129
]));
11301130
});
11311131

1132-
it('should disptach events in correct batch size and separate events with differnt contexts in separate batch', async () => {
1132+
it('should dispatch events in correct batch size and separate events with differnt contexts in separate batch', async () => {
11331133
const eventDispatcher = getMockDispatcher();
11341134
const mockDispatch: MockInstance<typeof eventDispatcher.dispatchEvent> = eventDispatcher.dispatchEvent;
11351135
mockDispatch.mockResolvedValue({});
@@ -1277,7 +1277,7 @@ describe('BatchEventProcessor', async () => {
12771277
expect(failedEventRepeater.stop).toHaveBeenCalledOnce();
12781278
});
12791279

1280-
it('should disptach the events in queue using the closing dispatcher if available', async () => {
1280+
it('should dispatch the events in queue using the closing dispatcher if available', async () => {
12811281
const eventDispatcher = getMockDispatcher();
12821282
const closingEventDispatcher = getMockDispatcher();
12831283
closingEventDispatcher.dispatchEvent.mockResolvedValue({});
@@ -1408,4 +1408,76 @@ describe('BatchEventProcessor', async () => {
14081408
await expect(processor.onTerminated()).resolves.not.toThrow();
14091409
});
14101410
});
1411+
1412+
describe('flushImmediately', () => {
1413+
it('should dispatch the events in queue using the closing dispatcher if available', async () => {
1414+
const eventDispatcher = getMockDispatcher();
1415+
const closingEventDispatcher = getMockDispatcher();
1416+
closingEventDispatcher.dispatchEvent.mockResolvedValue({});
1417+
1418+
const dispatchRepeater = getMockRepeater();
1419+
const failedEventRepeater = getMockRepeater();
1420+
1421+
const processor = new BatchEventProcessor({
1422+
eventDispatcher,
1423+
closingEventDispatcher,
1424+
dispatchRepeater,
1425+
failedEventRepeater,
1426+
batchSize: 100,
1427+
});
1428+
1429+
processor.start();
1430+
await processor.onRunning();
1431+
1432+
const events: ProcessableEvent[] = [];
1433+
for(let i = 0; i < 10; i++) {
1434+
const event = createImpressionEvent(`id-${i}`);
1435+
events.push(event);
1436+
await processor.process(event);
1437+
}
1438+
1439+
expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(0);
1440+
expect(closingEventDispatcher.dispatchEvent).toHaveBeenCalledTimes(0);
1441+
1442+
processor.flushImmediately();
1443+
expect(closingEventDispatcher.dispatchEvent).toHaveBeenCalledTimes(1);
1444+
expect(closingEventDispatcher.dispatchEvent).toHaveBeenCalledWith(buildLogEvent(events));
1445+
1446+
expect(processor.isRunning()).toBe(true);
1447+
});
1448+
1449+
1450+
it('should dispatch the events in queue using eventDispatcher if closingEventDispatcher is not available', async () => {
1451+
const eventDispatcher = getMockDispatcher();
1452+
eventDispatcher.dispatchEvent.mockResolvedValue({});
1453+
1454+
const dispatchRepeater = getMockRepeater();
1455+
const failedEventRepeater = getMockRepeater();
1456+
1457+
const processor = new BatchEventProcessor({
1458+
eventDispatcher,
1459+
dispatchRepeater,
1460+
failedEventRepeater,
1461+
batchSize: 100,
1462+
});
1463+
1464+
processor.start();
1465+
await processor.onRunning();
1466+
1467+
const events: ProcessableEvent[] = [];
1468+
for(let i = 0; i < 10; i++) {
1469+
const event = createImpressionEvent(`id-${i}`);
1470+
events.push(event);
1471+
await processor.process(event);
1472+
}
1473+
1474+
expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(0);
1475+
1476+
processor.flushImmediately();
1477+
expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(1);
1478+
expect(eventDispatcher.dispatchEvent).toHaveBeenCalledWith(buildLogEvent(events));
1479+
1480+
expect(processor.isRunning()).toBe(true);
1481+
});
1482+
});
14111483
});

lib/event_processor/batch_event_processor.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,14 +230,14 @@ export class BatchEventProcessor extends BaseService implements EventProcessor {
230230
});
231231
}
232232

233-
private async flush(closing = false): Promise<void> {
233+
private async flush(useClosingDispatcher = false): Promise<void> {
234234
const batch = this.createNewBatch();
235235
if (!batch) {
236236
return;
237237
}
238238

239239
this.dispatchRepeater.reset();
240-
this.dispatchBatch(batch, closing);
240+
this.dispatchBatch(batch, useClosingDispatcher);
241241
}
242242

243243
async process(event: ProcessableEvent): Promise<void> {
@@ -332,6 +332,13 @@ export class BatchEventProcessor extends BaseService implements EventProcessor {
332332
}
333333
}
334334

335+
flushImmediately(): Promise<unknown> {
336+
if (!this.isRunning()) {
337+
return Promise.resolve();
338+
}
339+
return this.flush(true);
340+
}
341+
335342
stop(): void {
336343
if (this.isDone()) {
337344
return;

lib/event_processor/event_builder/log_event.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ function makeVisitor(data: ImpressionEvent | ConversionEvent): Visitor {
222222

223223
export function buildLogEvent(events: UserEvent[]): LogEvent {
224224
const region = events[0]?.context.region || 'US';
225-
const url = logxEndpoint[region];
225+
const url = logxEndpoint[region] || logxEndpoint['US'];
226226

227227
return {
228228
url,

lib/event_processor/event_processor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,5 @@ export interface EventProcessor extends Service {
2828
process(event: ProcessableEvent): Promise<unknown>;
2929
onDispatch(handler: Consumer<LogEvent>): Fn;
3030
setLogger(logger: LoggerFacade): void;
31+
flushImmediately(): Promise<unknown>;
3132
}

lib/event_processor/forwarding_event_processor.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,8 @@ export class ForwardingEventProcessor extends BaseService implements EventProces
6969
onDispatch(handler: Consumer<LogEvent>): Fn {
7070
return this.eventEmitter.on('dispatch', handler);
7171
}
72+
73+
flushImmediately(): Promise<unknown> {
74+
return Promise.resolve();
75+
}
7276
}

lib/index.browser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export const createInstance = function(config: Config): Client {
3737
window.addEventListener(
3838
unloadEvent,
3939
() => {
40-
client.close();
40+
client.flushImmediately();
4141
},
4242
);
4343
}

lib/odp/event_manager/odp_event_manager.spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,4 +1010,45 @@ describe('DefaultOdpEventManager', () => {
10101010
await expect(odpEventManager.onTerminated()).resolves.not.toThrow();
10111011
expect(odpEventManager.getState()).toBe(ServiceState.Terminated);
10121012
});
1013+
1014+
it('should flush the queue when flushImmediately() is called in running state', async () => {
1015+
const repeater = getMockRepeater();
1016+
1017+
const apiManager = getMockApiManager();
1018+
apiManager.sendEvents.mockResolvedValue({ statusCode: 200 });
1019+
1020+
const odpEventManager = new DefaultOdpEventManager({
1021+
repeater: repeater,
1022+
apiManager: apiManager,
1023+
batchSize: 30,
1024+
retryConfig: {
1025+
maxRetries: 3,
1026+
backoffProvider: vi.fn(),
1027+
},
1028+
});
1029+
1030+
odpEventManager.updateConfig({
1031+
integrated: true,
1032+
odpConfig: config,
1033+
});
1034+
1035+
odpEventManager.start();
1036+
await expect(odpEventManager.onRunning()).resolves.not.toThrow();
1037+
1038+
const events: OdpEvent[] = [];
1039+
for(let i = 0; i < 10; i++) {
1040+
events.push(makeEvent(i));
1041+
odpEventManager.sendEvent(events[i]);
1042+
}
1043+
1044+
await exhaustMicrotasks();
1045+
expect(apiManager.sendEvents).not.toHaveBeenCalled();
1046+
1047+
odpEventManager.flushImmediately();
1048+
await exhaustMicrotasks();
1049+
1050+
expect(apiManager.sendEvents).toHaveBeenCalledTimes(1);
1051+
expect(apiManager.sendEvents).toHaveBeenCalledWith(config, events);
1052+
expect(odpEventManager.isRunning()).toBe(true);
1053+
});
10131054
});

lib/odp/event_manager/odp_event_manager.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export interface OdpEventManager extends Service {
4242
updateConfig(odpIntegrationConfig: OdpIntegrationConfig): void;
4343
sendEvent(event: OdpEvent): void;
4444
setLogger(logger: LoggerFacade): void;
45+
flushImmediately(): Promise<unknown>;
4546
}
4647

4748
export type RetryConfig = {
@@ -160,6 +161,13 @@ export class DefaultOdpEventManager extends BaseService implements OdpEventManag
160161
this.startPromise.resolve();
161162
}
162163

164+
flushImmediately(): Promise<unknown> {
165+
if (!this.isRunning()) {
166+
return Promise.resolve();
167+
}
168+
return this.flush();
169+
}
170+
163171
stop(): void {
164172
if (this.isDone()) {
165173
return;

lib/odp/odp_manager.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const getMockOdpEventManager = () => {
5353
sendEvent: vi.fn(),
5454
makeDisposable: vi.fn(),
5555
setLogger: vi.fn(),
56+
flushImmediately: vi.fn(),
5657
};
5758
};
5859

@@ -780,6 +781,29 @@ describe('DefaultOdpManager', () => {
780781
odpManager.makeDisposable();
781782

782783
expect(eventManager.makeDisposable).toHaveBeenCalled();
784+
});
785+
786+
it('should call flushImmediately() on eventManager when flushImmediately() is called on odpManager', async () => {
787+
const eventManager = getMockOdpEventManager();
788+
eventManager.onRunning.mockResolvedValue({});
789+
const segmentManager = getMockOdpSegmentManager();
790+
791+
eventManager.flushImmediately.mockResolvedValue({});
792+
793+
const odpManager = new DefaultOdpManager({
794+
segmentManager,
795+
eventManager,
796+
});
797+
798+
odpManager.updateConfig({ integrated: true, odpConfig: config });
799+
odpManager.start();
800+
801+
await odpManager.onRunning();
802+
803+
odpManager.flushImmediately();
804+
805+
expect(eventManager.flushImmediately).toHaveBeenCalledOnce();
806+
expect(odpManager.isRunning()).toBe(true);
783807
})
784808
});
785809

0 commit comments

Comments
 (0)