fix(core): Catch beforeSendSpan callback throws

packages/core/src/client.ts

@@ -51,11 +51,11 @@ import { prepareEvent } from './utils/prepareEvent';
 import { makePromiseBuffer, type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from './utils/promisebuffer';
 import { safeMathRandom } from './utils/randomSafeContext';
 import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
-import { showSpanDropWarning } from './utils/spanUtils';
 import { safeUnref } from './utils/timer';
 import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
 import { maybeWarnAboutIgnoredTransactionOptions } from './utils/warnAboutIgnoredTransactionOptions';
 import { resolveDataCollectionOptions } from './utils/data-collection/resolveDataCollectionOptions';
+import { applyBeforeSendSpanCallback } from './tracing/spans/beforeSendSpan';
 
 const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
 const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of missing or non-string release';
@@ -1729,13 +1729,9 @@ function processBeforeSend(
 
       // 1.2 If a `beforeSendSpan` callback is defined, process the root span
       if (beforeSendSpan) {
-        const processedRootSpanJson = beforeSendSpan(rootSpanJson);
-        if (!processedRootSpanJson) {
-          showSpanDropWarning();
-        } else {
-          // update event with processed root span values
-          processedEvent = merge(event, convertSpanJsonToTransactionEvent(processedRootSpanJson));
-        }
+        const processedRootSpanJson = applyBeforeSendSpanCallback(rootSpanJson, beforeSendSpan);
+        // update event with processed root span values
+        processedEvent = merge(event, convertSpanJsonToTransactionEvent(processedRootSpanJson));
       }
 
       // 2. Process child spans
@@ -1756,13 +1752,7 @@ function processBeforeSend(
 
           // 2.b If a `beforeSendSpan` callback is defined, process the child span
           if (beforeSendSpan) {
-            const processedSpan = beforeSendSpan(span);
-            if (!processedSpan) {
-              showSpanDropWarning();
-              processedSpans.push(span);
-            } else {
-              processedSpans.push(processedSpan);
-            }
+            processedSpans.push(applyBeforeSendSpanCallback(span, beforeSendSpan));
           } else {
             processedSpans.push(span);
           }

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

@@ -1,6 +1,8 @@
+import { DEBUG_BUILD } from '../../debug-build';
 import type { BeforeSendStaticSpanCallback, BeforeSendStreamedSpanCallback } from '../../types/options';
-import type { StreamedSpanJSON } from '../../types/span';
+import type { SpanJSON, StreamedSpanJSON } from '../../types/span';
 import { addNonEnumerableProperty } from '../../utils/object';
+import { consoleSandbox, debug } from '../../utils/debug-logger';
 
 /**
  * A wrapper to use the static, transaction-based span format in your `beforeSendSpan` callback.
@@ -53,3 +55,34 @@ export function withStreamedSpan(
 export function isStaticBeforeSendSpanCallback(callback: unknown): callback is BeforeSendStaticSpanCallback {
   return !!callback && typeof callback === 'function' && '_static' in callback && !!callback._static;
 }
+
+let hasShownSpanDropWarning = false;
+/**
+ * Apply a user-provided beforeSendSpan callback to a span JSON.
+ */
+export function applyBeforeSendSpanCallback<T extends StreamedSpanJSON | SpanJSON>(
+  span: T,
+  beforeSendSpan: (span: T) => T,
+): T {
+  try {
+    const modifedSpan = beforeSendSpan(span);
+    if (!modifedSpan) {
+      if (!hasShownSpanDropWarning) {
+        consoleSandbox(() => {
+          // eslint-disable-next-line no-console
+          console.warn(
+            '[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.',
+          );
+        });
+        hasShownSpanDropWarning = true;
+      }
+      return span;
+    }
+    return modifedSpan;
+  } 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;
+  }
+}

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

@@ -15,13 +15,12 @@ import type { SerializedStreamedSpan, Span, SpanAttributeValue, SpanJSON, Stream
 import { getCombinedScopeData } from '../../utils/scopeData';
 import {
   INTERNAL_getSegmentSpan,
-  showSpanDropWarning,
   spanToJSON,
   spanToStreamedSpanJSON,
   streamedSpanJsonToSerializedSpan,
 } from '../../utils/spanUtils';
 import { getCapturedScopesOnSpan } from '../utils';
-import { isStaticBeforeSendSpanCallback } from './beforeSendSpan';
+import { applyBeforeSendSpanCallback, isStaticBeforeSendSpanCallback } from './beforeSendSpan';
 import { spanJsonToSerializedStreamedSpan } from './spanJsonToStreamedSpan';
 import { scopeContextsToSpanAttributes } from './scopeContextAttributes';
 import { DEFAULT_ENVIRONMENT } from '../../constants';
@@ -199,22 +198,7 @@ export function captureStandaloneSpanWithStaticCallback(
     }
   });
 
-  const processedSpan = beforeSendSpan(spanJSON) || (showSpanDropWarning(), spanJSON);
+  const processedSpan = applyBeforeSendSpanCallback(spanJSON, beforeSendSpan);
 
   return spanJsonToSerializedStreamedSpan(processedSpan);
 }
-
-/**
- * Apply a user-provided beforeSendSpan callback to a span JSON.
- */
-export function applyBeforeSendSpanCallback(
-  span: StreamedSpanJSON,
-  beforeSendSpan: (span: StreamedSpanJSON) => StreamedSpanJSON,
-): StreamedSpanJSON {
-  const modifedSpan = beforeSendSpan(span);
-  if (!modifedSpan) {
-    showSpanDropWarning();
-    return span;
-  }
-  return modifedSpan;
-}

packages/core/src/utils/spanUtils.ts

@@ -30,15 +30,12 @@ import { addNonEnumerableProperty } from '../utils/object';
 import { generateSpanId } from '../utils/propagationContext';
 import { timestampInSeconds } from '../utils/time';
 import { generateSentryTraceHeader, generateTraceparentHeader } from '../utils/tracing';
-import { consoleSandbox } from './debug-logger';
 import { _getSpanForScope } from './spanOnScope';
 
 // These are aligned with OpenTelemetry trace flags
 export const TRACE_FLAG_NONE = 0x0;
 export const TRACE_FLAG_SAMPLED = 0x1;
 
-let hasShownSpanDropWarning = false;
-
 /**
  * Convert a span to a trace context, which can be sent as the `trace` context in an event.
  * By default, this will only include trace_id, span_id & parent_span_id.
@@ -442,21 +439,6 @@ export function getActiveSpan(): Span | undefined {
   return _getSpanForScope(getCurrentScope());
 }
 
-/**
- * Logs a warning once if `beforeSendSpan` is used to drop spans.
- */
-export function showSpanDropWarning(): void {
-  if (!hasShownSpanDropWarning) {
-    consoleSandbox(() => {
-      // eslint-disable-next-line no-console
-      console.warn(
-        '[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.',
-      );
-    });
-    hasShownSpanDropWarning = true;
-  }
-}
-
 /**
  * Updates the name of the given span and ensures that the span name is not
  * overwritten by the Sentry SDK.

packages/core/test/lib/client.test.ts

@@ -1684,7 +1684,9 @@ describe('Client', () => {
     test('does not discard span and warn when returning null from `beforeSendSpan', () => {
       const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
 
-      const beforeSendSpan = withStaticSpan(vi.fn(() => null as unknown as SpanJSON));
+      // @ts-expect-error - intentionally violating the type signature here
+      const beforeSendSpan = withStaticSpan(vi.fn(() => null));
+
       const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan });
       const client = new TestClient(options);
 
@@ -1724,6 +1726,57 @@ describe('Client', () => {
       consoleWarnSpy.mockRestore();
     });
 
+    test("doesn't throw if the `beforeSendSpan` callback throws", () => {
+      const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error').mockImplementation(() => undefined);
+      const error = new Error('beforeSendSpan is broken');
+      const beforeSendSpan = withStaticSpan(
+        vi.fn(() => {
+          throw error;
+        }),
+      );
+
+      const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendSpan, debug: true });
+      const client = new TestClient(options);
+
+      const transaction: Event = {
+        transaction: '/dogs/are/great',
+        type: 'transaction',
+        spans: [
+          {
+            description: 'first span',
+            span_id: '9e15bf99fbe4bc80',
+            start_timestamp: 1591603196.637835,
+            trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
+            status: 'ok',
+          },
+          {
+            description: 'second span',
+            span_id: 'aa554c1f506b0783',
+            start_timestamp: 1591603196.637835,
+            trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
+            status: 'ok',
+          },
+        ],
+      };
+
+      expect(() => client.captureEvent(transaction)).not.toThrow();
+
+      expect(beforeSendSpan).toHaveBeenCalledTimes(3);
+
+      const capturedEvent = TestClient.instance!.event!;
+      expect(capturedEvent.spans).toHaveLength(2);
+      expect(client['_outcomes']).toEqual({});
+
+      expect(debugErrorSpy).toHaveBeenCalledTimes(3);
+      expect(debugErrorSpy).toHaveBeenCalledWith(
+        'The `beforeSendSpan` callback threw an error, sending the span unmodified:',
+        error,
+      );
+      debugErrorSpy.mockRestore();
+    });
+
     test('calls `beforeSend` and logs info about invalid return value', () => {
       const invalidValues = [undefined, false, true, [], 1];
       expect.assertions(invalidValues.length * 3);