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

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-async.ts

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

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,42 @@
+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();
+});
+
+test('records a client report and no extra error event when an async event processor rejects', async () => {
+  await createRunner(__dirname, 'scenario-async.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-fallback.ts

@@ -0,0 +1,19 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  traceLifecycle: 'static',
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+  tracesSampleRate: 1,
+  tracesSampler: () => {
+    throw new Error('tracesSampler failed');
+  },
+});
+
+Sentry.startSpan({ name: 'sampled via tracesSampleRate fallback' }, () => {
+  // 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/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,35 @@
+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();
+});
+
+test('sends the span when tracesSampler throws but tracesSampleRate is 1', async () => {
+  await createRunner(__dirname, 'scenario-fallback.ts')
+    .expect({
+      transaction: {
+        transaction: 'sampled via tracesSampleRate fallback',
+      },
+    })
+    .start()
+    .completed();
+});

packages/core/src/breadcrumbs.ts

@@ -1,6 +1,8 @@
 import { getClient, getIsolationScope } from './currentScopes';
+import { DEBUG_BUILD } from './debug-build';
 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 +30,11 @@ export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): vo
   const timestamp = dateTimestampInSeconds();
   const mergedBreadcrumb = { timestamp, ...breadcrumb };
   const finalBreadcrumb = beforeBreadcrumb
-    ? consoleSandbox(() => beforeBreadcrumb(mergedBreadcrumb, hint))
+    ? safeCallback(
+        DEBUG_BUILD ? '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(
+      DEBUG_BUILD ? '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(
+        DEBUG_BUILD ? '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(
+    DEBUG_BUILD ? `${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(
+        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,
+      )
+    : processedLog;
   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,6 +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 { getCombinedScopeData } from '../utils/scopeData';
 import { getActiveSpan } from '../utils/spanUtils';
 import { timestampInSeconds } from '../utils/time';
@@ -181,9 +182,16 @@ export function _INTERNAL_captureMetric(beforeMetric: Metric, options?: Internal
 
   client.emit('processMetric', enrichedMetric);
 
-  const processedMetric = beforeSendMetric ? beforeSendMetric(enrichedMetric) : enrichedMetric;
+  const processedMetric = beforeSendMetric
+    ? safeCallback(
+        DEBUG_BUILD ? 'The `beforeSendMetric` callback threw an error, dropping the metric:' : '',
+        () => beforeSendMetric(enrichedMetric),
+        () => null,
+      )
+    : enrichedMetric;
 
   if (!processedMetric) {
+    client.recordDroppedEvent('before_send', 'metric', 1);
     DEBUG_BUILD && debug.log('`beforeSendMetric` returned `null`, will not send metric.');
     return;
   }