feat(core): Record callback_error client reports for throwing user callbacks

Events, logs, metrics and root spans dropped because a user callback threw
were reported with the same outcome as a legitimate filter (`before_send`,
`event_processor`, `sample_rate`). A dedicated `callback_error` reason makes
them distinguishable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: msonnb

dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send-throws/test.ts

@@ -14,7 +14,7 @@ test('records a client report and no extra error event when beforeSend throws',
           {
             category: 'error',
             quantity: 1,
-            reason: 'before_send',
+            reason: 'callback_error',
           },
         ],
       },

dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processor-throws/test.ts

@@ -14,7 +14,7 @@ test('records a client report and no extra error event when an event processor t
           {
             category: 'error',
             quantity: 1,
-            reason: 'event_processor',
+            reason: 'callback_error',
           },
         ],
       },
@@ -32,7 +32,7 @@ test('records a client report and no extra error event when an async event proce
           {
             category: 'error',
             quantity: 1,
-            reason: 'event_processor',
+            reason: 'callback_error',
           },
         ],
       },

dev-packages/node-integration-tests/suites/client-reports/drop-reasons/traces-sampler-throws/test.ts

@@ -14,7 +14,7 @@ test('records a client report and no error event when tracesSampler throws', asy
           {
             category: 'span',
             quantity: 1,
-            reason: 'sample_rate',
+            reason: 'callback_error',
           },
         ],
       },

packages/core/src/client.ts

@@ -50,7 +50,7 @@ import { parseSampleRate } from './utils/parseSampleRate';
 import { prepareEvent } from './utils/prepareEvent';
 import { makePromiseBuffer, type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from './utils/promisebuffer';
 import { safeMathRandom } from './utils/randomSafeContext';
-import { safeCallback } from './utils/safeCallback';
+import { CALLBACK_ERROR, safeCallback } from './utils/safeCallback';
 import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
 import { safeUnref } from './utils/timer';
 import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
@@ -1522,6 +1522,14 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     const parsedSampleRate = typeof sampleRate === 'undefined' ? undefined : parseSampleRate(sampleRate);
     const dataCategory = getDataCategoryByType(event.type);
 
+    const recordDroppedEvent = (reason: EventDropReason): void => {
+      this.recordDroppedEvent(reason, dataCategory);
+      if (isTransaction) {
+        // the transaction itself counts as one span, plus all the child spans that are added
+        this.recordDroppedEvent(reason, 'span', 1 + (event.spans || []).length);
+      }
+    };
+
     return this._prepareEvent(event, hint, currentScope, isolationScope)
       .then(prepared => {
         if (prepared === null) {
@@ -1539,13 +1547,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
       })
       .then(processedEvent => {
         if (processedEvent === null) {
-          this.recordDroppedEvent('before_send', dataCategory);
-          if (isTransaction) {
-            const spans = event.spans || [];
-            // the transaction itself counts as one span, plus all the child spans that are added
-            const spanCount = 1 + spans.length;
-            this.recordDroppedEvent('before_send', 'span', spanCount);
-          }
+          recordDroppedEvent('before_send');
           throw _makeDoNotSendEventError(`${beforeSendLabel} returned \`null\`, will not send event.`);
         }
 
@@ -1587,6 +1589,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
         return processedEvent;
       })
       .then(null, reason => {
+        if (reason === CALLBACK_ERROR) {
+          recordDroppedEvent('callback_error');
+          throw _makeDoNotSendEventError('A user callback threw an error, will not send event.');
+        }
+
         if (_isDoNotSendEventError(reason) || _isInternalError(reason)) {
           throw reason;
         }
@@ -1702,17 +1709,13 @@ function _validateBeforeSendResult(
 ): PromiseLike<Event | null> | Event | null {
   const invalidValueError = `${beforeSendLabel} must return \`null\` or a valid event.`;
   if (isThenable(beforeSendResult)) {
-    return beforeSendResult.then(
-      event => {
-        if (!isPlainObject(event) && event !== null) {
-          throw _makeInternalError(invalidValueError);
-        }
-        return event;
-      },
-      e => {
-        throw _makeInternalError(`${beforeSendLabel} rejected with ${e}`);
-      },
-    );
+    // A rejection can only be `CALLBACK_ERROR` here, as `safeCallback` already handled the user callback rejecting
+    return beforeSendResult.then(event => {
+      if (!isPlainObject(event) && event !== null) {
+        throw _makeInternalError(invalidValueError);
+      }
+      return event;
+    });
   } else if (!isPlainObject(beforeSendResult) && beforeSendResult !== null) {
     throw _makeInternalError(invalidValueError);
   }
@@ -1743,7 +1746,9 @@ function processBeforeSend(
     return safeCallback(
       DEBUG_BUILD ? 'The `beforeSend` callback threw an error, dropping the event:' : '',
       () => beforeSend(errorEvent, hint),
-      () => null,
+      () => {
+        throw CALLBACK_ERROR;
+      },
     );
   }
 
@@ -1818,7 +1823,9 @@ function processBeforeSend(
       return safeCallback(
         DEBUG_BUILD ? 'The `beforeSendTransaction` callback threw an error, dropping the event:' : '',
         () => beforeSendTransaction(processedEvent as TransactionEvent, hint),
-        () => null,
+        () => {
+          throw CALLBACK_ERROR;
+        },
       );
     }
   }

packages/core/src/eventProcessors.ts

@@ -3,11 +3,12 @@ import type { Event, EventHint } from './types/event';
 import type { EventProcessor } from './types/eventprocessor';
 import { debug } from './utils/debug-logger';
 import { isThenable } from './utils/is';
-import { safeCallback } from './utils/safeCallback';
+import { CALLBACK_ERROR, safeCallback } from './utils/safeCallback';
 import { rejectedSyncPromise, resolvedSyncPromise } from './utils/syncpromise';
 
 /**
  * Process an array of event processors, returning the processed event (or `null` if the event was dropped).
+ * Rejects with `CALLBACK_ERROR` if a processor throws.
  */
 export function notifyEventProcessors(
   processors: EventProcessor[],
@@ -40,7 +41,9 @@ function _notifyEventProcessors(
   const result = safeCallback(
     DEBUG_BUILD ? `${processorName} threw an error, dropping event:` : '',
     () => processor({ ...event }, hint),
-    () => null,
+    () => {
+      throw CALLBACK_ERROR;
+    },
   );
 
   DEBUG_BUILD && result === null && debug.log(`${processorName} dropped event`);

packages/core/src/logs/internal.ts

@@ -8,7 +8,7 @@ import type { Integration } from '../types/integration';
 import type { Log, SerializedLog } from '../types/log';
 import { consoleSandbox, debug } from '../utils/debug-logger';
 import { isParameterizedString } from '../utils/is';
-import { safeCallback } from '../utils/safeCallback';
+import { CALLBACK_ERROR, safeCallback } from '../utils/safeCallback';
 import { getCombinedScopeData } from '../utils/scopeData';
 import { getActiveSpan } from '../utils/spanUtils';
 import { timestampInSeconds } from '../utils/time';
@@ -144,13 +144,17 @@ export function _INTERNAL_captureLog(
   client.emit('beforeCaptureLog', processedLog);
 
   const log = beforeSendLog
-    ? safeCallback(
+    ? safeCallback<Log | null | typeof CALLBACK_ERROR>(
         DEBUG_BUILD ? 'The `beforeSendLog` callback threw an error, dropping the log:' : '',
         // We need to wrap this in `consoleSandbox` to avoid recursive calls to `beforeSendLog`
         () => consoleSandbox(() => beforeSendLog(processedLog)),
-        () => null,
+        () => CALLBACK_ERROR,
       )
     : processedLog;
+  if (log === CALLBACK_ERROR) {
+    client.recordDroppedEvent('callback_error', 'log_item', 1);
+    return;
+  }
   if (!log) {
     client.recordDroppedEvent('before_send', 'log_item', 1);
     DEBUG_BUILD && debug.warn('beforeSendLog returned null, log will not be captured.');

packages/core/src/metrics/internal.ts

@@ -8,7 +8,7 @@ import type { Integration } from '../types/integration';
 import type { Metric, SerializedMetric } from '../types/metric';
 import type { User } from '../types/user';
 import { debug } from '../utils/debug-logger';
-import { safeCallback } from '../utils/safeCallback';
+import { CALLBACK_ERROR, safeCallback } from '../utils/safeCallback';
 import { getCombinedScopeData } from '../utils/scopeData';
 import { getActiveSpan } from '../utils/spanUtils';
 import { timestampInSeconds } from '../utils/time';
@@ -183,13 +183,18 @@ export function _INTERNAL_captureMetric(beforeMetric: Metric, options?: Internal
   client.emit('processMetric', enrichedMetric);
 
   const processedMetric = beforeSendMetric
-    ? safeCallback(
+    ? safeCallback<Metric | null | typeof CALLBACK_ERROR>(
         DEBUG_BUILD ? 'The `beforeSendMetric` callback threw an error, dropping the metric:' : '',
         () => beforeSendMetric(enrichedMetric),
-        () => null,
+        () => CALLBACK_ERROR,
       )
     : enrichedMetric;
 
+  if (processedMetric === CALLBACK_ERROR) {
+    client.recordDroppedEvent('callback_error', 'metric', 1);
+    return;
+  }
+
   if (!processedMetric) {
     client.recordDroppedEvent('before_send', 'metric', 1);
     DEBUG_BUILD && debug.log('`beforeSendMetric` returned `null`, will not send metric.');

packages/core/src/tracing/sampling.ts

@@ -6,6 +6,14 @@ import { hasSpansEnabled } from '../utils/hasSpansEnabled';
 import { parseSampleRate } from '../utils/parseSampleRate';
 import { safeCallback } from '../utils/safeCallback';
 
+interface SamplingDecision {
+  sampled: boolean;
+  sampleRate?: number;
+  localSampleRateWasApplied?: boolean;
+  /** Set when the span was dropped for a reason other than the sampling decision itself. */
+  dropReason?: 'callback_error';
+}
+
 /**
  * Makes a sampling decision for the given options.
  *
@@ -16,15 +24,17 @@ export function sampleSpan(
   options: Pick<CoreOptions, 'tracesSampleRate' | 'tracesSampler'>,
   samplingContext: SamplingContext,
   sampleRand: number,
-): [sampled: boolean, sampleRate?: number, localSampleRateWasApplied?: boolean] {
+): SamplingDecision {
   // nothing to do if span recording is not enabled
   if (!hasSpansEnabled(options)) {
-    return [false];
+    return { sampled: false };
   }
 
   const resolved = resolveSampleRate(options, samplingContext);
   if (!resolved) {
-    return [false];
+    // `hasSpansEnabled` guarantees either `tracesSampleRate` or `tracesSampler` is set, so the only way to end up
+    // without a sample rate is a throwing `tracesSampler` with nothing to fall back to.
+    return { sampled: false, dropReason: 'callback_error' };
   }
   const [sampleRate, localSampleRateWasApplied] = resolved;
 
@@ -39,7 +49,7 @@ export function sampleSpan(
           sampleRate,
         )} of type ${JSON.stringify(typeof sampleRate)}.`,
       );
-    return [false];
+    return { sampled: false };
   }
 
   // if the function returned 0 (or false), or if `tracesSampleRate` is 0, it's a sign the transaction should be dropped
@@ -52,7 +62,7 @@ export function sampleSpan(
             : 'a negative sampling decision was inherited or tracesSampleRate is set to 0'
         }`,
       );
-    return [false, parsedSampleRate, localSampleRateWasApplied];
+    return { sampled: false, sampleRate: parsedSampleRate, localSampleRateWasApplied };
   }
 
   // We always compare the sample rand for the current execution context against the chosen sample rate.
@@ -69,7 +79,7 @@ export function sampleSpan(
       );
   }
 
-  return [shouldSample, parsedSampleRate, localSampleRateWasApplied];
+  return { sampled: shouldSample, sampleRate: parsedSampleRate, localSampleRateWasApplied };
 }
 
 /**

packages/core/src/tracing/trace.ts

@@ -495,8 +495,8 @@ function _startRootSpan(
   const currentPropagationContext = scope.getPropagationContext();
   const _isTracingSuppressed = isTracingSuppressed(scope);
 
-  const [sampled, sampleRate, localSampleRateWasApplied] = _isTracingSuppressed
-    ? [false]
+  const { sampled, sampleRate, localSampleRateWasApplied, dropReason } = _isTracingSuppressed
+    ? { sampled: false }
     : sampleSpan(
         options,
         {
@@ -522,7 +522,7 @@ function _startRootSpan(
 
   if (!sampled && client && !_isTracingSuppressed) {
     DEBUG_BUILD && debug.log('[Tracing] Discarding root span because its trace was not chosen to be sampled.');
-    client.recordDroppedEvent('sample_rate', hasSpanStreamingEnabled(client) ? 'span' : 'transaction');
+    client.recordDroppedEvent(dropReason || 'sample_rate', hasSpanStreamingEnabled(client) ? 'span' : 'transaction');
   }
 
   setCapturedScopesOnSpan(rootSpan, scope, isolationScope);

packages/core/src/types/clientreport.ts

@@ -2,6 +2,7 @@ import type { DataCategory } from './datacategory';
 
 export type EventDropReason =
   | 'before_send'
+  | 'callback_error'
   | 'event_processor'
   | 'network_error'
   | 'queue_overflow'