fix(replay): Fix xhr start timestamps (#9341)

We've been using the wrong `startTimestamp` for the core xhr
instrumentation. Outside of replay, this wasn't noticed because we are
not actually using the anywhere 😬 But in replay, it lead to all xhr
breadrcumbs showing an incorrect duration of `0`.

Note that this is _maybe_ not 100% correct, as in theory you could call
`xhr.send()` later, which is probably the _most correct_ start
timestamp. But this would require us to keep the start time somewhere on
the xhr object, which is a bit trickier than this solution. So I think
it is fine to do this based on `xhr.open()` (and _definitely_ more
correct than it was before).

fixes getsentry/sentry#52790

Author: mydea

packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureTimestamps/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,
+});
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  sampleRate: 1,
+  // We ensure to sample for errors, so by default nothing is sent
+  replaysSessionSampleRate: 0.0,
+  replaysOnErrorSampleRate: 1.0,
+
+  integrations: [window.Replay],
+});

packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureTimestamps/test.ts

@@ -0,0 +1,67 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../../utils/fixtures';
+import { envelopeRequestParser, waitForErrorRequest } from '../../../../../utils/helpers';
+import {
+  getCustomRecordingEvents,
+  shouldSkipReplayTest,
+  waitForReplayRequest,
+} from '../../../../../utils/replayHelpers';
+
+sentryTest('captures correct timestamps', async ({ getLocalTestPath, page, browserName }) => {
+  // These are a bit flaky on non-chromium browsers
+  if (shouldSkipReplayTest() || browserName !== 'chromium') {
+    sentryTest.skip();
+  }
+
+  await page.route('**/foo', route => {
+    return route.fulfill({
+      status: 200,
+    });
+  });
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', async route => {
+    await new Promise(resolve => setTimeout(resolve, 10));
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const requestPromise = waitForErrorRequest(page);
+  const replayRequestPromise1 = waitForReplayRequest(page, 0);
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+  await page.goto(url);
+
+  await page.evaluate(() => {
+    /* eslint-disable */
+    fetch('http://localhost:7654/foo', {
+      method: 'POST',
+      body: '{"foo":"bar"}',
+    }).then(() => {
+      // @ts-expect-error Sentry is a global
+      Sentry.captureException('test error');
+    });
+    /* eslint-enable */
+  });
+
+  const request = await requestPromise;
+  const eventData = envelopeRequestParser(request);
+
+  const replayReq1 = await replayRequestPromise1;
+  const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1);
+
+  const xhrSpan = performanceSpans1.find(span => span.op === 'resource.fetch')!;
+
+  expect(xhrSpan).toBeDefined();
+
+  const { startTimestamp, endTimestamp } = xhrSpan;
+
+  expect(startTimestamp).toEqual(expect.any(Number));
+  expect(endTimestamp).toEqual(expect.any(Number));
+  expect(endTimestamp).toBeGreaterThan(startTimestamp);
+
+  expect(eventData!.breadcrumbs![0].timestamp).toBeGreaterThan(startTimestamp);
+});

packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureTimestamps/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,
+});
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  sampleRate: 1,
+  // We ensure to sample for errors, so by default nothing is sent
+  replaysSessionSampleRate: 0.0,
+  replaysOnErrorSampleRate: 1.0,
+
+  integrations: [window.Replay],
+});

packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureTimestamps/test.ts

@@ -0,0 +1,75 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../../../utils/fixtures';
+import { envelopeRequestParser, waitForErrorRequest } from '../../../../../utils/helpers';
+import {
+  getCustomRecordingEvents,
+  shouldSkipReplayTest,
+  waitForReplayRequest,
+} from '../../../../../utils/replayHelpers';
+
+sentryTest('captures correct timestamps', async ({ getLocalTestPath, page, browserName }) => {
+  // These are a bit flaky on non-chromium browsers
+  if (shouldSkipReplayTest() || browserName !== 'chromium') {
+    sentryTest.skip();
+  }
+
+  await page.route('**/foo', route => {
+    return route.fulfill({
+      status: 200,
+    });
+  });
+
+  await page.route('https://dsn.ingest.sentry.io/**/*', async route => {
+    await new Promise(resolve => setTimeout(resolve, 10));
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const requestPromise = waitForErrorRequest(page);
+  const replayRequestPromise1 = waitForReplayRequest(page, 0);
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+  await page.goto(url);
+
+  void page.evaluate(() => {
+    /* eslint-disable */
+    const xhr = new XMLHttpRequest();
+
+    xhr.open('POST', 'http://localhost:7654/foo');
+    xhr.setRequestHeader('Accept', 'application/json');
+    xhr.setRequestHeader('Content-Type', 'application/json');
+    xhr.setRequestHeader('Cache', 'no-cache');
+    xhr.setRequestHeader('X-Test-Header', 'test-value');
+    xhr.send();
+
+    xhr.addEventListener('readystatechange', function () {
+      if (xhr.readyState === 4) {
+        // @ts-expect-error Sentry is a global
+        setTimeout(() => Sentry.captureException('test error', 0));
+      }
+    });
+    /* eslint-enable */
+  });
+
+  const request = await requestPromise;
+  const eventData = envelopeRequestParser(request);
+
+  const replayReq1 = await replayRequestPromise1;
+  const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1);
+
+  const xhrSpan = performanceSpans1.find(span => span.op === 'resource.xhr')!;
+
+  expect(xhrSpan).toBeDefined();
+
+  const { startTimestamp, endTimestamp } = xhrSpan;
+
+  expect(startTimestamp).toEqual(expect.any(Number));
+  expect(endTimestamp).toEqual(expect.any(Number));
+  expect(endTimestamp).toBeGreaterThan(startTimestamp);
+
+  expect(eventData!.breadcrumbs![0].timestamp).toBeGreaterThan(startTimestamp);
+});

packages/utils/src/instrument.ts

@@ -257,6 +257,8 @@ export function instrumentXHR(): void {
 
   fill(xhrproto, 'open', function (originalOpen: () => void): () => void {
     return function (this: XMLHttpRequest & SentryWrappedXMLHttpRequest, ...args: any[]): void {
+      const startTimestamp = Date.now();
+
       const url = args[1];
       const xhrInfo: SentryXhrData = (this[SENTRY_XHR_DATA_KEY] = {
         // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
@@ -291,7 +293,7 @@ export function instrumentXHR(): void {
           triggerHandlers('xhr', {
             args: args as [string, string],
             endTimestamp: Date.now(),
-            startTimestamp: Date.now(),
+            startTimestamp,
             xhr: this,
           } as HandlerDataXhr);
         }