Merge branch 'develop' into feat-replay-promote-delay-on-flush-native

getsentry · Jun 16, 2023 · 9f66dc4 · 9f66dc4
2 parents d5f3e29 + a4c858f
commit 9f66dc4
Show file tree

Hide file tree

Showing 27 changed files with 1,357 additions and 284 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts
@@ -65,8 +65,10 @@ import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest }
       expect(slowClickBreadcrumbs).toEqual([
         {
           category: 'ui.slowClickDetected',
+          type: 'default',
           data: {
             endReason: 'timeout',
+            clickCount: 1,
             node: {
               attributes: expect.objectContaining({
                 id,

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/multiClick/test.ts
@@ -0,0 +1,60 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';
+
+sentryTest('captures multi click when not detecting slow click', async ({ getLocalTestUrl, page }) => {
+  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;
+
+  const reqPromise1 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.multiClick');
+  });
+
+  await page.click('#mutationButtonImmediately', { clickCount: 4 });
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
+
+  expect(slowClickBreadcrumbs).toEqual([
+    {
+      category: 'ui.multiClick',
+      type: 'default',
+      data: {
+        clickCount: 4,
+        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),
+    },
+  ]);
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts
@@ -30,8 +30,6 @@ sentryTest('mutation after threshold results in slow click', async ({ forceFlush
     return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
   });
 
-  // Trigger this twice, sometimes this was flaky otherwise...
-  await page.click('#mutationButton');
   await page.click('#mutationButton');
 
   const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
@@ -41,8 +39,71 @@ sentryTest('mutation after threshold results in slow click', async ({ forceFlush
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
+      data: {
+        endReason: 'mutation',
+        clickCount: 1,
+        node: {
+          attributes: {
+            id: 'mutationButton',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ********',
+        },
+        nodeId: expect.any(Number),
+        timeAfterClickMs: expect.any(Number),
+        url: 'http://sentry-test.io/index.html',
+      },
+      message: 'body > button#mutationButton',
+      timestamp: expect.any(Number),
+    },
+  ]);
+
+  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
+  expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
+});
+
+sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => {
+  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;
+
+  const reqPromise1 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+  });
+
+  void page.click('#mutationButton', { clickCount: 4 });
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
+  const multiClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.multiClick');
+
+  expect(slowClickBreadcrumbs).toEqual([
+    {
+      category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'mutation',
+        clickCount: 4,
         node: {
           attributes: {
             id: 'mutationButton',
@@ -59,18 +120,17 @@ sentryTest('mutation after threshold results in slow click', async ({ forceFlush
       timestamp: expect.any(Number),
     },
   ]);
+  expect(multiClickBreadcrumbs.length).toEqual(0);
 
   expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
   expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
 });
 
-sentryTest(
-  'immediate mutation does not trigger slow click',
-  async ({ browserName, forceFlushReplay, getLocalTestUrl, page }) => {
-    // This test seems to only be flakey on firefox
-    if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
-      sentryTest.skip();
-    }
+sentryTest('immediate mutation does not trigger slow click', async ({ forceFlushReplay, browserName, getLocalTestUrl, page }) => {
+  // This test seems to only be flakey on firefox
+  if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
+    sentryTest.skip();
+  }
 
     const reqPromise0 = waitForReplayRequest(page, 0);
 
@@ -171,3 +231,55 @@ sentryTest('inline click handler does not trigger slow click', async ({ forceFlu
     },
   ]);
 });
+
+sentryTest('mouseDown events are considered', async ({ browserName, getLocalTestUrl, page }) => {
+  // This test seems to only be flakey on firefox
+  if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) {
+    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;
+
+  const reqPromise1 = waitForReplayRequest(page, (event, res) => {
+    const { breadcrumbs } = getCustomRecordingEvents(res);
+
+    return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
+  });
+
+  await page.click('#mouseDownButton');
+
+  const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
+
+  expect(breadcrumbs).toEqual([
+    {
+      category: 'ui.click',
+      data: {
+        node: {
+          attributes: {
+            id: 'mouseDownButton',
+          },
+          id: expect.any(Number),
+          tagName: 'button',
+          textContent: '******* ******** ** ***** ****',
+        },
+        nodeId: expect.any(Number),
+      },
+      message: 'body > button#mouseDownButton',
+      timestamp: expect.any(Number),
+      type: 'default',
+    },
+  ]);
+});
diff --git a/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts
@@ -89,8 +89,10 @@ sentryTest('late scroll triggers slow click', async ({ getLocalTestUrl, page })
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'timeout',
+        clickCount: 1,
         node: {
           attributes: {
             id: 'scrollLateButton',

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/template.html b/packages/browser-integration-tests/suites/replay/slowClick/template.html
@@ -18,6 +18,7 @@
     <button id="scrollButton">Trigger scroll</button>
     <button id="scrollLateButton">Trigger scroll late</button>
     <button id="mutationIgnoreButton" class="ignore-class">Trigger scroll late</button>
+    <button id="mouseDownButton">Trigger mutation on mouse down</button>
 
     <a href="#" id="link">Link</a>
     <a href="#" target="_blank" id="linkExternal">Link external</a>
@@ -69,6 +70,9 @@ <h1 id="h2">Bottom</h1>
           console.log('DONE');
         }, 3001);
       });
+      document.getElementById('mouseDownButton').addEventListener('mousedown', () => {
+        document.getElementById('out').innerHTML += 'mutationButton clicked<br>';
+      });
 
       // Do nothing on these elements
       document

diff --git a/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts
@@ -38,8 +38,10 @@ sentryTest('mutation after timeout results in slow click', async ({ getLocalTest
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'timeout',
+        clickCount: 1,
         node: {
           attributes: {
             id: 'mutationButtonLate',
@@ -93,8 +95,10 @@ sentryTest('console.log results in slow click', async ({ getLocalTestUrl, page }
   expect(slowClickBreadcrumbs).toEqual([
     {
       category: 'ui.slowClickDetected',
+      type: 'default',
       data: {
         endReason: 'timeout',
+        clickCount: 1,
         node: {
           attributes: {
             id: 'consoleLogButton',

diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts
@@ -38,9 +38,9 @@ export function prepareEvent(
   applyClientOptions(prepared, options);
   applyIntegrationsMetadata(prepared, integrations);
 
-  // Only apply debug metadata to error events.
+  // Only put debug IDs onto frames for error events.
   if (event.type === undefined) {
-    applyDebugMetadata(prepared, options.stackParser);
+    applyDebugIds(prepared, options.stackParser);
   }
 
   // If we have scope given to us, use it as the base for further modifications.
@@ -75,6 +75,14 @@ export function prepareEvent(
   }
 
   return result.then(evt => {
+    if (evt) {
+      // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified
+      // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed.
+      // This should not cause any PII issues, since we're only moving data that is already on the event and not adding
+      // any new data
+      applyDebugMeta(evt);
+    }
+
     if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
       return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth);
     }
@@ -121,9 +129,9 @@ function applyClientOptions(event: Event, options: ClientOptions): void {
 const debugIdStackParserCache = new WeakMap<StackParser, Map<string, StackFrame[]>>();
 
 /**
- * Applies debug metadata images to the event in order to apply source maps by looking up their debug ID.
+ * Puts debug IDs into the stack frames of an error event.
  */
-export function applyDebugMetadata(event: Event, stackParser: StackParser): void {
+export function applyDebugIds(event: Event, stackParser: StackParser): void {
   const debugIdMap = GLOBAL_OBJ._sentryDebugIds;
 
   if (!debugIdMap) {
@@ -160,34 +168,60 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void
     return acc;
   }, {});
 
-  // Get a Set of filenames in the stack trace
-  const errorFileNames = new Set<string>();
   try {
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     event!.exception!.values!.forEach(exception => {
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       exception.stacktrace!.frames!.forEach(frame => {
         if (frame.filename) {
-          errorFileNames.add(frame.filename);
+          frame.debug_id = filenameDebugIdMap[frame.filename];
+        }
+      });
+    });
+  } catch (e) {
+    // To save bundle size we're just try catching here instead of checking for the existence of all the different objects.
+  }
+}
+
+/**
+ * Moves debug IDs from the stack frames of an error event into the debug_meta field.
+ */
+export function applyDebugMeta(event: Event): void {
+  // Extract debug IDs and filenames from the stack frames on the event.
+  const filenameDebugIdMap: Record<string, string> = {};
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    event.exception!.values!.forEach(exception => {
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      exception.stacktrace!.frames!.forEach(frame => {
+        if (frame.debug_id) {
+          if (frame.abs_path) {
+            filenameDebugIdMap[frame.abs_path] = frame.debug_id;
+          } else if (frame.filename) {
+            filenameDebugIdMap[frame.filename] = frame.debug_id;
+          }
+          delete frame.debug_id;
         }
       });
     });
   } catch (e) {
     // To save bundle size we're just try catching here instead of checking for the existence of all the different objects.
   }
 
+  if (Object.keys(filenameDebugIdMap).length === 0) {
+    return;
+  }
+
   // Fill debug_meta information
   event.debug_meta = event.debug_meta || {};
   event.debug_meta.images = event.debug_meta.images || [];
   const images = event.debug_meta.images;
-  errorFileNames.forEach(filename => {
-    if (filenameDebugIdMap[filename]) {
-      images.push({
-        type: 'sourcemap',
-        code_file: filename,
-        debug_id: filenameDebugIdMap[filename],
-      });
-    }
+  Object.keys(filenameDebugIdMap).forEach(filename => {
+    images.push({
+      type: 'sourcemap',
+      code_file: filename,
+      debug_id: filenameDebugIdMap[filename],
+    });
   });
 }