refactor for less flakiness??

getsentry · May 10, 2023 · ba4637f · ba4637f
1 parent 9038496
commit ba4637f
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 60 deletions.
diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts
@@ -1,7 +1,14 @@
 import { expect } from '@playwright/test';
+import type { Breadcrumb } from '@sentry/types';
 
 import { sentryTest } from '../../../utils/fixtures';
-import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';
+import type { PerformanceSpan } from '../../../utils/replayHelpers';
+import {
+  getCustomRecordingEvents,
+  getReplayEventFromRequest,
+  shouldSkipReplayTest,
+  waitForReplayRequest,
+} from '../../../utils/replayHelpers';
 
 const COUNT = 250;
 const THROTTLE_LIMIT = 300;
@@ -49,86 +56,77 @@ sentryTest(
     await page.goto(url);
     await reqPromise0;
 
-    const reqPromise1 = waitForReplayRequest(
-      page,
-      (_event, res) => {
-        const { performanceSpans } = getCustomRecordingEvents(res);
-
-        return performanceSpans.some(span => span.op === 'resource.script');
-      },
-      10_000,
-    );
-    const reqPromise1Breadcrumbs = waitForReplayRequest(
-      page,
-      (_event, res) => {
-        const { breadcrumbs } = getCustomRecordingEvents(res);
-
-        return breadcrumbs.some(breadcrumb => breadcrumb.category === 'replay.throttled');
-      },
-      10_000,
-    );
+    let collectedSpans: PerformanceSpan[] = [];
+    let collectedBreadcrumbs: Breadcrumb[] = [];
+
+    page.on('response', response => {
+      // We only capture sentry stuff
+      if (!response.url().includes('https://dsn.ingest.sentry')) {
+        return;
+      }
+
+      // If this is undefined, this is not a replay request
+      if (!getReplayEventFromRequest(response.request())) {
+        return;
+      }
+
+      const { performanceSpans, breadcrumbs } = getCustomRecordingEvents(response);
+
+      collectedSpans.push(
+        ...performanceSpans.filter(span => span.op === 'resource.script' || span.op === 'resource.fetch'),
+      );
+      collectedBreadcrumbs.push(...breadcrumbs.filter(breadcrumb => breadcrumb.category === 'replay.throttled'));
+    });
 
     await page.click('[data-network]');
     await page.click('[data-fetch]');
 
     await page.waitForFunction('window.__isLoaded()');
     await forceFlushReplay();
 
-    const { performanceSpans } = getCustomRecordingEvents(await reqPromise1);
-    const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1Breadcrumbs);
+    await waitForFunction(() => collectedBreadcrumbs.length === 1, 10_000, 100);
 
     // All assets have been _loaded_
     expect(scriptsLoaded).toBe(COUNT);
     expect(fetchLoaded).toBe(COUNT);
 
     // But only some have been captured by replay
-    // We check for <= THROTTLE_LIMIT, as there have been some captured before, which take up some of the throttle limit
-    expect(performanceSpans.length).toBeLessThanOrEqual(THROTTLE_LIMIT);
-    expect(performanceSpans.length).toBeGreaterThan(THROTTLE_LIMIT - 50);
-
-    expect(breadcrumbs.filter(({ category }) => category === 'replay.throttled').length).toBe(1);
+    // We give it some wiggle room to account for flakyness
+    expect(collectedSpans.length).toBeLessThanOrEqual(THROTTLE_LIMIT);
+    expect(collectedSpans.length).toBeGreaterThanOrEqual(THROTTLE_LIMIT - 50);
+    expect(collectedBreadcrumbs.length).toBe(1);
 
     // Now we wait for 6s (5s + some wiggle room), and make some requests again
-    await page.waitForTimeout(7_000);
+    await page.waitForTimeout(6_000);
     await forceFlushReplay();
 
-    const reqPromise2 = waitForReplayRequest(
-      page,
-      (_event, res) => {
-        const { performanceSpans } = getCustomRecordingEvents(res);
-
-        return performanceSpans.some(span => span.op === 'resource.script');
-      },
-      10_000,
-    );
-    const reqPromise2Breadcrumbs = waitForReplayRequest(
-      page,
-      (_event, res) => {
-        const { breadcrumbs } = getCustomRecordingEvents(res);
-
-        return breadcrumbs.some(breadcrumb => breadcrumb.category === 'replay.throttled');
-      },
-      10_000,
-    );
+    // Reset collectors
+    collectedSpans = [];
+    collectedBreadcrumbs = [];
 
     await page.click('[data-network]');
     await page.click('[data-fetch]');
 
     await page.waitForFunction('window.__isLoaded(2)');
     await forceFlushReplay();
 
-    const { performanceSpans: performanceSpans2 } = getCustomRecordingEvents(await reqPromise2);
-    const { breadcrumbs: breadcrumbs2 } = getCustomRecordingEvents(await reqPromise2Breadcrumbs);
+    await waitForFunction(() => collectedBreadcrumbs.length === 1, 10_000, 100);
 
     // All assets have been _loaded_
     expect(scriptsLoaded).toBe(COUNT * 2);
     expect(fetchLoaded).toBe(COUNT * 2);
 
     // But only some have been captured by replay
-    // We check for <= THROTTLE_LIMIT, as there have been some captured before, which take up some of the throttle limit
-    expect(performanceSpans2.length).toBeLessThanOrEqual(THROTTLE_LIMIT);
-    expect(performanceSpans2.length).toBeGreaterThan(THROTTLE_LIMIT - 50);
-
-    expect(breadcrumbs2.filter(({ category }) => category === 'replay.throttled').length).toBe(1);
+    // We give it some wiggle room to account for flakyness
+    expect(collectedSpans.length).toBeLessThanOrEqual(THROTTLE_LIMIT);
+    expect(collectedSpans.length).toBeGreaterThanOrEqual(THROTTLE_LIMIT - 50);
+    expect(collectedBreadcrumbs.length).toBe(1);
   },
 );
+
+async function waitForFunction(cb: () => boolean, timeout = 2000, increment = 100) {
+  while (timeout > 0 && !cb()) {
+    await new Promise(resolve => setTimeout(resolve, increment));
+    await waitForFunction(cb, timeout - increment, increment);
+  }
+}
diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts
@@ -34,6 +34,25 @@ export type IncrementalRecordingSnapshot = eventWithTime & {
 
 export type RecordingSnapshot = FullRecordingSnapshot | IncrementalRecordingSnapshot;
 
+/** Returns the replay event from the given request, or undefined if this is not a replay request. */
+export function getReplayEventFromRequest(req: Request): ReplayEvent | undefined {
+  const postData = req.postData();
+  if (!postData) {
+    return undefined;
+  }
+
+  try {
+    const event = envelopeRequestParser(req);
+
+    if (!isReplayEvent(event)) {
+      return undefined;
+    }
+
+    return event;
+  } catch {
+    return undefined;
+  }
+}
 /**
  * Waits for a replay request to be sent by the page and returns it.
  *
@@ -58,18 +77,13 @@ export function waitForReplayRequest(
     res => {
       const req = res.request();
 
-      const postData = req.postData();
-      if (!postData) {
+      const event = getReplayEventFromRequest(req);
+
+      if (!event) {
         return false;
       }
 
       try {
-        const event = envelopeRequestParser(req);
-
-        if (!isReplayEvent(event)) {
-          return false;
-        }
-
         if (callback) {
           return callback(event, res);
         }