fix(replay): Ensure multi click has correct timestamps

getsentry · Jul 19, 2023 · 19ca48c · 19ca48c
1 parent cfd2149
commit 19ca48c
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 7 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts
@@ -62,6 +62,43 @@ sentryTest('captures multi click when not detecting slow click', async ({ getLoc
       timestamp: expect.any(Number),
     },
   ]);
+
+  // When this has been flushed, the timeout has exceeded - so add a new click now, which should trigger another multi click
+
+  const reqPromise2 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.multiClick');
+  });
+
+  await page.click('#mutationButtonImmediately', { clickCount: 3 });
+
+  const { breadcrumbs: breadcrumbb2 } = getCustomRecordingEvents(await reqPromise2);
+
+  const slowClickBreadcrumbs2 = breadcrumbb2.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
+
+  expect(slowClickBreadcrumbs2).toEqual([
+    {
+      category: 'ui.multiClick',
+      type: 'default',
+      data: {
+        clickCount: 3,
+        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),
+    },
+  ]);
 });
 
 sentryTest('captures multiple multi clicks', async ({ getLocalTestUrl, page, forceFlushReplay, browserName }) => {

diff --git a/packages/replay/src/coreHandlers/handleClick.ts b/packages/replay/src/coreHandlers/handleClick.ts
@@ -2,6 +2,7 @@ import type { Breadcrumb } from '@sentry/types';
 
 import { WINDOW } from '../constants';
 import type { MultiClickFrame, ReplayClickDetector, ReplayContainer, SlowClickConfig, SlowClickFrame } from '../types';
+import { timestampToS } from '../util/timestamp';
 import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
 import { getClickTargetNode } from './util/domUtils';
 import { onWindowOpen } from './util/onWindowOpen';
@@ -125,7 +126,7 @@ export class ClickDetector implements ReplayClickDetector {
     }
 
     const newClick: Click = {
-      timestamp: breadcrumb.timestamp,
+      timestamp: timestampToS(breadcrumb.timestamp),
       clickBreadcrumb: breadcrumb,
       // Set this to 0 so we know it originates from the click breadcrumb
       clickCount: 0,
@@ -165,17 +166,18 @@ export class ClickDetector implements ReplayClickDetector {
         click.scrollAfter = click.timestamp <= this._lastScroll ? this._lastScroll - click.timestamp : undefined;
       }
 
+      // All of these are in seconds!
       if (click.timestamp + this._timeout <= now) {
         timedOutClicks.push(click);
       }
     });
 
     // Remove "old" clicks
     for (const click of timedOutClicks) {
-      this._generateBreadcrumbs(click);
-
       const pos = this._clicks.indexOf(click);
-      if (pos !== -1) {
+
+      if (pos > -1) {
+        this._generateBreadcrumbs(click);
         this._clicks.splice(pos, 1);
       }
     }

diff --git a/packages/replay/src/eventBuffer/EventBufferArray.ts b/packages/replay/src/eventBuffer/EventBufferArray.ts
@@ -1,6 +1,6 @@
 import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants';
 import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types';
-import { timestampToMs } from '../util/timestampToMs';
+import { timestampToMs } from '../util/timestamp';
 import { EventBufferSizeExceededError } from './error';
 
 /**

diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts
@@ -2,7 +2,7 @@ import type { ReplayRecordingData } from '@sentry/types';
 
 import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants';
 import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types';
-import { timestampToMs } from '../util/timestampToMs';
+import { timestampToMs } from '../util/timestamp';
 import { EventBufferSizeExceededError } from './error';
 import { WorkerHandler } from './WorkerHandler';
 

diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts
@@ -4,7 +4,7 @@ import { logger } from '@sentry/utils';
 
 import { EventBufferSizeExceededError } from '../eventBuffer/error';
 import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent, ReplayPluginOptions } from '../types';
-import { timestampToMs } from './timestampToMs';
+import { timestampToMs } from './timestamp';
 
 function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent {
   return event.type === EventType.Custom;

diff --git a/packages/replay/src/util/timestampToMs.ts → packages/replay/src/util/timestamp.ts b/packages/replay/src/util/timestampToMs.ts → packages/replay/src/util/timestamp.ts
@@ -5,3 +5,11 @@ export function timestampToMs(timestamp: number): number {
   const isMs = timestamp > 9999999999;
   return isMs ? timestamp : timestamp * 1000;
 }
+
+/**
+ * Converts a timestamp to s, if it was in ms, or keeps it as s.
+ */
+export function timestampToS(timestamp: number): number {
+  const isMs = timestamp > 9999999999;
+  return isMs ? timestamp / 1000 : timestamp;
+}