diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send-throws/scenario.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send-throws/scenario.ts new file mode 100644 index 000000000000..299630f41cce --- /dev/null +++ b/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(); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send-throws/test.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send-throws/test.ts new file mode 100644 index 000000000000..e30038efc57b --- /dev/null +++ b/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(); +}); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processor-throws/scenario.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processor-throws/scenario.ts new file mode 100644 index 000000000000..31d53fa48621 --- /dev/null +++ b/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(); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processor-throws/test.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processor-throws/test.ts new file mode 100644 index 000000000000..8e591de321f5 --- /dev/null +++ b/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(); +}); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/traces-sampler-throws/scenario.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/traces-sampler-throws/scenario.ts new file mode 100644 index 000000000000..fac664dbde15 --- /dev/null +++ b/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(); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/traces-sampler-throws/test.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/traces-sampler-throws/test.ts new file mode 100644 index 000000000000..8c2aff9e9636 --- /dev/null +++ b/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(); +}); diff --git a/packages/core/src/breadcrumbs.ts b/packages/core/src/breadcrumbs.ts index d511c6f5801f..ff623bdb989b 100644 --- a/packages/core/src/breadcrumbs.ts +++ b/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; diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 5a14b13c07fa..8972eac47d80 100644 --- a/packages/core/src/client.ts +++ b/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, + ); } } diff --git a/packages/core/src/eventProcessors.ts b/packages/core/src/eventProcessors.ts index 99a15781e06c..c43552ba9343 100644 --- a/packages/core/src/eventProcessors.ts +++ b/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)); diff --git a/packages/core/src/logs/internal.ts b/packages/core/src/logs/internal.ts index 4b610b288ca8..6387cdaa5065 100644 --- a/packages/core/src/logs/internal.ts +++ b/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.'); diff --git a/packages/core/src/metrics/internal.ts b/packages/core/src/metrics/internal.ts index c884399624b8..d090e38ab83f 100644 --- a/packages/core/src/metrics/internal.ts +++ b/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( + '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; } diff --git a/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index efc743238107..43f62ff48fde 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -4,6 +4,7 @@ import type { SamplingContext } from '../types/samplingcontext'; import { debug } from '../utils/debug-logger'; import { hasSpansEnabled } from '../utils/hasSpansEnabled'; import { parseSampleRate } from '../utils/parseSampleRate'; +import { safeCallback } from '../utils/safeCallback'; /** * Makes a sampling decision for the given options. @@ -21,37 +22,11 @@ export function sampleSpan( return [false]; } - let localSampleRateWasApplied = undefined; - - // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` were defined, so one of these should - // work; prefer the hook if so - let sampleRate; - if (typeof options.tracesSampler === 'function') { - sampleRate = options.tracesSampler({ - ...samplingContext, - inheritOrSampleWith: fallbackSampleRate => { - // If we have an incoming parent sample rate, we'll just use that one. - // The sampling decision will be inherited because of the sample_rand that was generated when the trace reached the incoming boundaries of the SDK. - if (typeof samplingContext.parentSampleRate === 'number') { - return samplingContext.parentSampleRate; - } - - // Fallback if parent sample rate is not on the incoming trace (e.g. if there is no baggage) - // This is to provide backwards compatibility if there are incoming traces from older SDKs that don't send a parent sample rate or a sample rand. In these cases we just want to force either a sampling decision on the downstream traces via the sample rate. - if (typeof samplingContext.parentSampled === 'boolean') { - return Number(samplingContext.parentSampled); - } - - return fallbackSampleRate; - }, - }); - localSampleRateWasApplied = true; - } else if (samplingContext.parentSampled !== undefined) { - sampleRate = samplingContext.parentSampled; - } else if (typeof options.tracesSampleRate !== 'undefined') { - sampleRate = options.tracesSampleRate; - localSampleRateWasApplied = true; + const resolved = resolveSampleRate(options, samplingContext); + if (!resolved) { + return [false]; } + const [sampleRate, localSampleRateWasApplied] = resolved; // Since this is coming from the user (or from a function provided by the user), who knows what we might get. // (The only valid values are booleans or numbers between 0 and 1.) @@ -96,3 +71,55 @@ export function sampleSpan( return [shouldSample, parsedSampleRate, localSampleRateWasApplied]; } + +/** + * Prefers `tracesSampler`. If it throws, falls back to the parent decision, then `tracesSampleRate`. + * Returns `undefined` when there is nothing to fall back to. + */ +function resolveSampleRate( + options: Pick, + samplingContext: SamplingContext, +): [sampleRate: unknown, localSampleRateWasApplied?: boolean] | undefined { + const { tracesSampler, tracesSampleRate } = options; + + if (typeof tracesSampler === 'function') { + const samplerResult = safeCallback( + 'The `tracesSampler` callback threw an error, falling back to the parent sampling decision or `tracesSampleRate`:', + (): [unknown, boolean] => [ + tracesSampler({ + ...samplingContext, + inheritOrSampleWith: fallbackSampleRate => { + // If we have an incoming parent sample rate, we'll just use that one. + // The sampling decision will be inherited because of the sample_rand that was generated when the trace reached the incoming boundaries of the SDK. + if (typeof samplingContext.parentSampleRate === 'number') { + return samplingContext.parentSampleRate; + } + + // Fallback if parent sample rate is not on the incoming trace (e.g. if there is no baggage) + // This is to provide backwards compatibility if there are incoming traces from older SDKs that don't send a parent sample rate or a sample rand. In these cases we just want to force either a sampling decision on the downstream traces via the sample rate. + if (typeof samplingContext.parentSampled === 'boolean') { + return Number(samplingContext.parentSampled); + } + + return fallbackSampleRate; + }, + }), + true, + ], + () => undefined, + ); + if (samplerResult) { + return samplerResult; + } + } + + if (samplingContext.parentSampled !== undefined) { + return [samplingContext.parentSampled]; + } + + if (typeof tracesSampleRate !== 'undefined') { + return [tracesSampleRate, true]; + } + + return undefined; +} diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index e3ee176bf638..e43c35d2fe63 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -412,6 +412,27 @@ describe('Client', () => { expect(isolationScopeBreadcrumbs).toEqual([]); }); + test('calls `beforeBreadcrumb` and discards the breadcrumb when it throws', () => { + const exception = new Error('beforeBreadcrumb failed'); + const beforeBreadcrumb = vi.fn(() => { + throw exception; + }); + const options = getDefaultTestClientOptions({ beforeBreadcrumb }); + const client = new TestClient(options); + setCurrentClient(client); + client.init(); + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + + expect(() => addBreadcrumb({ message: 'hello' })).not.toThrow(); + + const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs; + expect(isolationScopeBreadcrumbs).toEqual([]); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeBreadcrumb` callback threw an error, dropping the breadcrumb:', + exception, + ); + }); + test('`beforeBreadcrumb` gets an access to a hint as a second argument', () => { const beforeBreadcrumb = vi.fn((breadcrumb, hint) => ({ ...breadcrumb, data: hint.data })); const options = getDefaultTestClientOptions({ beforeBreadcrumb }); @@ -2185,11 +2206,12 @@ describe('Client', () => { }); }); - test('event processor sends an event and logs when it crashes synchronously', () => { + test('drops the event and records a client report when an event processor throws synchronously', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); const captureExceptionSpy = vi.spyOn(client, 'captureException'); - const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); const scope = new Scope(); const exception = new Error('sorry 1'); scope.addEventProcessor(() => { @@ -2198,71 +2220,46 @@ describe('Client', () => { client.captureEvent({ message: 'hello' }, {}, scope); - expect(TestClient.instance!.event!.exception!.values![0]).toStrictEqual({ - type: 'Error', - value: 'sorry 1', - mechanism: { type: 'internal', handled: false }, - }); - expect(captureExceptionSpy).toBeCalledWith(exception, { - data: { - __sentry__: true, - }, - originalException: exception, - mechanism: { type: 'internal', handled: false }, - }); - expect(loggerWarnSpy).toBeCalledWith( - `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${exception}`, - ); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "?" threw an error, dropping event:', exception); }); - test('event processor sends an event and logs when it crashes asynchronously', async () => { + test('drops the event and records a client report when an event processor rejects', async () => { vi.useFakeTimers(); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); const captureExceptionSpy = vi.spyOn(client, 'captureException'); - const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); const scope = new Scope(); const exception = new Error('sorry 2'); - scope.addEventProcessor(() => { - return new Promise((_resolve, reject) => { - reject(exception); - }); - }); + scope.addEventProcessor(() => Promise.reject(exception)); client.captureEvent({ message: 'hello' }, {}, scope); await vi.runOnlyPendingTimersAsync(); - expect(TestClient.instance!.event!.exception!.values![0]).toStrictEqual({ - type: 'Error', - value: 'sorry 2', - mechanism: { type: 'internal', handled: false }, - }); - expect(captureExceptionSpy).toBeCalledWith(exception, { - data: { - __sentry__: true, - }, - originalException: exception, - mechanism: { type: 'internal', handled: false }, - }); - expect(loggerWarnSpy).toBeCalledWith( - `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${exception}`, - ); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "?" threw an error, dropping event:', exception); }); - test('event processor sends an event and logs when it crashes synchronously in processor chain', () => { + test('a synchronously throwing event processor stops the processor chain', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); const captureExceptionSpy = vi.spyOn(client, 'captureException'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); const scope = new Scope(); - const exception = new Error('sorry 3'); const processor1 = vi.fn(event => { return event; }); const processor2 = vi.fn(() => { - throw exception; + throw new Error('sorry 3'); }); const processor3 = vi.fn(event => { return event; @@ -2278,29 +2275,25 @@ describe('Client', () => { expect(processor2).toHaveBeenCalledTimes(1); expect(processor3).toHaveBeenCalledTimes(0); - expect(captureExceptionSpy).toBeCalledWith(exception, { - data: { - __sentry__: true, - }, - originalException: exception, - mechanism: { type: 'internal', handled: false }, - }); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); }); - test('event processor sends an event and logs when it crashes asynchronously in processor chain', async () => { + test('a rejecting event processor stops the processor chain', async () => { vi.useFakeTimers(); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); const captureExceptionSpy = vi.spyOn(client, 'captureException'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); const scope = new Scope(); - const exception = new Error('sorry 4'); const processor1 = vi.fn(async event => { return event; }); const processor2 = vi.fn(async () => { - throw exception; + throw new Error('sorry 4'); }); const processor3 = vi.fn(event => { return event; @@ -2317,38 +2310,143 @@ describe('Client', () => { expect(processor2).toHaveBeenCalledTimes(1); expect(processor3).toHaveBeenCalledTimes(0); - expect(captureExceptionSpy).toBeCalledWith(exception, { - data: { - __sentry__: true, - }, - originalException: exception, - mechanism: { type: 'internal', handled: false }, - }); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); }); - test('client-level event processor that throws on all events does not cause infinite recursion', () => { + test('client-level event processor that throws on all events does not capture a new event', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); + const captureExceptionSpy = vi.spyOn(client, 'captureException'); - let processorCallCount = 0; - // Add processor at client level - this runs on ALL events including internal exceptions - client.addEventProcessor(() => { - processorCallCount++; + const processor = vi.fn(() => { throw new Error('Processor always throws'); }); + client.addEventProcessor(processor); client.captureMessage('test message'); - // Should be called once for the original message - // internal exception events skips event processors entirely. - expect(processorCallCount).toBe(1); + expect(processor).toHaveBeenCalledTimes(1); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + }); + + test('drops the event and records a client report when `beforeSend` throws', () => { + const exception = new Error('beforeSend failed'); + const beforeSend = vi.fn(() => { + throw exception; + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); + const client = new TestClient(options); + const captureExceptionSpy = vi.spyOn(client, 'captureException'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + + client.captureEvent({ message: 'hello' }); + + expect(beforeSend).toHaveBeenCalledTimes(1); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledTimes(1); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeSend` callback threw an error, dropping the event:', + exception, + ); + }); + + test('drops the event and records a client report when `beforeSend` rejects', async () => { + vi.useFakeTimers(); + + const exception = new Error('beforeSend failed'); + const beforeSend = vi.fn(() => Promise.reject(exception)); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); + const client = new TestClient(options); + const captureExceptionSpy = vi.spyOn(client, 'captureException'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + + client.captureEvent({ message: 'hello' }); + await vi.runOnlyPendingTimersAsync(); + + expect(beforeSend).toHaveBeenCalledTimes(1); + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'error'); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeSend` callback threw an error, dropping the event:', + exception, + ); + }); + + test('drops the transaction and its spans when `beforeSendTransaction` throws', () => { + const exception = new Error('beforeSendTransaction failed'); + const beforeSendTransaction = vi.fn(() => { + throw exception; + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction }); + const client = new TestClient(options); + const captureExceptionSpy = vi.spyOn(client, 'captureException'); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + + client.captureEvent({ + transaction: '/dogs/are/great', + type: 'transaction', + spans: [ + { + description: 'first span', + span_id: '9e15bf99fbe4bc80', + start_timestamp: 1591603196.637835, + trace_id: '86f39e84263a4de99c326acab3bfe3bd', + data: {}, + status: 'ok', + }, + { + description: 'second span', + span_id: 'aa554c1f506b0783', + start_timestamp: 1591603196.637835, + trace_id: '86f39e84263a4de99c326acab3bfe3bd', + data: {}, + status: 'ok', + }, + ], + }); + + expect(TestClient.instance!.event).toBeUndefined(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'transaction'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span', 3); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeSendTransaction` callback threw an error, dropping the event:', + exception, + ); + }); + + test('captures an internal event when the event processing pipeline itself throws', async () => { + vi.useFakeTimers(); + + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + const captureExceptionSpy = vi.spyOn(client, 'captureException'); + const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn'); + const exception = new Error('sdk bug'); + vi.spyOn(client as any, '_prepareEvent').mockImplementation(() => Promise.reject(exception)); - // Verify the processor error was captured and sent - expect(TestClient.instance!.event!.exception!.values![0]).toStrictEqual({ - type: 'Error', - value: 'Processor always throws', + client.captureEvent({ message: 'hello' }); + await vi.runOnlyPendingTimersAsync(); + + expect(captureExceptionSpy).toBeCalledWith(exception, { + data: { + __sentry__: true, + }, + originalException: exception, mechanism: { type: 'internal', handled: false }, }); + expect(loggerWarnSpy).toBeCalledWith( + `Event processing pipeline threw an error, original event will not be sent. Details have been sent as a new event.\nReason: ${exception}`, + ); }); test('records events dropped due to `sampleRate` option', () => { diff --git a/packages/core/test/lib/eventProcessors.test.ts b/packages/core/test/lib/eventProcessors.test.ts new file mode 100644 index 000000000000..5570788cdcaf --- /dev/null +++ b/packages/core/test/lib/eventProcessors.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it, vi } from 'vitest'; +import { notifyEventProcessors } from '../../src/eventProcessors'; +import type { EventProcessor } from '../../src/types/eventprocessor'; +import * as debugLoggerModule from '../../src/utils/debug-logger'; + +describe('notifyEventProcessors', () => { + it('passes the event through all processors', async () => { + const processors: EventProcessor[] = [ + event => ({ ...event, tags: { first: 'yes' } }), + async event => ({ ...event, tags: { ...event.tags, second: 'yes' } }), + ]; + + const result = await notifyEventProcessors(processors, { message: 'hello' }, {}); + + expect(result).toEqual({ message: 'hello', tags: { first: 'yes', second: 'yes' } }); + }); + + it('stops when a processor returns null', async () => { + const later = vi.fn(event => event); + + const result = await notifyEventProcessors([() => null, later], { message: 'hello' }, {}); + + expect(result).toBeNull(); + expect(later).not.toHaveBeenCalled(); + }); + + it('drops the event when a processor throws synchronously', async () => { + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + const error = new Error('boom'); + const throwing: EventProcessor = () => { + throw error; + }; + throwing.id = 'Throwing'; + const later = vi.fn(event => event); + + const result = await notifyEventProcessors([throwing, later], { message: 'hello' }, {}); + + expect(result).toBeNull(); + expect(later).not.toHaveBeenCalled(); + expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "Throwing" threw an error, dropping event:', error); + }); + + it('drops the event when a processor rejects', async () => { + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + const error = new Error('boom'); + const later = vi.fn(event => event); + + const result = await notifyEventProcessors([() => Promise.reject(error), later], { message: 'hello' }, {}); + + expect(result).toBeNull(); + expect(later).not.toHaveBeenCalled(); + expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "?" threw an error, dropping event:', error); + }); +}); diff --git a/packages/core/test/lib/logs/internal.test.ts b/packages/core/test/lib/logs/internal.test.ts index 61de9ef2a7a8..d34df4ba16e6 100644 --- a/packages/core/test/lib/logs/internal.test.ts +++ b/packages/core/test/lib/logs/internal.test.ts @@ -370,6 +370,35 @@ describe('_INTERNAL_captureLog', () => { ); }); + it('drops logs when beforeSendLog throws', () => { + const exception = new Error('beforeSendLog failed'); + const beforeSendLog = vi.fn(() => { + throw exception; + }); + const recordDroppedEventSpy = vi.spyOn(TestClient.prototype, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(loggerModule.debug, 'error'); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + beforeSendLog, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setClient(client); + + expect(() => _INTERNAL_captureLog({ level: 'info', message: 'test message' }, scope)).not.toThrow(); + + expect(beforeSendLog).toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'log_item', 1); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeSendLog` callback threw an error, dropping the log:', + exception, + ); + expect(_INTERNAL_getLogBuffer(client)).toBeUndefined(); + + recordDroppedEventSpy.mockRestore(); + }); + it('drops logs when beforeSendLog returns null', () => { const beforeSendLog = vi.fn().mockReturnValue(null); const recordDroppedEventSpy = vi.spyOn(TestClient.prototype, 'recordDroppedEvent'); diff --git a/packages/core/test/lib/metrics/internal.test.ts b/packages/core/test/lib/metrics/internal.test.ts index 2f9b46606857..95e2a4ccea97 100644 --- a/packages/core/test/lib/metrics/internal.test.ts +++ b/packages/core/test/lib/metrics/internal.test.ts @@ -337,6 +337,7 @@ describe('_INTERNAL_captureMetric', () => { it('drops metrics when beforeSendMetric returns null', () => { const beforeSendMetric = vi.fn().mockReturnValue(null); + const recordDroppedEventSpy = vi.spyOn(TestClient.prototype, 'recordDroppedEvent'); const loggerWarnSpy = vi.spyOn(loggerModule.debug, 'log').mockImplementation(() => undefined); const options = getDefaultTestClientOptions({ @@ -357,12 +358,43 @@ describe('_INTERNAL_captureMetric', () => { ); expect(beforeSendMetric).toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'metric', 1); expect(loggerWarnSpy).toHaveBeenCalledWith('`beforeSendMetric` returned `null`, will not send metric.'); expect(_INTERNAL_getMetricBuffer(client)).toBeUndefined(); + recordDroppedEventSpy.mockRestore(); loggerWarnSpy.mockRestore(); }); + it('drops metrics when beforeSendMetric throws', () => { + const exception = new Error('beforeSendMetric failed'); + const beforeSendMetric = vi.fn(() => { + throw exception; + }); + const recordDroppedEventSpy = vi.spyOn(TestClient.prototype, 'recordDroppedEvent'); + const debugErrorSpy = vi.spyOn(loggerModule.debug, 'error'); + + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + beforeSendMetric, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setClient(client); + + expect(() => _INTERNAL_captureMetric({ type: 'counter', name: 'test.metric', value: 1 }, { scope })).not.toThrow(); + + expect(beforeSendMetric).toHaveBeenCalled(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'metric', 1); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeSendMetric` callback threw an error, dropping the metric:', + exception, + ); + expect(_INTERNAL_getMetricBuffer(client)).toBeUndefined(); + + recordDroppedEventSpy.mockRestore(); + }); + it('emits afterCaptureMetric event', () => { const afterCaptureMetricSpy = vi.spyOn(TestClient.prototype, 'emit'); const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); diff --git a/packages/core/test/lib/tracing/sampling.test.ts b/packages/core/test/lib/tracing/sampling.test.ts new file mode 100644 index 000000000000..5caa3ea35470 --- /dev/null +++ b/packages/core/test/lib/tracing/sampling.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it, vi } from 'vitest'; +import { sampleSpan } from '../../../src/tracing/sampling'; +import * as debugLoggerModule from '../../../src/utils/debug-logger'; + +describe('sampleSpan', () => { + describe('when `tracesSampler` throws', () => { + const exception = new Error('tracesSampler failed'); + const tracesSampler = vi.fn(() => { + throw exception; + }); + const expectedMessage = + 'The `tracesSampler` callback threw an error, falling back to the parent sampling decision or `tracesSampleRate`:'; + + it('inherits the parent sampling decision', () => { + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + + expect(sampleSpan({ tracesSampler }, { name: 'test', attributes: {}, parentSampled: true }, 0.5)).toEqual([ + true, + 1, + undefined, + ]); + expect(sampleSpan({ tracesSampler }, { name: 'test', attributes: {}, parentSampled: false }, 0.5)).toEqual([ + false, + 0, + undefined, + ]); + expect(debugErrorSpy).toHaveBeenCalledWith(expectedMessage, exception); + }); + + it('falls back to `tracesSampleRate` without a parent decision', () => { + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + + expect(sampleSpan({ tracesSampler, tracesSampleRate: 0.6 }, { name: 'test', attributes: {} }, 0.5)).toEqual([ + true, + 0.6, + true, + ]); + expect(debugErrorSpy).toHaveBeenCalledWith(expectedMessage, exception); + }); + + it('does not sample when there is nothing to fall back to', () => { + const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); + const debugWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn'); + + expect(sampleSpan({ tracesSampler }, { name: 'test', attributes: {} }, 0.5)).toEqual([false]); + expect(debugErrorSpy).toHaveBeenCalledWith(expectedMessage, exception); + expect(debugWarnSpy).not.toHaveBeenCalled(); + }); + }); +});