fix: Prevent double import of Session Replay aggregate class (#1558)

Author: metal-messiah

src/common/constants/agent-constants.js

@@ -5,3 +5,4 @@
 export const IDEAL_PAYLOAD_SIZE = 16000
 export const MAX_PAYLOAD_SIZE = 1000000
 export const DEFAULT_KEY = 'NR_CONTAINER_AGENT'
+export const SESSION_ERROR = 'SESSION_ERROR'

src/features/session_replay/aggregate/index.js

@@ -43,8 +43,10 @@ export class Aggregate extends AggregateBase {
     /** set at BCS response, stored in runtime */
     this.timeKeeper = undefined
 
-    this.recorder = args?.recorder
-    this.errorNoticed = args?.errorNoticed || false
+    this.instrumentClass = args
+    // point this var here just in case it already exists and can be used by APIs (session pause, resume, etc.) before handling rum flags
+    this.recorder = this.instrumentClass?.recorder
+
     this.harvestOpts.raw = true
 
     this.isSessionTrackingEnabled = canEnableSessionTracking(agentRef.init) && !!agentRef.runtime.session
@@ -64,7 +66,7 @@ export class Aggregate extends AggregateBase {
       // if the mode changed on a different tab, it needs to update this instance to match
       this.mode = agentRef.runtime.session.state.sessionReplayMode
       if (!this.initialized || this.mode === MODE.OFF) return
-      this.recorder?.startRecording()
+      this.recorder?.startRecording(TRIGGERS.RESUME, this.mode)
     })
 
     this.ee.on(SESSION_EVENTS.UPDATE, (type, data) => {
@@ -131,7 +133,7 @@ export class Aggregate extends AggregateBase {
     this.mode = MODE.FULL
     // if the error was noticed AFTER the recorder was already imported....
     if (this.recorder && this.initialized) {
-      if (!this.agentRef.runtime.isRecording) this.recorder.startRecording()
+      if (!this.agentRef.runtime.isRecording) this.recorder.startRecording(TRIGGERS.SWITCH_TO_FULL, this.mode) // off --> full
       this.syncWithSessionManager({ sessionReplayMode: this.mode })
     } else {
       this.initializeRecording(MODE.FULL, true)
@@ -142,9 +144,10 @@ export class Aggregate extends AggregateBase {
    * Evaluate entitlements and sampling before starting feature mechanics, importing and configuring recording library, and setting storage state
    * @param {boolean} srMode - the true/false state of the "sr" flag (aka. entitlements) from RUM response
    * @param {boolean} ignoreSession - whether to force the method to ignore the session state and use just the sample flags
+   * @param {TRIGGERS} [trigger=TRIGGERS.INITIALIZE] - the trigger that initiated the recording.  Usually TRIGGERS.INITIALIZE, but could be TRIGGERS.API since in certain cases that trigger calls this method
    * @returns {void}
    */
-  async initializeRecording (srMode, ignoreSession) {
+  async initializeRecording (srMode, ignoreSession, trigger = TRIGGERS.INITIALIZE) {
     this.initialized = true
     if (!this.entitled) return
 
@@ -156,7 +159,8 @@ export class Aggregate extends AggregateBase {
     // session replays can continue if already in progress
     const { session, timeKeeper } = this.agentRef.runtime
     this.timeKeeper = timeKeeper
-    if (this.recorder?.parent.trigger === TRIGGERS.API && this.agentRef.runtime.isRecording) {
+
+    if (this.recorder?.trigger === TRIGGERS.API && this.agentRef.runtime.isRecording) {
       this.mode = MODE.FULL
     } else if (!session.isNew && !ignoreSession) { // inherit the mode of the existing session
       this.mode = session.state.sessionReplayMode
@@ -167,29 +171,24 @@ export class Aggregate extends AggregateBase {
     // If off, then don't record (early return)
     if (this.mode === MODE.OFF) return
 
-    if (!this.recorder) {
-      try {
-        // Do not change the webpackChunkName or it will break the webpack nrba-chunking plugin
-        const { Recorder } = (await import(/* webpackChunkName: "recorder" */'../shared/recorder'))
-        this.recorder = new Recorder(this)
-        this.recorder.events.hasError = this.errorNoticed
-      } catch (err) {
-        return this.abort(ABORT_REASONS.IMPORT)
-      }
-    } else {
-      this.recorder.parent = this
+    try {
+      /** will return a recorder instance if already imported, otherwise, will fetch the recorder and initialize it */
+      this.recorder ??= await this.instrumentClass.importRecorder()
+    } catch (err) {
+      /** if the recorder fails to import, abort the feature */
+      return this.abort(ABORT_REASONS.IMPORT, err)
     }
 
     // If an error was noticed before the mode could be set (like in the early lifecycle of the page), immediately set to FULL mode
-    if (this.mode === MODE.ERROR && this.errorNoticed) this.mode = MODE.FULL
+    if (this.mode === MODE.ERROR && this.instrumentClass.errorNoticed) { this.mode = MODE.FULL }
 
     // FULL mode records AND reports from the beginning, while ERROR mode only records (but does not report).
     // ERROR mode will do this until an error is thrown, and then switch into FULL mode.
     // The makeHarvestPayload should ensure that no payload is returned if we're not in FULL mode...
 
     await this.prepUtils()
 
-    if (!this.agentRef.runtime.isRecording) this.recorder.startRecording()
+    if (!this.agentRef.runtime.isRecording) this.recorder.startRecording(trigger, this.mode)
 
     this.syncWithSessionManager({ sessionReplayMode: this.mode })
   }

src/features/session_replay/constants.js

@@ -54,5 +54,9 @@ export const ABORT_REASONS = {
 export const QUERY_PARAM_PADDING = 5000
 
 export const TRIGGERS = {
-  API: 'api'
+  API: 'api',
+  RESUME: 'resume',
+  SWITCH_TO_FULL: 'switchToFull',
+  INITIALIZE: 'initialize',
+  PRELOAD: 'preload'
 }

src/features/session_replay/instrument/index.js

@@ -16,9 +16,11 @@ import { setupPauseReplayAPI } from '../../../loaders/api/pauseReplay'
 
 export class Instrument extends InstrumentBase {
   static featureName = FEATURE_NAME
+  /** @type {Promise|undefined} A promise that resolves when the recorder module is imported and added to the class. Undefined if the recorder has never been staged to import with `importRecorder`. */
+  #stagedImport
+  /** The RRWEB recorder instance, if imported */
+  recorder
 
-  #mode
-  #agentRef
   constructor (agentRef) {
     super(agentRef, FEATURE_NAME)
 
@@ -27,7 +29,6 @@ export class Instrument extends InstrumentBase {
     setupPauseReplayAPI(agentRef)
 
     let session
-    this.#agentRef = agentRef
     try {
       session = JSON.parse(localStorage.getItem(`${PREFIX}_${DEFAULT_KEY}`))
     } catch (err) { }
@@ -37,15 +38,17 @@ export class Instrument extends InstrumentBase {
     }
 
     if (this.#canPreloadRecorder(session)) {
-      this.#mode = session?.sessionReplayMode
-      this.#preloadStartRecording()
-    } else {
-      this.importAggregator(this.#agentRef, () => import(/* webpackChunkName: "session_replay-aggregate" */ '../aggregate'))
+      this.importRecorder().then(recorder => {
+        recorder.startRecording(TRIGGERS.PRELOAD, session?.sessionReplayMode)
+      }) // could handle specific fail-state behaviors with a .catch block here
     }
 
+    this.importAggregator(this.agentRef, () => import(/* webpackChunkName: "session_replay-aggregate" */ '../aggregate'), this)
+
     /** If the recorder is running, we can pass error events on to the agg to help it switch to full mode later */
     this.ee.on('err', (e) => {
-      if (this.#agentRef.runtime.isRecording) {
+      if (this.blocked) return
+      if (this.agentRef.runtime.isRecording) {
         this.errorNoticed = true
         handle(SR_EVENT_EMITTER_TYPES.ERROR_DURING_REPLAY, [e], undefined, this.featureName, this.ee)
       }
@@ -57,54 +60,50 @@ export class Instrument extends InstrumentBase {
     if (!session) { // this might be a new session if entity initializes: conservatively start recording if first-time config allows
       // Note: users with SR enabled, as well as these other configs enabled by-default, will be penalized by the recorder overhead EVEN IF they don't actually have or get
       // entitlement or sampling decision, or otherwise intentionally opted-in for the feature.
-      return isPreloadAllowed(this.#agentRef.init)
+      return isPreloadAllowed(this.agentRef.init)
     } else if (session.sessionReplayMode === MODE.FULL || session.sessionReplayMode === MODE.ERROR) {
       return true // existing sessions get to continue recording, regardless of this page's configs or if it has expired (conservatively)
     } else { // SR mode was OFF but may potentially be turned on if session resets and configs allows the new session to have replay...
-      return isPreloadAllowed(this.#agentRef.init)
+      return isPreloadAllowed(this.agentRef.init)
     }
   }
 
-  #alreadyStarted = false
   /**
-   * This func is use for early pre-load recording prior to replay feature (agg) being loaded onto the page. It should only setup once, including if already called and in-progress.
+   * Returns a promise that imports the recorder module. Only lets the recorder module be imported and instantiated once. Rejects if failed to import/instantiate.
+   * @returns {Promise}
    */
-  async #preloadStartRecording (trigger) {
-    if (this.#alreadyStarted) return
-    this.#alreadyStarted = true
-
-    try {
-      const { Recorder } = (await import(/* webpackChunkName: "recorder" */'../shared/recorder'))
+  importRecorder () {
+    /** if we already have a recorder fully set up, just return it */
+    if (this.recorder) return Promise.resolve(this.recorder)
+    /** conditional -- if we have never started importing, stage the import and store it in state */
+    this.#stagedImport ??= import(/* webpackChunkName: "recorder" */'../shared/recorder')
+      .then(({ Recorder }) => {
+        this.recorder = new Recorder(this)
+        /** return the recorder for promise chaining */
+        return this.recorder
+      })
+      .catch(err => {
+        this.ee.emit('internal-error', [err])
+        this.blocked = true
+        /** return the err for promise chaining */
+        throw err
+      })
 
-      // If startReplay() has been used by this point, we must record in full mode regardless of session preload:
-      // Note: recorder starts here with w/e the mode is at this time, but this may be changed later (see #apiStartOrRestartReplay else-case)
-      this.recorder ??= new Recorder({ ...this, mode: this.#mode, agentRef: this.#agentRef, trigger, timeKeeper: this.#agentRef.runtime.timeKeeper }) // if TK exists due to deferred state, pass it
-      this.recorder.startRecording()
-      this.abortHandler = this.recorder.stopRecording
-    } catch (err) {
-      this.parent.ee.emit('internal-error', [err])
-    }
-    this.importAggregator(this.#agentRef, () => import(/* webpackChunkName: "session_replay-aggregate" */ '../aggregate'), { recorder: this.recorder, errorNoticed: this.errorNoticed })
+    return this.#stagedImport
   }
 
   /**
    * Called whenever startReplay API is used. That could occur any time, pre or post load.
    */
   #apiStartOrRestartReplay () {
+    if (this.blocked) return
     if (this.featAggregate) { // post-load; there's possibly already an ongoing recording
-      if (this.featAggregate.mode !== MODE.FULL) this.featAggregate.initializeRecording(MODE.FULL, true)
-    } else { // pre-load
-      this.#mode = MODE.FULL
-      this.#preloadStartRecording(TRIGGERS.API)
-      // There's a race here wherein either:
-      // a. Recorder has not been initialized, and we've set the enforced mode, so we're good, or;
-      // b. Record has been initialized, possibly with the "wrong" mode, so we have to correct that + restart.
-      if (this.recorder && this.recorder.parent.mode !== MODE.FULL) {
-        this.recorder.parent.mode = MODE.FULL
-        this.recorder.stopRecording()
-        this.recorder.startRecording()
-        this.abortHandler = this.recorder.stopRecording
-      }
+      if (this.featAggregate.mode !== MODE.FULL) this.featAggregate.initializeRecording(MODE.FULL, true, TRIGGERS.API)
+    } else {
+      this.importRecorder()
+        .then(() => {
+          this.recorder.startRecording(TRIGGERS.API, MODE.FULL)
+        }) // could handle specific fail-state behaviors with a .catch block here
     }
   }
 }