fix(replay): Adjust slow/multi click handling (#8380)

Implements the changes outlined in
#8379

Author: mydea

packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts

@@ -1,7 +1,12 @@
 import { expect } from '@playwright/test';
 
 import { sentryTest } from '../../../../utils/fixtures';
-import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';
+import {
+  getCustomRecordingEvents,
+  shouldSkipReplayTest,
+  waitForReplayRequest,
+  waitForReplayRequests,
+} from '../../../../utils/replayHelpers';
 
 sentryTest('captures multi click when not detecting slow click', async ({ getLocalTestUrl, page }) => {
   if (shouldSkipReplayTest()) {
@@ -58,3 +63,97 @@ sentryTest('captures multi click when not detecting slow click', async ({ getLoc
     },
   ]);
 });
+
+sentryTest('captures multiple multi clicks', async ({ getLocalTestUrl, page, forceFlushReplay }) => {
+  if (shouldSkipReplayTest()) {
+    sentryTest.skip();
+  }
+
+  const reqPromise0 = waitForReplayRequest(page, 0);
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+  await reqPromise0;
+
+  let multiClickBreadcrumbCount = 0;
+
+  const reqsPromise = waitForReplayRequests(page, (_event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+    const count = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick').length;
+
+    multiClickBreadcrumbCount += count;
+
+    if (multiClickBreadcrumbCount === 2) {
+      return true;
+    }
+
+    return false;
+  });
+
+  await page.click('#mutationButtonImmediately', { clickCount: 4 });
+  await forceFlushReplay();
+
+  // Ensure we waited at least 1s, which is the threshold to create a new ui.click breadcrumb
+  await new Promise(resolve => setTimeout(resolve, 1001));
+
+  await page.click('#mutationButtonImmediately', { clickCount: 2 });
+  await forceFlushReplay();
+
+  const responses = await reqsPromise;
+
+  const slowClickBreadcrumbs = responses
+    .flatMap(res => getCustomRecordingEvents(res).breadcrumbs)
+    .filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
+
+  expect(slowClickBreadcrumbs).toEqual([
+    {
+      category: 'ui.multiClick',
+      type: 'default',
+      data: {
+        clickCount: 6,
+        metric: true,
+        node: {
+          attributes: {
+            id: 'mutationButtonImmediately',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ******** ***********',
+        },
+        nodeId: expect.any(Number),
+        url: 'http://sentry-test.io/index.html',
+      },
+      message: 'body > button#mutationButtonImmediately',
+      timestamp: expect.any(Number),
+    },
+    {
+      category: 'ui.multiClick',
+      type: 'default',
+      data: {
+        clickCount: 2,
+        metric: true,
+        node: {
+          attributes: {
+            id: 'mutationButtonImmediately',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ******** ***********',
+        },
+        nodeId: expect.any(Number),
+        url: 'http://sentry-test.io/index.html',
+      },
+      message: 'body > button#mutationButtonImmediately',
+      timestamp: expect.any(Number),
+    },
+  ]);
+});

packages/browser-integration-tests/utils/replayHelpers.ts

@@ -101,6 +101,46 @@ export function waitForReplayRequest(
   );
 }
 
+/**
+ * Wait until a callback returns true, collecting all replay responses along the way.
+ * This can be useful when you don't know if stuff will be in one or multiple replay requests.
+ */
+export function waitForReplayRequests(
+  page: Page,
+  callback: (event: ReplayEvent, res: Response) => boolean,
+  timeout?: number,
+): Promise<Response[]> {
+  const responses: Response[] = [];
+
+  return new Promise<Response[]>(resolve => {
+    void page.waitForResponse(
+      res => {
+        const req = res.request();
+
+        const event = getReplayEventFromRequest(req);
+
+        if (!event) {
+          return false;
+        }
+
+        responses.push(res);
+
+        try {
+          if (callback(event, res)) {
+            resolve(responses);
+            return true;
+          }
+
+          return false;
+        } catch {
+          return false;
+        }
+      },
+      timeout ? { timeout } : undefined,
+    );
+  });
+}
+
 export function isReplayEvent(event: Event): event is ReplayEvent {
   return event.type === 'replay_event';
 }

packages/replay/src/constants.ts

@@ -42,8 +42,6 @@ export const CONSOLE_ARG_MAX_SIZE = 5_000;
 export const SLOW_CLICK_THRESHOLD = 3_000;
 /* For scroll actions after a click, we only look for a very short time period to detect programmatic scrolling. */
 export const SLOW_CLICK_SCROLL_TIMEOUT = 300;
-/* Clicks in this time period are considered e.g. double/triple clicks. */
-export const MULTI_CLICK_TIMEOUT = 1_000;
 
 /** When encountering a total segment size exceeding this size, stop the replay (as we cannot properly ingest it). */
 export const REPLAY_MAX_EVENT_BUFFER_SIZE = 20_000_000; // ~20MB

packages/replay/src/coreHandlers/handleClick.ts

@@ -33,7 +33,6 @@ export class ClickDetector implements ReplayClickDetector {
   private _clicks: Click[] = [];
   private _teardown: undefined | (() => void);
 
-  private _multiClickTimeout: number;
   private _threshold: number;
   private _scollTimeout: number;
   private _timeout: number;
@@ -51,7 +50,6 @@ export class ClickDetector implements ReplayClickDetector {
   ) {
     // We want everything in s, but options are in ms
     this._timeout = slowClickConfig.timeout / 1000;
-    this._multiClickTimeout = slowClickConfig.multiClickTimeout / 1000;
     this._threshold = slowClickConfig.threshold / 1000;
     this._scollTimeout = slowClickConfig.scrollTimeout / 1000;
     this._replay = replay;
@@ -126,13 +124,6 @@ export class ClickDetector implements ReplayClickDetector {
       return;
     }
 
-    const click = this._getClick(node);
-
-    if (click) {
-      // this means a click on the same element was captured in the last 1s, so we consider this a multi click
-      return;
-    }
-
     const newClick: Click = {
       timestamp: breadcrumb.timestamp,
       clickBreadcrumb: breadcrumb,
@@ -150,22 +141,14 @@ export class ClickDetector implements ReplayClickDetector {
 
   /** Count multiple clicks on elements. */
   private _handleMultiClick(node: HTMLElement): void {
-    const click = this._getClick(node);
-
-    if (!click) {
-      return;
-    }
-
-    click.clickCount++;
+    this._getClicks(node).forEach(click => {
+      click.clickCount++;
+    });
   }
 
-  /** Try to get an existing click on the given element. */
-  private _getClick(node: HTMLElement): Click | undefined {
-    const now = nowInSeconds();
-
-    // Find any click on the same element in the last second
-    // If one exists, we consider this click as a double/triple/etc click
-    return this._clicks.find(click => click.node === node && now - click.timestamp < this._multiClickTimeout);
+  /** Get all pending clicks for a given node. */
+  private _getClicks(node: HTMLElement): Click[] {
+    return this._clicks.filter(click => click.node === node);
   }
 
   /** Check the clicks that happened. */
@@ -182,13 +165,6 @@ export class ClickDetector implements ReplayClickDetector {
         click.scrollAfter = click.timestamp <= this._lastScroll ? this._lastScroll - click.timestamp : undefined;
       }
 
-      // If an action happens after the multi click threshold, we can skip waiting and handle the click right away
-      const actionTime = click.scrollAfter || click.mutationAfter || 0;
-      if (actionTime && actionTime >= this._multiClickTimeout) {
-        timedOutClicks.push(click);
-        return;
-      }
-
       if (click.timestamp + this._timeout <= now) {
         timedOutClicks.push(click);
       }
@@ -269,6 +245,10 @@ export class ClickDetector implements ReplayClickDetector {
 
   /** Schedule to check current clicks. */
   private _scheduleCheckClicks(): void {
+    if (this._checkClickTimeout) {
+      clearTimeout(this._checkClickTimeout);
+    }
+
     this._checkClickTimeout = setTimeout(() => this._checkClicks(), 1000);
   }
 }

packages/replay/src/replay.ts

@@ -7,7 +7,6 @@ import { logger } from '@sentry/utils';
 import {
   BUFFER_CHECKOUT_TIME,
   MAX_SESSION_LIFE,
-  MULTI_CLICK_TIMEOUT,
   SESSION_IDLE_EXPIRE_DURATION,
   SESSION_IDLE_PAUSE_DURATION,
   SLOW_CLICK_SCROLL_TIMEOUT,
@@ -175,7 +174,6 @@ export class ReplayContainer implements ReplayContainerInterface {
           timeout: slowClickTimeout,
           scrollTimeout: SLOW_CLICK_SCROLL_TIMEOUT,
           ignoreSelector: slowClickIgnoreSelectors ? slowClickIgnoreSelectors.join(',') : '',
-          multiClickTimeout: MULTI_CLICK_TIMEOUT,
         }
       : undefined;
 

packages/replay/src/types/replay.ts

@@ -485,5 +485,4 @@ export interface SlowClickConfig {
   timeout: number;
   scrollTimeout: number;
   ignoreSelector: string;
-  multiClickTimeout: number;
 }