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

Wraps `beforeSend`, `beforeSendTransaction`, event processors,
`tracesSampler`, `beforeBreadcrumb`, `beforeSendLog` and
`beforeSendMetric` in the `safeCallback` helper from #23760. A throwing
or rejecting callback no longer escapes into the calling code and is no
longer captured as an `internal` error event; the
event/breadcrumb/log/metric is dropped, a client report is recorded
where a category exists, and the error is logged in debug mode.

part of #23755

---------

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-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,
+      );
     }
   }