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

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

@@ -20,7 +20,7 @@ import type { EventDropReason, Outcome } from './types/clientreport';
 import type { DataCategory } from './types/datacategory';
 import type { DsnComponents } from './types/dsn';
 import type { DynamicSamplingContext, Envelope } from './types/envelope';
-import type { ErrorEvent, Event, EventHint, EventType, TransactionEvent } from './types/event';
+import type { ErrorEvent, Event, EventHint, TransactionEvent } from './types/event';
 import type { EventProcessor } from './types/eventprocessor';
 import type { FeedbackEvent } from './types/feedback';
 import type { Integration } from './types/integration';
@@ -41,7 +41,7 @@ import type { ResolvedDataCollection } from './types/datacollection';
 import { createClientReportEnvelope } from './utils/clientreport';
 import { consoleSandbox, debug } from './utils/debug-logger';
 import { dsnToString, makeDsn } from './utils/dsn';
-import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils/envelope';
+import { addItemToEnvelope, createAttachmentEnvelopeItem, getDataCategoryByType } from './utils/envelope';
 import { getPossibleEventMessages } from './utils/eventUtils';
 import { isObjectLike, isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils/is';
 import { merge } from './utils/merge';
@@ -1515,6 +1515,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     const isError = isErrorEvent(event);
     const eventType = event.type || 'error';
     const beforeSendLabel = `before send for type \`${eventType}\``;
+    let beforeSendDropReason: 'before_send' | 'callback_error' = 'before_send';
 
     // 1.0 === 100% events are sent
     // 0.0 === 0% events are sent
@@ -1525,7 +1526,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     return this._prepareEvent(event, hint, currentScope, isolationScope)
       .then(prepared => {
         if (prepared === null) {
-          this.recordDroppedEvent('event_processor', dataCategory);
           throw _makeDoNotSendEventError('An event processor returned `null`, will not send event.');
         }
 
@@ -1534,19 +1534,21 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
           return prepared;
         }
 
-        const result = processBeforeSend(this, options, prepared, hint);
+        const result = processBeforeSend(this, options, prepared, hint, () => {
+          beforeSendDropReason = 'callback_error';
+        });
         return _validateBeforeSendResult(result, beforeSendLabel);
       })
       .then(processedEvent => {
         if (processedEvent === null) {
-          this.recordDroppedEvent('before_send', dataCategory);
+          this.recordDroppedEvent(beforeSendDropReason, 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);
+            this.recordDroppedEvent(beforeSendDropReason, 'span', 1 + spans.length);
           }
-          throw _makeDoNotSendEventError(`${beforeSendLabel} returned \`null\`, will not send event.`);
+          const dropMessage = beforeSendDropReason === 'callback_error' ? 'threw an error' : 'returned `null`';
+          throw _makeDoNotSendEventError(`${beforeSendLabel} ${dropMessage}, will not send event.`);
         }
 
         const session = currentScope.getSession() || isolationScope.getSession();
@@ -1689,10 +1691,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   ): PromiseLike<Event>;
 }
 
-function getDataCategoryByType(type: EventType | 'replay_event' | undefined): DataCategory {
-  return type === 'replay_event' ? 'replay' : type || 'error';
-}
-
 /**
  * Verifies that return value of configured `beforeSend` or `beforeSendTransaction` is of expected type, and returns the value if so.
  */
@@ -1727,6 +1725,7 @@ function processBeforeSend(
   options: ClientOptions,
   event: Event,
   hint: EventHint,
+  onCallbackError: () => void,
 ): PromiseLike<Event | null> | Event | null {
   const {
     beforeSend,
@@ -1743,7 +1742,10 @@ function processBeforeSend(
     return safeCallback(
       DEBUG_BUILD ? 'The `beforeSend` callback threw an error, dropping the event:' : '',
       () => beforeSend(errorEvent, hint),
-      () => null,
+      () => {
+        onCallbackError();
+        return null;
+      },
     );
   }
 
@@ -1818,7 +1820,10 @@ function processBeforeSend(
       return safeCallback(
         DEBUG_BUILD ? 'The `beforeSendTransaction` callback threw an error, dropping the event:' : '',
         () => beforeSendTransaction(processedEvent as TransactionEvent, hint),
-        () => null,
+        () => {
+          onCallbackError();
+          return null;
+        },
       );
     }
   }

packages/core/src/eventProcessors.ts

@@ -6,6 +6,8 @@ import { isThenable } from './utils/is';
 import { safeCallback } from './utils/safeCallback';
 import { rejectedSyncPromise, resolvedSyncPromise } from './utils/syncpromise';
 
+type EventProcessorDropReason = 'event_processor' | 'callback_error';
+
 /**
  * Process an array of event processors, returning the processed event (or `null` if the event was dropped).
  */
@@ -14,9 +16,10 @@ export function notifyEventProcessors(
   event: Event | null,
   hint: EventHint,
   index: number = 0,
+  onDrop?: (reason: EventProcessorDropReason) => void,
 ): PromiseLike<Event | null> {
   try {
-    const result = _notifyEventProcessors(event, hint, processors, index);
+    const result = _notifyEventProcessors(event, hint, processors, index, onDrop);
     return isThenable(result) ? result : resolvedSyncPromise(result);
   } catch (error) {
     return rejectedSyncPromise(error);
@@ -28,6 +31,7 @@ function _notifyEventProcessors(
   hint: EventHint,
   processors: EventProcessor[],
   index: number,
+  onDrop?: (reason: EventProcessorDropReason) => void,
 ): Event | null | PromiseLike<Event | null> {
   const processor = processors[index];
 
@@ -36,18 +40,32 @@ function _notifyEventProcessors(
   }
 
   const processorName = `Event processor "${processor.id || '?'}"`;
+  let callbackError = false;
 
   const result = safeCallback(
     DEBUG_BUILD ? `${processorName} threw an error, dropping event:` : '',
     () => processor({ ...event }, hint),
-    () => null,
+    () => {
+      callbackError = true;
+      return null;
+    },
   );
 
   DEBUG_BUILD && result === null && debug.log(`${processorName} dropped event`);
 
   if (isThenable(result)) {
-    return result.then(final => _notifyEventProcessors(final, hint, processors, index + 1));
+    return result.then(final => {
+      if (!final) {
+        onDrop?.(callbackError ? 'callback_error' : 'event_processor');
+        return null;
+      }
+      return _notifyEventProcessors(final, hint, processors, index + 1, onDrop);
+    });
   }
 
-  return _notifyEventProcessors(result, hint, processors, index + 1);
+  if (!result) {
+    onDrop?.(callbackError ? 'callback_error' : 'event_processor');
+    return null;
+  }
+  return _notifyEventProcessors(result, hint, processors, index + 1, onDrop);
 }

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

@@ -16,15 +16,17 @@ export function sampleSpan(
   options: Pick<CoreOptions, 'tracesSampleRate' | 'tracesSampler'>,
   samplingContext: SamplingContext,
   sampleRand: number,
-): [sampled: boolean, sampleRate?: number, localSampleRateWasApplied?: boolean] {
+): [sampled: boolean, sampleRate?: number, localSampleRateWasApplied?: boolean, dropReason?: 'callback_error'] {
   // nothing to do if span recording is not enabled
   if (!hasSpansEnabled(options)) {
     return [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 [false, undefined, undefined, 'callback_error'];
   }
   const [sampleRate, localSampleRateWasApplied] = resolved;
 

packages/core/src/tracing/trace.ts

@@ -495,7 +495,7 @@ function _startRootSpan(
   const currentPropagationContext = scope.getPropagationContext();
   const _isTracingSuppressed = isTracingSuppressed(scope);
 
-  const [sampled, sampleRate, localSampleRateWasApplied] = _isTracingSuppressed
+  const [sampled, sampleRate, localSampleRateWasApplied, dropReason] = _isTracingSuppressed
     ? [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'

packages/core/src/utils/envelope.ts

@@ -10,7 +10,7 @@ import type {
   EnvelopeItemType,
   EventEnvelopeHeaders,
 } from '../types/envelope';
-import type { Event } from '../types/event';
+import type { Event, EventType } from '../types/event';
 import type { SdkInfo } from '../types/sdkinfo';
 import type { SdkMetadata } from '../types/sdkmetadata';
 import { dsnToString } from './dsn';
@@ -251,3 +251,10 @@ export function createEventEnvelopeHeaders(
     }),
   };
 }
+
+/**
+ * Maps an event type to the data category used for client reports.
+ */
+export function getDataCategoryByType(type: EventType): DataCategory {
+  return type === 'replay_event' ? 'replay' : type || 'error';
+}

packages/core/src/utils/prepareEvent.ts

@@ -7,6 +7,7 @@ import type { Event, EventHint } from '../types/event';
 import type { ClientOptions } from '../types/options';
 import type { StackParser } from '../types/stacktrace';
 import { getFilenameToDebugIdMap } from './debug-ids';
+import { getDataCategoryByType } from './envelope';
 import { addExceptionMechanismToCapturedException, uuid4 } from './misc';
 import { normalize } from './normalize';
 import { applyScopeDataToEvent, applySpanToEvent, getCombinedScopeData } from './scopeData';
@@ -36,7 +37,8 @@ export type ExclusiveEventHintOrCaptureContext =
  * @param event The original event.
  * @param hint May contain additional information about the original exception.
  * @param scope A scope containing event metadata.
- * @returns A new event with more information.
+ * @returns A new event with more information, or `null` if an event processor dropped it (or threw). In that case the
+ *   drop has already been recorded on the client, so callers must not record it again.
  * @hidden
  */
 export function prepareEvent(
@@ -102,19 +104,30 @@ export function prepareEvent(
   // Skip event processors for internal exceptions to prevent recursion
   // oxlint-disable-next-line typescript/prefer-optional-chain
   const isInternalException = hint.data && (hint.data as { __sentry__: boolean }).__sentry__ === true;
-  const result = isInternalException
+  const result: PromiseLike<Event | null> = isInternalException
     ? resolvedSyncPromise(prepared)
-    : notifyEventProcessors(eventProcessors, prepared, hint);
+    : notifyEventProcessors(eventProcessors, prepared, hint, 0, reason => {
+        if (!client) {
+          return;
+        }
+
+        client.recordDroppedEvent(reason, getDataCategoryByType(event.type));
+        if (reason === 'callback_error' && event.type === 'transaction') {
+          client.recordDroppedEvent(reason, 'span', 1 + (event.spans || []).length);
+        }
+      });
 
   return result.then(evt => {
-    if (evt) {
-      // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified
-      // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed.
-      // This should not cause any PII issues, since we're only moving data that is already on the event and not adding
-      // any new data
-      applyDebugMeta(evt);
+    if (!evt) {
+      return null;
     }
 
+    // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified
+    // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed.
+    // This should not cause any PII issues, since we're only moving data that is already on the event and not adding
+    // any new data
+    applyDebugMeta(evt);
+
     if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
       return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth);
     }