fix(browser): Ensure pageload & navigation spans have correct data

dev-packages/browser-integration-tests/suites/integrations/Breadcrumbs/history/navigation/test.ts

@@ -29,14 +29,14 @@ sentryTest('should record history changes as navigation breadcrumbs', async ({ g
       category: 'navigation',
       data: {
         from: '/bar?a=1#fragment',
-        to: '[object Object]',
+        to: '/[object%20Object]',
       },
       timestamp: expect.any(Number),
     },
     {
       category: 'navigation',
       data: {
-        from: '[object Object]',
+        from: '/[object%20Object]',
         to: '/bar?a=1#fragment',
       },
       timestamp: expect.any(Number),

dev-packages/browser-integration-tests/suites/replay/multiple-pages/test.ts

@@ -210,7 +210,7 @@ sentryTest(
     expect(replayEvent6).toEqual(
       getExpectedReplayEvent({
         segment_id: 6,
-        urls: ['/spa'],
+        urls: [`${TEST_HOST}/spa`],
         request: {
           url: `${TEST_HOST}/spa`,
           headers: {

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js

@@ -9,4 +9,4 @@ Sentry.init({
 });
 
 // Immediately navigate to a new page to abort the pageload
-window.location.href = '#foo';
+window.history.pushState({}, '', '/sub-page');

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts

@@ -40,6 +40,9 @@ sentryTest(
     expect(navigationTraceId).toBeDefined();
     expect(pageloadTraceId).not.toEqual(navigationTraceId);
 
+    expect(pageloadRequest.transaction).toEqual('/index.html');
+    expect(navigationRequest.transaction).toEqual('/sub-page');
+
     expect(pageloadRequest.contexts?.trace?.data).toMatchObject({
       [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
       [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
@@ -54,5 +57,17 @@ sentryTest(
       [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
       ['sentry.idle_span_finish_reason']: 'idleTimeout',
     });
+    expect(pageloadRequest.request).toEqual({
+      headers: {
+        'User-Agent': expect.any(String),
+      },
+      url: 'http://sentry-test.io/index.html',
+    });
+    expect(navigationRequest.request).toEqual({
+      headers: {
+        'User-Agent': expect.any(String),
+      },
+      url: 'http://sentry-test.io/sub-page',
+    });
   },
 );

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation/test.ts

@@ -7,7 +7,12 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
 } from '@sentry/core';
 import { sentryTest } from '../../../../utils/fixtures';
-import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
+import {
+  envelopeRequestParser,
+  getFirstSentryEnvelopeRequest,
+  shouldSkipTracingTest,
+  waitForTransactionRequest,
+} from '../../../../utils/helpers';
 
 sentryTest('should create a navigation transaction on page navigation', async ({ getLocalTestUrl, page }) => {
   if (shouldSkipTracingTest()) {
@@ -31,6 +36,10 @@ sentryTest('should create a navigation transaction on page navigation', async ({
   expect(navigationTraceId).toBeDefined();
   expect(pageloadTraceId).not.toEqual(navigationTraceId);
 
+  expect(pageloadRequest.transaction).toEqual('/index.html');
+  // Fragment is not in transaction name
+  expect(navigationRequest.transaction).toEqual('/index.html');
+
   expect(pageloadRequest.contexts?.trace?.data).toMatchObject({
     [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
     [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
@@ -45,6 +54,18 @@ sentryTest('should create a navigation transaction on page navigation', async ({
     [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
     ['sentry.idle_span_finish_reason']: 'idleTimeout',
   });
+  expect(pageloadRequest.request).toEqual({
+    headers: {
+      'User-Agent': expect.any(String),
+    },
+    url: 'http://sentry-test.io/index.html',
+  });
+  expect(navigationRequest.request).toEqual({
+    headers: {
+      'User-Agent': expect.any(String),
+    },
+    url: 'http://sentry-test.io/index.html#foo',
+  });
 
   const pageloadSpans = pageloadRequest.spans;
   const navigationSpans = navigationRequest.spans;
@@ -69,3 +90,65 @@ sentryTest('should create a navigation transaction on page navigation', async ({
 
   expect(pageloadSpanId).not.toEqual(navigationSpanId);
 });
+
+//
+sentryTest('should handle pushState with full URL', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload');
+  const navigationRequestPromise = waitForTransactionRequest(
+    page,
+    event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/sub-page',
+  );
+  const navigationRequestPromise2 = waitForTransactionRequest(
+    page,
+    event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/sub-page-2',
+  );
+
+  await page.goto(url);
+  await pageloadRequestPromise;
+
+  await page.evaluate("window.history.pushState({}, '', `${window.location.origin}/sub-page`);");
+
+  const navigationRequest = envelopeRequestParser(await navigationRequestPromise);
+
+  expect(navigationRequest.transaction).toEqual('/sub-page');
+
+  expect(navigationRequest.contexts?.trace?.data).toMatchObject({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
+    [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+    [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+    ['sentry.idle_span_finish_reason']: 'idleTimeout',
+  });
+  expect(navigationRequest.request).toEqual({
+    headers: {
+      'User-Agent': expect.any(String),
+    },
+    url: 'http://sentry-test.io/sub-page',
+  });
+
+  await page.evaluate("window.history.pushState({}, '', `${window.location.origin}/sub-page-2`);");
+
+  const navigationRequest2 = envelopeRequestParser(await navigationRequestPromise2);
+
+  expect(navigationRequest2.transaction).toEqual('/sub-page-2');
+
+  expect(navigationRequest2.contexts?.trace?.data).toMatchObject({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
+    [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+    [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+    ['sentry.idle_span_finish_reason']: 'idleTimeout',
+  });
+  expect(navigationRequest2.request).toEqual({
+    headers: {
+      'User-Agent': expect.any(String),
+    },
+    url: 'http://sentry-test.io/sub-page-2',
+  });
+});

packages/browser-utils/src/instrument/history.ts

@@ -47,9 +47,15 @@ export function instrumentHistory(): void {
     return function (this: History, ...args: unknown[]): void {
       const url = args.length > 2 ? args[2] : undefined;
       if (url) {
-        // coerce to string (this is what pushState does)
         const from = lastHref;
-        const to = String(url);
+
+        // Ensure the URL is absolute
+        // this can be either a path, then it is relative to the current origin
+        // or a full URL of the current origin - other origins are not allowed
+        // See: https://developer.mozilla.org/en-US/docs/Web/API/History/pushState#url
+        // coerce to string (this is what pushState does)
+        const to = getAbsoluteUrl(String(url));
+
         // keep track of the current URL state, as we always receive only the updated state
         lastHref = to;
 
@@ -67,3 +73,13 @@ export function instrumentHistory(): void {
   fill(WINDOW.history, 'pushState', historyReplacementFunction);
   fill(WINDOW.history, 'replaceState', historyReplacementFunction);
 }
+
+function getAbsoluteUrl(urlOrPath: string): string {
+  try {
+    const url = new URL(urlOrPath, WINDOW.location.origin);
+    return url.toString();
+  } catch {
+    // fallback, just do nothing
+    return urlOrPath;
+  }
+}

packages/browser/src/helpers.ts

@@ -4,6 +4,7 @@ import {
   addExceptionTypeValue,
   addNonEnumerableProperty,
   captureException,
+  getLocationHref,
   getOriginalFunction,
   GLOBAL_OBJ,
   markFunctionWrapped,
@@ -175,3 +176,24 @@ export function wrap<T extends WrappableFunction, NonFunction>(
 
   return sentryWrapped;
 }
+
+/**
+ * Get HTTP request data from the current page.
+ */
+export function getHttpRequestData(): { url: string; headers: Record<string, string> } {
+  // grab as much info as exists and add it to the event
+  const url = getLocationHref();
+  const { referrer } = WINDOW.document || {};
+  const { userAgent } = WINDOW.navigator || {};
+
+  const headers = {
+    ...(referrer && { Referer: referrer }),
+    ...(userAgent && { 'User-Agent': userAgent }),
+  };
+  const request = {
+    url,
+    headers,
+  };
+
+  return request;
+}

packages/browser/src/integrations/httpcontext.ts

@@ -1,5 +1,5 @@
-import { defineIntegration, getLocationHref } from '@sentry/core';
-import { WINDOW } from '../helpers';
+import { defineIntegration } from '@sentry/core';
+import { getHttpRequestData, WINDOW } from '../helpers';
 
 /**
  * Collects information about HTTP request headers and
@@ -14,23 +14,17 @@ export const httpContextIntegration = defineIntegration(() => {
         return;
       }
 
-      // grab as much info as exists and add it to the event
-      const url = event.request?.url || getLocationHref();
-      const { referrer } = WINDOW.document || {};
-      const { userAgent } = WINDOW.navigator || {};
-
+      const reqData = getHttpRequestData();
       const headers = {
+        ...reqData.headers,
         ...event.request?.headers,
-        ...(referrer && { Referer: referrer }),
-        ...(userAgent && { 'User-Agent': userAgent }),
       };
-      const request = {
+
+      event.request = {
+        ...reqData,
         ...event.request,
-        ...(url && { url }),
         headers,
       };
-
-      event.request = request;
     },
   };
 });

packages/browser/src/tracing/browserTracingIntegration.ts

@@ -12,6 +12,7 @@ import {
   getLocationHref,
   GLOBAL_OBJ,
   logger,
+  parseStringToURLObject,
   propagationContextFromHeaders,
   registerSpanErrorInstrumentation,
   SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
@@ -33,7 +34,7 @@ import {
   startTrackingWebVitals,
 } from '@sentry-internal/browser-utils';
 import { DEBUG_BUILD } from '../debug-build';
-import { WINDOW } from '../helpers';
+import { getHttpRequestData, WINDOW } from '../helpers';
 import { registerBackgroundTabDetection } from './backgroundtab';
 import { linkTraces } from './linkedTraces';
 import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request';
@@ -399,7 +400,14 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         maybeEndActiveSpan();
 
         getIsolationScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
-        getCurrentScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
+
+        const scope = getCurrentScope();
+        scope.setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
+        // We reset this to ensure we do not have lingering incorrect data here
+        // places that call this hook may set this where appropriate - else, the URL at span sending time is used
+        scope.setSDKProcessingMetadata({
+          normalizedRequest: undefined,
+        });
 
         _createRouteSpan(client, {
           op: 'navigation',
@@ -417,7 +425,15 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         const baggage = traceOptions.baggage || getMetaContent('baggage');
 
         const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
-        getCurrentScope().setPropagationContext(propagationContext);
+
+        const scope = getCurrentScope();
+        scope.setPropagationContext(propagationContext);
+
+        // We store the normalized request data on the scope, so we get the request data at time of span creation
+        // otherwise, the URL etc. may already be of the following navigation, and we'd report the wrong URL
+        scope.setSDKProcessingMetadata({
+          normalizedRequest: getHttpRequestData(),
+        });
 
         _createRouteSpan(client, {
           op: 'pageload',
@@ -459,16 +475,25 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
               return;
             }
 
-            if (from !== to) {
-              startingUrl = undefined;
-              startBrowserTracingNavigationSpan(client, {
-                name: WINDOW.location.pathname,
-                attributes: {
-                  [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
-                  [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
-                },
-              });
-            }
+            startingUrl = undefined;
+            const parsed = parseStringToURLObject(to);
+            startBrowserTracingNavigationSpan(client, {
+              name: parsed?.pathname || WINDOW.location.pathname,
+              attributes: {
+                [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+                [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
+              },
+            });
+
+            // We store the normalized request data on the scope, so we get the request data at time of span creation
+            // otherwise, the URL etc. may already be of the following navigation, and we'd report the wrong URL
+            getCurrentScope().setSDKProcessingMetadata({
+              normalizedRequest: {
+                ...getHttpRequestData(),
+                // Ensure to set this, so this matches the target route even if the URL has not yet been updated
+                url: to,
+              },
+            });
           });
         }
       }

packages/core/src/tracing/sentrySpan.ts

@@ -336,6 +336,8 @@ export class SentrySpan implements Span {
 
     const { scope: capturedSpanScope, isolationScope: capturedSpanIsolationScope } = getCapturedScopesOnSpan(this);
 
+    const normalizedRequest = capturedSpanScope?.getScopeData().sdkProcessingMetadata?.normalizedRequest;
+
     if (this._sampled !== true) {
       return undefined;
     }
@@ -374,6 +376,7 @@ export class SentrySpan implements Span {
         capturedSpanIsolationScope,
         dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
       },
+      request: normalizedRequest,
       ...(source && {
         transaction_info: {
           source,

packages/core/src/types-hoist/instrument.ts

@@ -78,7 +78,9 @@ export interface HandlerDataConsole {
 }
 
 export interface HandlerDataHistory {
+  /** The full URL of the previous page */
   from: string | undefined;
+  /** The full URL of the new page */
   to: string;
 }