fix(core): Don't let a throwing beforeSendSpan callback escape into user code

packages/core/src/tracing/spans/captureSpan.ts

@@ -1,5 +1,6 @@
 import type { RawAttributes } from '../../attributes';
 import type { Client } from '../../client';
+import { DEBUG_BUILD } from '../../debug-build';
 import type { ScopeData } from '../../scope';
 import {
   SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT,
@@ -12,6 +13,7 @@ import {
   SEMANTIC_ATTRIBUTE_USER_USERNAME,
 } from '../../semanticAttributes';
 import type { SerializedStreamedSpan, Span, SpanJSON, StreamedSpanJSON } from '../../types/span';
+import { debug } from '../../utils/debug-logger';
 import { getCombinedScopeData } from '../../utils/scopeData';
 import {
   INTERNAL_getSegmentSpan,
@@ -183,7 +185,16 @@ export function applyBeforeSendSpanCallback<T extends StreamedSpanJSON | SpanJSO
   span: T,
   beforeSendSpan: (span: T) => T,
 ): T {
-  const modifedSpan = beforeSendSpan(span);
+  let modifedSpan: T;
+  try {
+    modifedSpan = beforeSendSpan(span);
+  } catch (error) {
+    // Spans are captured synchronously when they end, so a throwing callback would otherwise
+    // propagate into whatever user code ended the span.
+    DEBUG_BUILD && debug.error('The `beforeSendSpan` callback threw an error, sending the span unmodified:', error);
+    return span;
+  }
+
   if (!modifedSpan) {
     showSpanDropWarning();
     return span;

packages/core/test/lib/tracing/spans/captureSpan.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest';
 import type { Contexts, SerializedStreamedSpan, SpanJSON, StreamedSpanJSON } from '../../../../src';
 import {
   captureSpan,
+  debug,
   SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -13,6 +14,7 @@ import {
   SEMANTIC_ATTRIBUTE_USER_ID,
   SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS,
   SEMANTIC_ATTRIBUTE_USER_USERNAME,
+  spanStreamingIntegration,
   startInactiveSpan,
   startSpan,
   withStaticSpan,
@@ -547,6 +549,70 @@ describe('captureSpan', () => {
       consoleWarnSpy.mockRestore();
     });
 
+    it('keeps the span and logs an error if the beforeSendSpan callback throws', () => {
+      const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined);
+      const error = new Error('beforeSendSpan is broken');
+      // A v10 callback that was not migrated to the streamed format throws like this, because
+      // `data` doesn't exist on a `StreamedSpanJSON`.
+      const beforeSendSpan = vi.fn(() => {
+        throw error;
+      });
+
+      const client = new TestClient(
+        getDefaultTestClientOptions({
+          dsn: 'https://dsn@ingest.f00.f00/1',
+          tracesSampleRate: 1,
+          traceLifecycle: 'stream',
+          beforeSendSpan: beforeSendSpan as unknown as TestClientOptions['beforeSendSpan'],
+        }),
+      );
+
+      const span = withScope(scope => {
+        scope.setClient(client);
+        const span = startInactiveSpan({ name: 'my-span', attributes: { 'sentry.op': 'http.client' } });
+        span.end();
+        return span;
+      });
+
+      const serialized = captureSpan(span, client);
+
+      expect(serialized.name).toBe('my-span');
+      expect(serialized.attributes['sentry.op']).toEqual({ type: 'string', value: 'http.client' });
+      expect(debugErrorSpy).toHaveBeenCalledWith(
+        'The `beforeSendSpan` callback threw an error, sending the span unmodified:',
+        error,
+      );
+
+      debugErrorSpy.mockRestore();
+    });
+
+    it("doesn't let a throwing beforeSendSpan callback propagate out of span.end()", () => {
+      const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined);
+      const client = new TestClient(
+        getDefaultTestClientOptions({
+          dsn: 'https://dsn@ingest.f00.f00/1',
+          tracesSampleRate: 1,
+          traceLifecycle: 'stream',
+          integrations: [spanStreamingIntegration()],
+          beforeSendSpan: (() => {
+            throw new Error('beforeSendSpan is broken');
+          }) as unknown as TestClientOptions['beforeSendSpan'],
+        }),
+      );
+
+      // Spans are captured synchronously from the `afterSpanEnd` hook, so a throwing callback
+      // would otherwise surface in user code that ended the span.
+      expect(() =>
+        withScope(scope => {
+          scope.setClient(client);
+          client.init();
+          startSpan({ name: 'my-span' }, () => undefined);
+        }),
+      ).not.toThrow();
+
+      debugErrorSpy.mockRestore();
+    });
+
     // Standalone spans (INP web vital spans) are streamed even with the static trace lifecycle,
     // so a `withStaticSpan` callback has to see them in the v1 format it expects.
     describe('with the static trace lifecycle', () => {