fix(session-replay-browser): prevent finder errors from affecting ses…

…sion replay (#934)

* fix(session-replay-browser): only setup hooks if interaction config is enabled

* fix(session-replay-browser): prevent finder errors from affecting session replay capture

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

@@ -4,6 +4,7 @@ import { SessionReplayEventsManager as AmplitudeSessionReplayEventsManager } fro
 import { PayloadBatcher } from 'src/track-destination';
 import { finder } from '../libs/finder';
 import { getGlobalScope } from '@amplitude/analytics-client-common';
+import type { Logger } from '@amplitude/analytics-types';
 
 // exported for testing
 export type ClickEvent = {
@@ -67,8 +68,8 @@ export const clickBatcher: PayloadBatcher = ({ version, events }) => {
   return { version, events: Object.values(reduced) };
 };
 
-export const clickHook: (options: Options) => mouseInteractionCallBack =
-  ({ eventsManager, sessionId, deviceIdFn }) =>
+export const clickHook: (logger: Logger, options: Options) => mouseInteractionCallBack =
+  (logger, { eventsManager, sessionId, deviceIdFn }) =>
   (e) => {
     if (e.type !== MouseInteractions.Click) {
       return;
@@ -93,7 +94,11 @@ export const clickHook: (options: Options) => mouseInteractionCallBack =
     const node = record.mirror.getNode(e.id);
     let selector;
     if (node) {
-      selector = finder(node as Element);
+      try {
+        selector = finder(node as Element);
+      } catch (err) {
+        logger.debug('error resolving selector from finder');
+      }
     }
 
     const { left: scrollX, top: scrollY } = utils.getWindowScroll(globalScope as unknown as Window);

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

@@ -321,7 +321,20 @@ export class SessionReplay implements AmplitudeSessionReplay {
       return;
     }
     this.stopRecordingEvents();
-    const { privacyConfig } = config;
+    const { privacyConfig, interactionConfig } = config;
+
+    const hooks = interactionConfig?.enabled
+      ? {
+          mouseInteraction:
+            this.eventsManager &&
+            clickHook(this.loggerProvider, {
+              eventsManager: this.eventsManager,
+              sessionId,
+              deviceIdFn: this.getDeviceId.bind(this),
+            }),
+          scroll: this.scrollHook,
+        }
+      : {};
 
     this.loggerProvider.log(`Session Replay capture beginning for ${sessionId}.`);
     this.recordCancelCallback = record({
@@ -339,16 +352,7 @@ export class SessionReplay implements AmplitudeSessionReplay {
         }
       },
       inlineStylesheet: config.shouldInlineStylesheet,
-      hooks: {
-        mouseInteraction:
-          this.eventsManager &&
-          clickHook({
-            eventsManager: this.eventsManager,
-            sessionId,
-            deviceIdFn: this.getDeviceId.bind(this),
-          }),
-        scroll: this.scrollHook,
-      },
+      hooks,
       maskAllInputs: true,
       maskTextClass: MASK_TEXT_CLASS,
       blockClass: BLOCK_CLASS,

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

@@ -1,14 +1,39 @@
+/* eslint-disable @typescript-eslint/no-unsafe-call */
+/* eslint-disable @typescript-eslint/no-unsafe-member-access */
+/* eslint-disable @typescript-eslint/no-unsafe-return */
 /* eslint-disable @typescript-eslint/no-unsafe-assignment */
 import * as AnalyticsClientCommon from '@amplitude/analytics-client-common';
 import { MouseInteractions } from '@amplitude/rrweb-types';
 import { SessionReplayEventsManager } from '../../src/typings/session-replay';
 import { UUID } from '@amplitude/analytics-core';
 import { ClickEvent, ClickEventWithCount, clickBatcher, clickHook, clickNonBatcher } from '../../src/hooks/click';
 import { record, utils } from '@amplitude/rrweb';
+import type { Logger } from '@amplitude/analytics-types';
+import { finder } from '../../src/libs/finder';
 
 jest.mock('@amplitude/rrweb');
+jest.mock('../../src/libs/finder');
 
 describe('click', () => {
+  const mockLoggerProvider: Logger = {
+    error: jest.fn(),
+    log: jest.fn(),
+    disable: jest.fn(),
+    enable: jest.fn(),
+    warn: jest.fn(),
+    debug: jest.fn(),
+  };
+
+  // Need to mock this in one function, but we want to use the real function everywhere else.
+  // If there is a better way to do this in Jest, please refactor this!
+  const mockFinder = finder as jest.Mock;
+  beforeEach(() => {
+    mockFinder.mockImplementation((arg) => {
+      const fn = jest.requireActual('../../src/libs/finder');
+      return fn.finder(arg);
+    });
+  });
+
   describe('clickHook', () => {
     const mockEventsManager: jest.Mocked<SessionReplayEventsManager<'interaction', string>> = {
       sendStoredEvents: jest.fn(),
@@ -39,7 +64,7 @@ describe('click', () => {
     const deviceId = UUID();
     const sessionId = Math.round(Math.random() * 1_000_000);
 
-    const hook = clickHook({
+    const hook = clickHook(mockLoggerProvider, {
       deviceIdFn: () => deviceId,
       eventsManager: mockEventsManager,
       sessionId: sessionId,
@@ -65,7 +90,7 @@ describe('click', () => {
     });
     test('do nothing if no window given', () => {
       mockGlobalScope(undefined);
-      const hook = clickHook({
+      const hook = clickHook(mockLoggerProvider, {
         deviceIdFn: () => deviceId,
         eventsManager: mockEventsManager,
         sessionId: sessionId,
@@ -82,7 +107,7 @@ describe('click', () => {
       mockGlobalScope({
         location: undefined,
       });
-      const hook = clickHook({
+      const hook = clickHook(mockLoggerProvider, {
         deviceIdFn: () => deviceId,
         eventsManager: mockEventsManager,
         sessionId: sessionId,
@@ -156,6 +181,39 @@ describe('click', () => {
         selector: 'div',
       });
     });
+    test('no selector info on finder failure', () => {
+      const mockFinder = finder as jest.Mock;
+      mockFinder.mockImplementation(() => {
+        throw new Error('');
+      });
+
+      (record.mirror.getNode as jest.Mock).mockImplementation(() => {
+        const ele = document.createElement('div');
+        document.body.appendChild(ele);
+        return ele;
+      }) as any;
+      const hook = clickHook(mockLoggerProvider, {
+        deviceIdFn: () => deviceId,
+        eventsManager: mockEventsManager,
+        sessionId: sessionId,
+      });
+      hook({
+        id: 1234,
+        type: MouseInteractions.Click,
+        x: 3,
+        y: 3,
+      });
+      expect(jest.spyOn(mockEventsManager, 'addEvent')).toHaveBeenCalledTimes(1);
+      expect(JSON.parse(mockEventsManager.addEvent.mock.calls[0][0].event.data)).toStrictEqual({
+        x: 3,
+        y: 3,
+        viewportHeight: 768,
+        viewportWidth: 1024,
+        pageUrl: 'http://localhost/',
+        timestamp: expect.any(Number),
+        type: 'click',
+      });
+    });
   });
 
   describe('clickBatcher', () => {

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

@@ -967,6 +967,41 @@ describe('SessionReplay', () => {
         recordArg?.errorHandler && recordArg?.errorHandler(error);
       }).toThrow(error);
     });
+
+    test('should not add hooks if interaction config is not enabled', async () => {
+      const sessionReplay = new SessionReplay();
+      await sessionReplay.init(apiKey, mockOptions).promise;
+      sessionReplay.recordEvents();
+      const recordArg = record.mock.calls[0][0];
+      const error = new Error('test') as Error & { _external_?: boolean };
+      error._external_ = true;
+      expect(recordArg?.hooks).toStrictEqual({});
+    });
+
+    test('should add hooks if interaction config is enabled', async () => {
+      // enable interaction config
+      getRemoteConfigMock = jest.fn().mockImplementation((namespace: string, key: keyof SessionReplayRemoteConfig) => {
+        if (namespace === 'sessionReplay' && key === 'sr_interaction_config') {
+          return {
+            enabled: true,
+          };
+        }
+        return;
+      });
+      jest.spyOn(RemoteConfigFetch, 'createRemoteConfigFetch').mockResolvedValue({
+        getRemoteConfig: getRemoteConfigMock,
+        metrics: {},
+      });
+
+      const sessionReplay = new SessionReplay();
+      await sessionReplay.init(apiKey, mockOptions).promise;
+      sessionReplay.recordEvents();
+      const recordArg = record.mock.calls[0][0];
+      const error = new Error('test') as Error & { _external_?: boolean };
+      error._external_ = true;
+      expect(recordArg?.hooks?.mouseInteraction).toBeDefined();
+      expect(recordArg?.hooks?.scroll).toBeDefined();
+    });
   });
 
   describe('getDeviceId', () => {