feat(sesssion replay): pr feedback

Author: Kelly Wallach

packages/session-replay-browser/src/session-replay.ts

@@ -197,7 +197,7 @@ export class SessionReplay implements AmplitudeSessionReplay {
     // Restart recording on focus to ensure that when user
     // switches tabs, we take a full snapshot
     this.stopRecordingEvents();
-    this.captureEventsIfShould();
+    this.captureEvents();
   };
 
   /**
@@ -239,7 +239,7 @@ export class SessionReplay implements AmplitudeSessionReplay {
       });
     }
 
-    this.captureEventsIfShould();
+    this.captureEvents();
   };
 
   sendEvents(sessionId?: number) {
@@ -288,12 +288,10 @@ export class SessionReplay implements AmplitudeSessionReplay {
           `Not capturing replays for session ${this.identifiers.sessionId} due to not matching targeting conditions.`,
         );
         return false;
-      } else {
-        this.loggerProvider.log(
-          `Capturing replays for session ${this.identifiers.sessionId} due to matching targeting conditions.`,
-        );
-        return true;
       }
+      this.loggerProvider.log(
+        `Capturing replays for session ${this.identifiers.sessionId} due to matching targeting conditions.`,
+      );
     } else {
       const isInSample = isSessionInSample(this.identifiers.sessionId, this.config.sampleRate);
       if (!isInSample) {
@@ -334,17 +332,18 @@ export class SessionReplay implements AmplitudeSessionReplay {
     return maskSelector as unknown as string;
   }
 
-  captureEventsIfShould() {
-    const shouldRecord = this.getShouldCapture();
-    const sessionId = this.identifiers?.sessionId;
-    if (!shouldRecord || !sessionId || !this.config) {
-      return;
-    }
+  captureEvents() {
     if (this.recordCancelCallback) {
       this.loggerProvider.debug('captureEvents method fired - Session Replay capture already in progress.');
       return;
     }
 
+    const shouldRecord = this.getShouldCapture();
+    const sessionId = this.identifiers && this.identifiers.sessionId;
+    if (!shouldRecord || !sessionId || !this.config) {
+      return;
+    }
+
     const privacyConfig = this.config.privacyConfig;
 
     this.loggerProvider.log('Session Replay capture beginning.');

packages/session-replay-browser/test/session-replay.test.ts

@@ -698,15 +698,15 @@ describe('SessionReplay', () => {
     });
   });
 
-  describe('captureEventsIfShould', () => {
+  describe('captureEvents', () => {
     test('should return early if no config', async () => {
       await sessionReplay.init(apiKey, mockOptions).promise;
       const createEventsIDBStoreInstance = await (SessionReplayIDB.createEventsIDBStore as jest.Mock).mock.results[0]
         .value;
 
       record.mockReset();
       sessionReplay.config = undefined;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       expect(record).not.toHaveBeenCalled();
       if (!sessionReplay.eventsManager) {
         throw new Error('Did not call init');
@@ -718,7 +718,7 @@ describe('SessionReplay', () => {
       await sessionReplay.init(apiKey, mockOptions).promise;
       record.mockReset();
       sessionReplay.identifiers = undefined;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       expect(record).not.toHaveBeenCalled();
     });
 
@@ -727,7 +727,7 @@ describe('SessionReplay', () => {
         .promise;
       const createEventsIDBStoreInstance = await (SessionReplayIDB.createEventsIDBStore as jest.Mock).mock.results[0]
         .value;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       expect(record).not.toHaveBeenCalled();
       if (!sessionReplay.eventsManager) {
         throw new Error('Did not call init');
@@ -740,7 +740,7 @@ describe('SessionReplay', () => {
       await sessionReplay.init(apiKey, mockOptions).promise;
       const createEventsIDBStoreInstance = await (SessionReplayIDB.createEventsIDBStore as jest.Mock).mock.results[0]
         .value;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       if (!sessionReplay.eventsManager) {
         throw new Error('Did not call init');
       }
@@ -762,7 +762,7 @@ describe('SessionReplay', () => {
       await sessionReplay.init(apiKey, mockOptions).promise;
       const createEventsIDBStoreInstance = await (SessionReplayIDB.createEventsIDBStore as jest.Mock).mock.results[0]
         .value;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       const stopRecordingMock = jest.fn();
       sessionReplay.recordCancelCallback = stopRecordingMock;
       if (!sessionReplay.eventsManager) {
@@ -788,7 +788,7 @@ describe('SessionReplay', () => {
 
     test('should add an error handler', async () => {
       await sessionReplay.init(apiKey, mockOptions).promise;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       const recordArg = record.mock.calls[0][0];
       const errorHandlerReturn = recordArg?.errorHandler && recordArg?.errorHandler(new Error('test error'));
       // eslint-disable-next-line @typescript-eslint/unbound-method
@@ -799,7 +799,7 @@ describe('SessionReplay', () => {
     test('should rethrow CSSStylesheet errors', async () => {
       const sessionReplay = new SessionReplay();
       await sessionReplay.init(apiKey, mockOptions).promise;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       const recordArg = record.mock.calls[0][0];
       const stylesheetErrorMessage =
         "Failed to execute 'insertRule' on 'CSSStyleSheet': Failed to parse the rule 'body::-ms-expand{display: none}";
@@ -811,7 +811,7 @@ describe('SessionReplay', () => {
     test('should rethrow external errors', async () => {
       const sessionReplay = new SessionReplay();
       await sessionReplay.init(apiKey, mockOptions).promise;
-      sessionReplay.captureEventsIfShould();
+      sessionReplay.captureEvents();
       const recordArg = record.mock.calls[0][0];
       const error = new Error('test') as Error & { _external_?: boolean };
       error._external_ = true;