From 96be7d44dbc5508fb166f35b9b7de69d47bc2b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 17 Apr 2020 15:53:34 +0200 Subject: [PATCH] [RUM] use a lifeCycle event for document unload This change is a first step to decouple the view logic from the Batch implementation. --- packages/core/src/transport.ts | 12 ++++-------- packages/rum/src/lifeCycle.ts | 5 ++++- packages/rum/src/rum.ts | 12 +++++++----- packages/rum/src/viewCollection.ts | 5 ++--- packages/rum/test/viewCollection.spec.ts | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/core/src/transport.ts b/packages/core/src/transport.ts index 45cd0ceae7..8a63a7496a 100644 --- a/packages/core/src/transport.ts +++ b/packages/core/src/transport.ts @@ -1,7 +1,7 @@ import lodashMerge from 'lodash.merge' import { monitor } from './internalMonitoring' -import { Context, DOM_EVENT, jsonStringify, objectValues } from './utils' +import { Context, DOM_EVENT, jsonStringify, noop, objectValues } from './utils' /** * Use POST request without content type to: @@ -45,7 +45,8 @@ export class Batch { private bytesLimit: number, private maxMessageSize: number, private flushTimeout: number, - private contextProvider: () => Context + private contextProvider: () => Context, + private willUnloadCallback: () => void = noop ) { this.flushOnVisibilityHidden() this.flushPeriodically() @@ -160,12 +161,7 @@ export class Batch { * register first to be sure to be called before flush on beforeunload * caveat: unload can still be canceled by another listener */ - window.addEventListener( - DOM_EVENT.BEFORE_UNLOAD, - monitor(() => { - this.beforeFlushOnUnloadHandlers.forEach((handler) => handler()) - }) - ) + window.addEventListener(DOM_EVENT.BEFORE_UNLOAD, monitor(this.willUnloadCallback)) /** * Only event that guarantee to fire on mobile devices when the page transitions to background state diff --git a/packages/rum/src/lifeCycle.ts b/packages/rum/src/lifeCycle.ts index b7c5f90725..6e93a86469 100644 --- a/packages/rum/src/lifeCycle.ts +++ b/packages/rum/src/lifeCycle.ts @@ -11,6 +11,7 @@ export enum LifeCycleEventType { SESSION_RENEWED, RESOURCE_ADDED_TO_BATCH, DOM_MUTATED, + WILL_UNLOAD, } export interface Subscription { @@ -31,6 +32,7 @@ export class LifeCycle { | LifeCycleEventType.SESSION_RENEWED | LifeCycleEventType.RESOURCE_ADDED_TO_BATCH | LifeCycleEventType.DOM_MUTATED + | LifeCycleEventType.WILL_UNLOAD ): void notify(eventType: LifeCycleEventType, data?: any) { const eventCallbacks = this.callbacks[eventType] @@ -55,7 +57,8 @@ export class LifeCycle { | LifeCycleEventType.SESSION_WILL_RENEW | LifeCycleEventType.SESSION_RENEWED | LifeCycleEventType.RESOURCE_ADDED_TO_BATCH - | LifeCycleEventType.DOM_MUTATED, + | LifeCycleEventType.DOM_MUTATED + | LifeCycleEventType.WILL_UNLOAD, callback: () => void ): Subscription subscribe(eventType: LifeCycleEventType, callback: (data?: any) => void) { diff --git a/packages/rum/src/rum.ts b/packages/rum/src/rum.ts index 716fc9b32a..b085cca30e 100644 --- a/packages/rum/src/rum.ts +++ b/packages/rum/src/rum.ts @@ -182,10 +182,11 @@ export function startRum( url: viewContext.location.href, }, }), - () => globalContext + () => globalContext, + () => lifeCycle.notify(LifeCycleEventType.WILL_UNLOAD) ) - trackView(window.location, lifeCycle, batch.upsertRumEvent, batch.beforeFlushOnUnload) + trackView(window.location, lifeCycle, batch.upsertRumEvent) trackErrors(lifeCycle, batch.addRumEvent) trackRequests(configuration, lifeCycle, session, batch.addRumEvent) trackPerformanceTiming(configuration, lifeCycle, batch.addRumEvent) @@ -221,7 +222,8 @@ function startRumBatch( configuration: Configuration, session: RumSession, rumContextProvider: () => Context, - globalContextProvider: () => Context + globalContextProvider: () => Context, + willUnloadCallback: () => void ) { const batch = new Batch( new HttpRequest(configuration.rumEndpoint, configuration.batchBytesLimit, true), @@ -229,7 +231,8 @@ function startRumBatch( configuration.batchBytesLimit, configuration.maxMessageSize, configuration.flushTimeout, - () => lodashMerge(withSnakeCaseKeys(rumContextProvider()), globalContextProvider()) + () => lodashMerge(withSnakeCaseKeys(rumContextProvider()), globalContextProvider()), + willUnloadCallback ) return { addRumEvent: (event: RumEvent, context?: Context) => { @@ -237,7 +240,6 @@ function startRumBatch( batch.add({ ...context, ...withSnakeCaseKeys((event as unknown) as Context) }) } }, - beforeFlushOnUnload: (handler: () => void) => batch.beforeFlushOnUnload(handler), upsertRumEvent: (event: RumEvent, key: string) => { if (session.isTracked()) { batch.upsert(withSnakeCaseKeys((event as unknown) as Context), key) diff --git a/packages/rum/src/viewCollection.ts b/packages/rum/src/viewCollection.ts index e91b2622fd..6719198e4d 100644 --- a/packages/rum/src/viewCollection.ts +++ b/packages/rum/src/viewCollection.ts @@ -31,8 +31,7 @@ let viewMeasures: ViewMeasures export function trackView( location: Location, lifeCycle: LifeCycle, - upsertRumEvent: (event: RumEvent, key: string) => void, - beforeFlushOnUnload: (handler: () => void) => void + upsertRumEvent: (event: RumEvent, key: string) => void ) { const scheduleViewUpdate = throttle(monitor(() => updateView(upsertRumEvent)), THROTTLE_VIEW_UPDATE_PERIOD, { leading: false, @@ -47,7 +46,7 @@ export function trackView( trackTimings(lifeCycle, scheduleViewUpdate) trackRenewSession(location, lifeCycle, resetEventCounts, upsertRumEvent) - beforeFlushOnUnload(() => updateView(upsertRumEvent)) + lifeCycle.subscribe(LifeCycleEventType.WILL_UNLOAD, () => updateView(upsertRumEvent)) } function newView( diff --git a/packages/rum/test/viewCollection.spec.ts b/packages/rum/test/viewCollection.spec.ts index d9c4f1864c..d3c2556467 100644 --- a/packages/rum/test/viewCollection.spec.ts +++ b/packages/rum/test/viewCollection.spec.ts @@ -19,7 +19,7 @@ function setup({ fakeLocation.hash = getHash(url) }) const fakeLocation: Partial = { pathname: '/foo' } - trackView(fakeLocation as Location, lifeCycle || new LifeCycle(), addRumEvent || (() => undefined), () => undefined) + trackView(fakeLocation as Location, lifeCycle || new LifeCycle(), addRumEvent || (() => undefined)) } describe('rum track url change', () => {