Merge branch 'develop' into timfish/feat/pino-integration

timfish · web-flow · commit 6b8f76e4ca5f · 2025-09-15T20:37:27.000+02:00
diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/init.js
@@ -0,0 +1,13 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  tracesSampleRate: 1,
+  integrations: [
+    Sentry.browserTracingIntegration({
+      _experiments: { enableInteractions: true },
+    }),
+  ],
+});
diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/subject.js
@@ -0,0 +1,7 @@
+// Clicking the navigate button will push a new history state, triggering navigation
+document.querySelector('[data-test-id=navigate-button]').addEventListener('click', () => {
+  const loc = window.location;
+  const url = loc.href.includes('#nav') ? loc.pathname : `${loc.pathname}#nav`;
+
+  history.pushState({}, '', url);
+});
diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/template.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button data-test-id="navigate-button">Navigate</button>
+  </body>
+  </html>
diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions-navigation-click/test.ts
@@ -0,0 +1,39 @@
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
+
+sentryTest(
+  'click-triggered navigation should produce a root navigation transaction',
+  async ({ getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+
+    await page.goto(url);
+    await waitForTransactionRequest(page); // "pageload" root span
+
+    const interactionRequestPromise = waitForTransactionRequest(
+      page,
+      evt => evt.contexts?.trace?.op === 'ui.action.click',
+    );
+    const navigationRequestPromise = waitForTransactionRequest(page, evt => evt.contexts?.trace?.op === 'navigation');
+
+    await page.locator('[data-test-id=navigate-button]').click();
+
+    const interactionEvent = envelopeRequestParser(await interactionRequestPromise);
+    const navigationEvent = envelopeRequestParser(await navigationRequestPromise);
+
+    // Navigation is root span, not a child span on the interaction
+    expect(interactionEvent.contexts?.trace?.op).toBe('ui.action.click');
+    expect(navigationEvent.contexts?.trace?.op).toBe('navigation');
+
+    expect(interactionEvent.contexts?.trace?.trace_id).not.toEqual(navigationEvent.contexts?.trace?.trace_id);
+
+    // does not contain a child navigation span
+    const interactionSpans = interactionEvent.spans || [];
+    const hasNavigationChild = interactionSpans.some(span => span.op === 'navigation' || span.op === 'http.server');
+    expect(hasNavigationChild).toBeFalsy();
+  },
+);
diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-hono/package.json b/dev-packages/e2e-tests/test-applications/cloudflare-hono/package.json
@@ -12,7 +12,7 @@
   },
   "dependencies": {
     "@sentry/cloudflare": "latest || *",
-    "hono": "4.7.10"
+    "hono": "4.9.7"
   },
   "devDependencies": {
     "@cloudflare/vitest-pool-workers": "^0.8.31",
diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -536,6 +536,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         _createRouteSpan(client, {
           op: 'navigation',
           ...startSpanOptions,
+          // Navigation starts a new trace and is NOT parented under any active interaction (e.g. ui.action.click)
+          parentSpan: null,
+          forceTransaction: true,
         });
       });
 
diff --git a/packages/core/src/eventProcessors.ts b/packages/core/src/eventProcessors.ts
@@ -3,7 +3,7 @@ import type { Event, EventHint } from './types-hoist/event';
 import type { EventProcessor } from './types-hoist/eventprocessor';
 import { debug } from './utils/debug-logger';
 import { isThenable } from './utils/is';
-import { SyncPromise } from './utils/syncpromise';
+import { rejectedSyncPromise, resolvedSyncPromise } from './utils/syncpromise';
 
 /**
  * Process an array of event processors, returning the processed event (or `null` if the event was dropped).
@@ -14,24 +14,33 @@ export function notifyEventProcessors(
   hint: EventHint,
   index: number = 0,
 ): PromiseLike<Event | null> {
-  return new SyncPromise<Event | null>((resolve, reject) => {
-    const processor = processors[index];
-    if (event === null || typeof processor !== 'function') {
-      resolve(event);
-    } else {
-      const result = processor({ ...event }, hint) as Event | null;
+  try {
+    const result = _notifyEventProcessors(event, hint, processors, index);
+    return isThenable(result) ? result : resolvedSyncPromise(result);
+  } catch (error) {
+    return rejectedSyncPromise(error);
+  }
+}
+
+function _notifyEventProcessors(
+  event: Event | null,
+  hint: EventHint,
+  processors: EventProcessor[],
+  index: number,
+): Event | null | PromiseLike<Event | null> {
+  const processor = processors[index];
+
+  if (!event || !processor) {
+    return event;
+  }
+
+  const result = processor({ ...event }, hint);
+
+  DEBUG_BUILD && result === null && debug.log(`Event processor "${processor.id || '?'}" dropped event`);
 
-      DEBUG_BUILD && processor.id && result === null && debug.log(`Event processor "${processor.id}" dropped event`);
+  if (isThenable(result)) {
+    return result.then(final => _notifyEventProcessors(final, hint, processors, index + 1));
+  }
 
-      if (isThenable(result)) {
-        void result
-          .then(final => notifyEventProcessors(processors, final, hint, index + 1).then(resolve))
-          .then(null, reject);
-      } else {
-        void notifyEventProcessors(processors, result, hint, index + 1)
-          .then(resolve)
-          .then(null, reject);
-      }
-    }
-  });
+  return _notifyEventProcessors(result, hint, processors, index + 1);
 }
diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts
@@ -1833,15 +1833,13 @@ describe('Client', () => {
       });
     });
 
-    test('event processor sends an event and logs when it crashes', () => {
-      expect.assertions(3);
-
+    test('event processor sends an event and logs when it crashes synchronously', () => {
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
       const client = new TestClient(options);
       const captureExceptionSpy = vi.spyOn(client, 'captureException');
       const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn');
       const scope = new Scope();
-      const exception = new Error('sorry');
+      const exception = new Error('sorry 1');
       scope.addEventProcessor(() => {
         throw exception;
       });
@@ -1850,7 +1848,43 @@ describe('Client', () => {
 
       expect(TestClient.instance!.event!.exception!.values![0]).toStrictEqual({
         type: 'Error',
-        value: 'sorry',
+        value: 'sorry 1',
+        mechanism: { type: 'internal', handled: false },
+      });
+      expect(captureExceptionSpy).toBeCalledWith(exception, {
+        data: {
+          __sentry__: true,
+        },
+        originalException: exception,
+        mechanism: { type: 'internal', handled: false },
+      });
+      expect(loggerWarnSpy).toBeCalledWith(
+        `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${exception}`,
+      );
+    });
+
+    test('event processor sends an event and logs when it crashes asynchronously', async () => {
+      vi.useFakeTimers();
+
+      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
+      const client = new TestClient(options);
+      const captureExceptionSpy = vi.spyOn(client, 'captureException');
+      const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn');
+      const scope = new Scope();
+      const exception = new Error('sorry 2');
+      scope.addEventProcessor(() => {
+        return new Promise((_resolve, reject) => {
+          reject(exception);
+        });
+      });
+
+      client.captureEvent({ message: 'hello' }, {}, scope);
+
+      await vi.runOnlyPendingTimersAsync();
+
+      expect(TestClient.instance!.event!.exception!.values![0]).toStrictEqual({
+        type: 'Error',
+        value: 'sorry 2',
         mechanism: { type: 'internal', handled: false },
       });
       expect(captureExceptionSpy).toBeCalledWith(exception, {
@@ -1865,6 +1899,81 @@ describe('Client', () => {
       );
     });
 
+    test('event processor sends an event and logs when it crashes synchronously in processor chain', () => {
+      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
+      const client = new TestClient(options);
+      const captureExceptionSpy = vi.spyOn(client, 'captureException');
+      const scope = new Scope();
+      const exception = new Error('sorry 3');
+
+      const processor1 = vi.fn(event => {
+        return event;
+      });
+      const processor2 = vi.fn(() => {
+        throw exception;
+      });
+      const processor3 = vi.fn(event => {
+        return event;
+      });
+
+      scope.addEventProcessor(processor1);
+      scope.addEventProcessor(processor2);
+      scope.addEventProcessor(processor3);
+
+      client.captureEvent({ message: 'hello' }, {}, scope);
+
+      expect(processor1).toHaveBeenCalledTimes(1);
+      expect(processor2).toHaveBeenCalledTimes(1);
+      expect(processor3).toHaveBeenCalledTimes(0);
+
+      expect(captureExceptionSpy).toBeCalledWith(exception, {
+        data: {
+          __sentry__: true,
+        },
+        originalException: exception,
+        mechanism: { type: 'internal', handled: false },
+      });
+    });
+
+    test('event processor sends an event and logs when it crashes asynchronously in processor chain', async () => {
+      vi.useFakeTimers();
+
+      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
+      const client = new TestClient(options);
+      const captureExceptionSpy = vi.spyOn(client, 'captureException');
+      const scope = new Scope();
+      const exception = new Error('sorry 4');
+
+      const processor1 = vi.fn(async event => {
+        return event;
+      });
+      const processor2 = vi.fn(async () => {
+        throw exception;
+      });
+      const processor3 = vi.fn(event => {
+        return event;
+      });
+
+      scope.addEventProcessor(processor1);
+      scope.addEventProcessor(processor2);
+      scope.addEventProcessor(processor3);
+
+      client.captureEvent({ message: 'hello' }, {}, scope);
+      await vi.runOnlyPendingTimersAsync();
+
+      expect(processor1).toHaveBeenCalledTimes(1);
+      expect(processor2).toHaveBeenCalledTimes(1);
+      expect(processor3).toHaveBeenCalledTimes(0);
+
+      expect(captureExceptionSpy).toBeCalledWith(exception, {
+        data: {
+          __sentry__: true,
+        },
+        originalException: exception,
+        mechanism: { type: 'internal', handled: false },
+      });
+    });
+
     test('records events dropped due to `sampleRate` option', () => {
       expect.assertions(1);
 
