add tests

getsentry · Oct 31, 2023 · 79fd745 · 79fd745
1 parent a54274d
commit 79fd745
Show file tree

Hide file tree

Showing 5 changed files with 205 additions and 6 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/error/init.js b/packages/browser-integration-tests/suites/replay/slowClick/error/init.js
@@ -0,0 +1,18 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+window.Replay = new Sentry.Replay({
+  flushMinDelay: 200,
+  flushMaxDelay: 200,
+  minReplayDuration: 0,
+  slowClickTimeout: 3500,
+});
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  sampleRate: 1,
+  replaysSessionSampleRate: 0.0,
+  replaysOnErrorSampleRate: 1.0,
+
+  integrations: [window.Replay],
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/error/template.html b/packages/browser-integration-tests/suites/replay/slowClick/error/template.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button id="buttonError">Trigger error</button>
+    <button id="buttonErrorMutation">Trigger error</button>
+
+    <script>
+      document.getElementById('buttonError').addEventListener('click', () => {
+        throw new Error('test error happened');
+      });
+
+      document.getElementById('buttonErrorMutation').addEventListener('click', () => {
+        document.getElementById('buttonErrorMutation').innerText = 'Test error happened!';
+
+        throw new Error('test error happened');
+      });
+    </script>
+  </body>
+</html>
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/error/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/error/test.ts
@@ -0,0 +1,140 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import {
+  getCustomRecordingEvents,
+  getReplayEventFromRequest,
+  shouldSkipReplayTest,
+  waitForReplayRequest,
+} from '../../../../utils/replayHelpers';
+
+sentryTest('slow click that triggers error is captured', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipReplayTest()) {
+    sentryTest.skip();
+  }
+
+  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);
+
+  const [req0] = await Promise.all([
+    waitForReplayRequest(page, (_event, res) => {
+      const { breadcrumbs } = getCustomRecordingEvents(res);
+
+      return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+    }),
+    page.click('#buttonError'),
+  ]);
+
+  const { breadcrumbs } = getCustomRecordingEvents(req0);
+
+  const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+
+  expect(slowClickBreadcrumbs).toEqual([
+    {
+      category: 'ui.slowClickDetected',
+      type: 'default',
+      data: {
+        endReason: 'timeout',
+        clickCount: 1,
+        node: {
+          attributes: {
+            id: 'buttonError',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* *****',
+        },
+        nodeId: expect.any(Number),
+        timeAfterClickMs: 3500,
+        url: 'http://sentry-test.io/index.html',
+      },
+      message: 'body > button#buttonError',
+      timestamp: expect.any(Number),
+    },
+  ]);
+});
+
+sentryTest(
+  'click that triggers error & mutation is not captured',
+  async ({ getLocalTestUrl, page, forceFlushReplay }) => {
+    if (shouldSkipReplayTest()) {
+      sentryTest.skip();
+    }
+
+    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);
+
+    let slowClickCount = 0;
+
+    page.on('response', res => {
+      const req = res.request();
+
+      const event = getReplayEventFromRequest(req);
+
+      if (!event) {
+        return;
+      }
+
+      const { breadcrumbs } = getCustomRecordingEvents(res);
+
+      const slowClicks = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+      slowClickCount += slowClicks.length;
+    });
+
+    const [req1] = await Promise.all([
+      waitForReplayRequest(page, (_event, res) => {
+        const { breadcrumbs } = getCustomRecordingEvents(res);
+
+        return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
+      }),
+      page.click('#buttonErrorMutation'),
+    ]);
+
+    const { breadcrumbs } = getCustomRecordingEvents(req1);
+
+    expect(breadcrumbs).toEqual([
+      {
+        category: 'ui.click',
+        data: {
+          node: {
+            attributes: {
+              id: 'buttonErrorMutation',
+            },
+            id: expect.any(Number),
+            tagName: 'button',
+            textContent: '******* *****',
+          },
+          nodeId: expect.any(Number),
+        },
+        message: 'body > button#buttonErrorMutation',
+        timestamp: expect.any(Number),
+        type: 'default',
+      },
+    ]);
+
+    // Ensure we wait for timeout, to make sure no slow click is created
+    // Waiting for 3500 + 1s rounding room
+    await new Promise(resolve => setTimeout(resolve, 4500));
+    await forceFlushReplay();
+
+    expect(slowClickCount).toBe(0);
+  },
+);
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts
@@ -141,8 +141,17 @@ sentryTest('immediate mutation does not trigger slow click', async ({ forceFlush
   await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]);
   await forceFlushReplay();
 
+  let slowClickCount = 0;
+
+  page.on('response', res => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    const slowClicks = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+    slowClickCount += slowClicks.length;
+  });
+
   const [req1] = await Promise.all([
-    waitForReplayRequest(page, (event, res) => {
+    waitForReplayRequest(page, (_event, res) => {
       const { breadcrumbs } = getCustomRecordingEvents(res);
 
       return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
@@ -171,6 +180,13 @@ sentryTest('immediate mutation does not trigger slow click', async ({ forceFlush
       type: 'default',
     },
   ]);
+
+  // Ensure we wait for timeout, to make sure no slow click is created
+  // Waiting for 3500 + 1s rounding room
+  await new Promise(resolve => setTimeout(resolve, 4500));
+  await forceFlushReplay();
+
+  expect(slowClickCount).toBe(0);
 });
 
 sentryTest('inline click handler does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => {

diff --git a/packages/replay/src/coreHandlers/handleClick.ts b/packages/replay/src/coreHandlers/handleClick.ts
@@ -10,7 +10,7 @@ import type {
   ReplaySlowClickFrame,
   SlowClickConfig,
 } from '../types';
-import { ReplayEventTypeFullSnapshot, ReplayEventTypeIncrementalSnapshot } from '../types';
+import { ReplayEventTypeIncrementalSnapshot } from '../types';
 import { timestampToS } from '../util/timestamp';
 import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
 import { getClosestInteractive } from './util/domUtils';
@@ -310,10 +310,13 @@ function nowInSeconds(): number {
 /** Update the click detector based on a recording event of rrweb. */
 export function updateClickDetectorForRecordingEvent(clickDetector: ReplayClickDetector, event: RecordingEvent): void {
   try {
-    // We interpret a full snapshot as a mutation (this may not be true, but there is no way for us to know)
-    if (event.type === ReplayEventTypeFullSnapshot) {
-      clickDetector.registerMutation(event.timestamp);
-    }
+    // note: We only consider incremental snapshots here
+    // This means that any full snapshot is ignored for mutation detection - the reason is that we simply cannot know if a mutation happened here.
+    // E.g. think that we are buffering, an error happens and we take a full snapshot because we switched to session mode -
+    // in this scenario, we would not know if a dead click happened because of the error, which is a key dead click scenario.
+    // Instead, by ignoring full snapshots, we have the risk that we generate a false positive
+    // (if a mutation _did_ happen but was "swallowed" by the full snapshot)
+    // But this should be more unlikely as we'd generally capture the incremental snapshot right away
 
     if (!isIncrementalEvent(event)) {
       return;