feat(core)!: Isolate throwing user callbacks instead of capturing them as events

Wraps `beforeSend`, `beforeSendTransaction`, event processors, `tracesSampler`,
`beforeBreadcrumb`, `beforeSendLog` and `beforeSendMetric` in `safeCallback`.
A throwing or rejecting callback now drops the event/breadcrumb/log/metric
(with a client report where one exists) instead of escaping into the calling
code or being captured as an `internal` error event. `tracesSampler` falls back
to the parent sampling decision, then `tracesSampleRate`.

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

Author: msonnb

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

@@ -0,0 +1,15 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+  beforeSend() {
+    throw new Error('beforeSend failed');
+  },
+});
+
+Sentry.captureException(new Error('this should get dropped because beforeSend throws'));
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+Sentry.flush();

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

@@ -0,0 +1,24 @@
+import { afterAll, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('records a client report and no extra error event when beforeSend throws', async () => {
+  await createRunner(__dirname, 'scenario.ts')
+    .unignore('client_report')
+    .expect({
+      client_report: {
+        discarded_events: [
+          {
+            category: 'error',
+            quantity: 1,
+            reason: 'before_send',
+          },
+        ],
+      },
+    })
+    .start()
+    .completed();
+});

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

@@ -0,0 +1,16 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+});
+
+Sentry.addEventProcessor(() => {
+  throw new Error('event processor failed');
+});
+
+Sentry.captureException(new Error('this should get dropped because the event processor throws'));
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+Sentry.flush();

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

@@ -0,0 +1,24 @@
+import { afterAll, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('records a client report and no extra error event when an event processor throws', async () => {
+  await createRunner(__dirname, 'scenario.ts')
+    .unignore('client_report')
+    .expect({
+      client_report: {
+        discarded_events: [
+          {
+            category: 'error',
+            quantity: 1,
+            reason: 'event_processor',
+          },
+        ],
+      },
+    })
+    .start()
+    .completed();
+});

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

@@ -0,0 +1,17 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+  tracesSampler: () => {
+    throw new Error('tracesSampler failed');
+  },
+});
+
+Sentry.startSpan({ name: 'this should not be sampled because tracesSampler throws' }, () => {
+  // no-op
+});
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+Sentry.flush();

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

@@ -0,0 +1,24 @@
+import { afterAll, test } from 'vitest';
+import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('records a client report and no error event when tracesSampler throws', async () => {
+  await createRunner(__dirname, 'scenario.ts')
+    .unignore('client_report')
+    .expect({
+      client_report: {
+        discarded_events: [
+          {
+            category: 'span',
+            quantity: 1,
+            reason: 'sample_rate',
+          },
+        ],
+      },
+    })
+    .start()
+    .completed();
+});

packages/core/src/breadcrumbs.ts

@@ -1,6 +1,7 @@
 import { getClient, getIsolationScope } from './currentScopes';
 import type { Breadcrumb, BreadcrumbHint } from './types/breadcrumb';
 import { consoleSandbox } from './utils/debug-logger';
+import { safeCallback } from './utils/safeCallback';
 import { dateTimestampInSeconds } from './utils/time';
 
 /**
@@ -28,7 +29,11 @@ export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): vo
   const timestamp = dateTimestampInSeconds();
   const mergedBreadcrumb = { timestamp, ...breadcrumb };
   const finalBreadcrumb = beforeBreadcrumb
-    ? consoleSandbox(() => beforeBreadcrumb(mergedBreadcrumb, hint))
+    ? safeCallback(
+        'The `beforeBreadcrumb` callback threw an error, dropping the breadcrumb:',
+        () => consoleSandbox(() => beforeBreadcrumb(mergedBreadcrumb, hint)),
+        () => null,
+      )
     : mergedBreadcrumb;
 
   if (finalBreadcrumb === null) return;

packages/core/src/client.ts

@@ -50,6 +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 { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span';
 import { safeUnref } from './utils/timer';
 import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent';
@@ -1738,7 +1739,12 @@ function processBeforeSend(
   let processedEvent = event;
 
   if (isErrorEvent(processedEvent) && beforeSend) {
-    return beforeSend(processedEvent, hint);
+    const errorEvent = processedEvent;
+    return safeCallback(
+      'The `beforeSend` callback threw an error, dropping the event:',
+      () => beforeSend(errorEvent, hint),
+      () => null,
+    );
   }
 
   if (isTransactionEvent(processedEvent)) {
@@ -1809,7 +1815,11 @@ function processBeforeSend(
           spanCountBeforeProcessing: spanCountBefore,
         };
       }
-      return beforeSendTransaction(processedEvent as TransactionEvent, hint);
+      return safeCallback(
+        'The `beforeSendTransaction` callback threw an error, dropping the event:',
+        () => beforeSendTransaction(processedEvent as TransactionEvent, hint),
+        () => null,
+      );
     }
   }
 

packages/core/src/eventProcessors.ts

@@ -3,6 +3,7 @@ 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 { rejectedSyncPromise, resolvedSyncPromise } from './utils/syncpromise';
 
 /**
@@ -34,9 +35,15 @@ function _notifyEventProcessors(
     return event;
   }
 
-  const result = processor({ ...event }, hint);
+  const processorName = `Event processor "${processor.id || '?'}"`;
 
-  DEBUG_BUILD && result === null && debug.log(`Event processor "${processor.id || '?'}" dropped event`);
+  const result = safeCallback(
+    `${processorName} threw an error, dropping event:`,
+    () => processor({ ...event }, hint),
+    () => null,
+  );
+
+  DEBUG_BUILD && result === null && debug.log(`${processorName} dropped event`);
 
   if (isThenable(result)) {
     return result.then(final => _notifyEventProcessors(final, hint, processors, index + 1));

packages/core/src/logs/internal.ts

@@ -8,6 +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 { getCombinedScopeData } from '../utils/scopeData';
 import { getActiveSpan } from '../utils/spanUtils';
 import { timestampInSeconds } from '../utils/time';
@@ -142,8 +143,14 @@ export function _INTERNAL_captureLog(
 
   client.emit('beforeCaptureLog', processedLog);
 
-  // We need to wrap this in `consoleSandbox` to avoid recursive calls to `beforeSendLog`
-  const log = beforeSendLog ? consoleSandbox(() => beforeSendLog(processedLog)) : processedLog;
+  const log = beforeSendLog
+    ? safeCallback(
+        '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,
+      )
+    : processedLog;
   if (!log) {
     client.recordDroppedEvent('before_send', 'log_item', 1);
     DEBUG_BUILD && debug.warn('beforeSendLog returned null, log will not be captured.');