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 index e30038efc57b..80d5813b5f88 100644 --- 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 @@ -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', }, ], }, 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 index 370cccc35410..0996c0841cf2 100644 --- 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 @@ -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', }, ], }, 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 index df6cbb3195b2..e511fc97e8a9 100644 --- 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 @@ -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', }, ], }, diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 2de21ba37b41..12d02b348bb2 100644 --- a/packages/core/src/client.ts +++ b/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 { 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 { 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 { 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 { ): PromiseLike; } -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 { 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; + }, ); } } diff --git a/packages/core/src/eventProcessors.ts b/packages/core/src/eventProcessors.ts index ef25375d7716..26a39c839538 100644 --- a/packages/core/src/eventProcessors.ts +++ b/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 { 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 { 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); } diff --git a/packages/core/src/logs/internal.ts b/packages/core/src/logs/internal.ts index 3ad106dfbb32..64bbabad80e6 100644 --- a/packages/core/src/logs/internal.ts +++ b/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( 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.'); diff --git a/packages/core/src/metrics/internal.ts b/packages/core/src/metrics/internal.ts index 621992b2ed70..443a5c508e80 100644 --- a/packages/core/src/metrics/internal.ts +++ b/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( 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.'); diff --git a/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index 2b9e696fc3fe..e036629db9b0 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -16,7 +16,7 @@ export function sampleSpan( options: Pick, 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]; @@ -24,7 +24,9 @@ export function sampleSpan( 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; diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index b7ac40595830..fdfaf51d4057 100644 --- a/packages/core/src/tracing/trace.ts +++ b/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); diff --git a/packages/core/src/types/clientreport.ts b/packages/core/src/types/clientreport.ts index 154b58c5705e..d9966813ba8e 100644 --- a/packages/core/src/types/clientreport.ts +++ b/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' diff --git a/packages/core/src/utils/envelope.ts b/packages/core/src/utils/envelope.ts index 58b3fe353bd6..4eacc1919410 100644 --- a/packages/core/src/utils/envelope.ts +++ b/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'; +} diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 2af0eed8233f..f8951b3b0c75 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/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 = 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); } diff --git a/packages/core/src/utils/safeCallback.ts b/packages/core/src/utils/safeCallback.ts index 5b9079ca7c5d..15a5d7bd3f79 100644 --- a/packages/core/src/utils/safeCallback.ts +++ b/packages/core/src/utils/safeCallback.ts @@ -2,6 +2,13 @@ import { DEBUG_BUILD } from '../debug-build'; import { debug } from './debug-logger'; import { isThenable } from './is'; +/** + * Lets a `safeCallback` fallback signal "the callback failed" as opposed to "the callback returned `null`", + * so the call site can report the drop as `callback_error`. Always return it, never throw it: a thrown + * sentinel would have to be caught at every boundary, and some of those (e.g. `prepareEvent`) are public. + */ +export const CALLBACK_ERROR = Symbol.for('SentryCallbackError'); + /** * Invokes a user-provided callback (e.g. `beforeSend`, `tracesSampler`, an integration hook) so that * neither a synchronous throw nor a rejected promise escapes into the caller. On failure the error is diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index e43c35d2fe63..3248dec5ccdb 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -2222,7 +2222,7 @@ describe('Client', () => { expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'error'); expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "?" threw an error, dropping event:', exception); }); @@ -2244,7 +2244,7 @@ describe('Client', () => { expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'error'); expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "?" threw an error, dropping event:', exception); }); @@ -2277,7 +2277,7 @@ describe('Client', () => { expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'error'); }); test('a rejecting event processor stops the processor chain', async () => { @@ -2312,7 +2312,7 @@ describe('Client', () => { expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'error'); }); test('client-level event processor that throws on all events does not capture a new event', () => { @@ -2348,7 +2348,7 @@ describe('Client', () => { expect(beforeSend).toHaveBeenCalledTimes(1); expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'error'); expect(recordDroppedEventSpy).toHaveBeenCalledTimes(1); expect(debugErrorSpy).toHaveBeenCalledWith( 'The `beforeSend` callback threw an error, dropping the event:', @@ -2373,7 +2373,8 @@ describe('Client', () => { expect(beforeSend).toHaveBeenCalledTimes(1); expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'error'); + expect(recordDroppedEventSpy).toHaveBeenCalledTimes(1); expect(debugErrorSpy).toHaveBeenCalledWith( 'The `beforeSend` callback threw an error, dropping the event:', exception, @@ -2416,8 +2417,9 @@ describe('Client', () => { expect(TestClient.instance!.event).toBeUndefined(); expect(captureExceptionSpy).not.toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'transaction'); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span', 3); + expect(recordDroppedEventSpy).toHaveBeenCalledTimes(2); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'transaction'); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'span', 3); expect(debugErrorSpy).toHaveBeenCalledWith( 'The `beforeSendTransaction` callback threw an error, dropping the event:', exception, diff --git a/packages/core/test/lib/eventProcessors.test.ts b/packages/core/test/lib/eventProcessors.test.ts index 5570788cdcaf..3c6cd578c029 100644 --- a/packages/core/test/lib/eventProcessors.test.ts +++ b/packages/core/test/lib/eventProcessors.test.ts @@ -17,14 +17,43 @@ describe('notifyEventProcessors', () => { it('stops when a processor returns null', async () => { const later = vi.fn(event => event); + const onDrop = vi.fn(); - const result = await notifyEventProcessors([() => null, later], { message: 'hello' }, {}); + const result = await notifyEventProcessors([() => null, later], { message: 'hello' }, {}, 0, onDrop); expect(result).toBeNull(); expect(later).not.toHaveBeenCalled(); + expect(onDrop).toHaveBeenCalledOnce(); + expect(onDrop).toHaveBeenCalledWith('event_processor'); }); - it('drops the event when a processor throws synchronously', async () => { + it('stops and reports the drop when a processor returns undefined', async () => { + const processor = (() => undefined) as unknown as EventProcessor; + const later = vi.fn(event => event); + const onDrop = vi.fn(); + + const result = await notifyEventProcessors([processor, later], { message: 'hello' }, {}, 0, onDrop); + + expect(result).toBeNull(); + expect(later).not.toHaveBeenCalled(); + expect(onDrop).toHaveBeenCalledOnce(); + expect(onDrop).toHaveBeenCalledWith('event_processor'); + }); + + it('stops and reports the drop when a processor resolves undefined', async () => { + const processor = (() => Promise.resolve(undefined)) as unknown as EventProcessor; + const later = vi.fn(event => event); + const onDrop = vi.fn(); + + const result = await notifyEventProcessors([processor, later], { message: 'hello' }, {}, 0, onDrop); + + expect(result).toBeNull(); + expect(later).not.toHaveBeenCalled(); + expect(onDrop).toHaveBeenCalledOnce(); + expect(onDrop).toHaveBeenCalledWith('event_processor'); + }); + + it('resolves with `null` and reports a callback error when a processor throws synchronously', async () => { const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); const error = new Error('boom'); const throwing: EventProcessor = () => { @@ -32,23 +61,29 @@ describe('notifyEventProcessors', () => { }; throwing.id = 'Throwing'; const later = vi.fn(event => event); + const onDrop = vi.fn(); - const result = await notifyEventProcessors([throwing, later], { message: 'hello' }, {}); + await expect(notifyEventProcessors([throwing, later], { message: 'hello' }, {}, 0, onDrop)).resolves.toBeNull(); - expect(result).toBeNull(); expect(later).not.toHaveBeenCalled(); + expect(onDrop).toHaveBeenCalledOnce(); + expect(onDrop).toHaveBeenCalledWith('callback_error'); expect(debugErrorSpy).toHaveBeenCalledWith('Event processor "Throwing" threw an error, dropping event:', error); }); - it('drops the event when a processor rejects', async () => { + it('resolves with `null` and reports a callback error when a processor rejects', async () => { const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error'); const error = new Error('boom'); const later = vi.fn(event => event); + const onDrop = vi.fn(); - const result = await notifyEventProcessors([() => Promise.reject(error), later], { message: 'hello' }, {}); + await expect( + notifyEventProcessors([() => Promise.reject(error), later], { message: 'hello' }, {}, 0, onDrop), + ).resolves.toBeNull(); - expect(result).toBeNull(); expect(later).not.toHaveBeenCalled(); + expect(onDrop).toHaveBeenCalledOnce(); + expect(onDrop).toHaveBeenCalledWith('callback_error'); 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 d34df4ba16e6..fa3797d7dfac 100644 --- a/packages/core/test/lib/logs/internal.test.ts +++ b/packages/core/test/lib/logs/internal.test.ts @@ -389,7 +389,7 @@ describe('_INTERNAL_captureLog', () => { expect(() => _INTERNAL_captureLog({ level: 'info', message: 'test message' }, scope)).not.toThrow(); expect(beforeSendLog).toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'log_item', 1); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'log_item', 1); expect(debugErrorSpy).toHaveBeenCalledWith( 'The `beforeSendLog` callback threw an error, dropping the log:', exception, diff --git a/packages/core/test/lib/metrics/internal.test.ts b/packages/core/test/lib/metrics/internal.test.ts index 95e2a4ccea97..3efda3259dfb 100644 --- a/packages/core/test/lib/metrics/internal.test.ts +++ b/packages/core/test/lib/metrics/internal.test.ts @@ -385,7 +385,7 @@ describe('_INTERNAL_captureMetric', () => { expect(() => _INTERNAL_captureMetric({ type: 'counter', name: 'test.metric', value: 1 }, { scope })).not.toThrow(); expect(beforeSendMetric).toHaveBeenCalled(); - expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'metric', 1); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'metric', 1); expect(debugErrorSpy).toHaveBeenCalledWith( 'The `beforeSendMetric` callback threw an error, dropping the metric:', exception, diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index bc2f64694e68..62ba08ee9d04 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -447,6 +447,88 @@ describe('prepareEvent', () => { }); }); + describe('dropped events', () => { + function createClient(eventProcessor: EventProcessor): Client { + return { + emit() { + // noop + }, + getEventProcessors() { + return [eventProcessor]; + }, + recordDroppedEvent: vi.fn(), + } as unknown as Client; + } + + it('records an `event_processor` drop and resolves `null` when a processor returns `null`', async () => { + const client = createClient(() => null); + + await expect( + prepareEvent({} as ClientOptions, { message: 'foo' }, { integrations: [] }, new Scope(), client), + ).resolves.toBeNull(); + + expect(client.recordDroppedEvent).toHaveBeenCalledWith('event_processor', 'error'); + }); + + it('records `callback_error` transaction and span drops when a processor throws', async () => { + const client = createClient(() => { + throw new Error('sorry'); + }); + + await expect( + prepareEvent( + {} as ClientOptions, + { + type: 'transaction', + transaction: '/checkout', + spans: [ + { + description: 'load cart', + span_id: '9e15bf99fbe4bc80', + start_timestamp: 1591603196.637835, + trace_id: '86f39e84263a4de99c326acab3bfe3bd', + }, + { + description: 'reserve inventory', + span_id: 'aa554c1f506b0783', + start_timestamp: 1591603196.637835, + trace_id: '86f39e84263a4de99c326acab3bfe3bd', + }, + ], + }, + { integrations: [] }, + new Scope(), + client, + ), + ).resolves.toBeNull(); + + expect(client.recordDroppedEvent).toHaveBeenCalledTimes(2); + expect(client.recordDroppedEvent).toHaveBeenCalledWith('callback_error', 'transaction'); + expect(client.recordDroppedEvent).toHaveBeenCalledWith('callback_error', 'span', 3); + }); + + it('records a `callback_error` drop and resolves `null` when a processor rejects', async () => { + const client = createClient(() => Promise.reject(new Error('sorry'))); + + await expect( + prepareEvent({} as ClientOptions, { type: 'replay_event' }, { integrations: [] }, new Scope(), client), + ).resolves.toBeNull(); + + expect(client.recordDroppedEvent).toHaveBeenCalledWith('callback_error', 'replay'); + }); + + it('resolves `null` without a client when a processor throws', async () => { + const scope = new Scope(); + scope.addEventProcessor(() => { + throw new Error('sorry'); + }); + + await expect( + prepareEvent({} as ClientOptions, { message: 'foo' }, { integrations: [] }, scope), + ).resolves.toBeNull(); + }); + }); + it('merges scope data', async () => { const breadcrumb1 = { message: '1', timestamp: 111 } as Breadcrumb; const breadcrumb2 = { message: '2', timestamp: 222 } as Breadcrumb; diff --git a/packages/core/test/lib/tracing/sampling.test.ts b/packages/core/test/lib/tracing/sampling.test.ts index 5caa3ea35470..2f78a782a1e5 100644 --- a/packages/core/test/lib/tracing/sampling.test.ts +++ b/packages/core/test/lib/tracing/sampling.test.ts @@ -38,11 +38,16 @@ describe('sampleSpan', () => { expect(debugErrorSpy).toHaveBeenCalledWith(expectedMessage, exception); }); - it('does not sample when there is nothing to fall back to', () => { + it('does not sample and reports a `callback_error` drop 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(sampleSpan({ tracesSampler }, { name: 'test', attributes: {} }, 0.5)).toEqual([ + false, + undefined, + undefined, + 'callback_error', + ]); expect(debugErrorSpy).toHaveBeenCalledWith(expectedMessage, exception); expect(debugWarnSpy).not.toHaveBeenCalled(); }); diff --git a/packages/replay-internal/src/util/sendReplayRequest.ts b/packages/replay-internal/src/util/sendReplayRequest.ts index 6dea2a9821eb..c06e688eb377 100644 --- a/packages/replay-internal/src/util/sendReplayRequest.ts +++ b/packages/replay-internal/src/util/sendReplayRequest.ts @@ -53,8 +53,6 @@ export async function sendReplayRequest({ const replayEvent = await prepareReplayEvent({ scope, client, replayId, event: baseEvent }); if (!replayEvent) { - // Taken from baseclient's `_processEvent` method, where this is handled for errors/transactions - client.recordDroppedEvent('event_processor', 'replay'); DEBUG_BUILD && debug.log('An event processor returned `null`, will not send event.'); return Promise.resolve({}); } diff --git a/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts b/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts index 576c1acbba2e..c57c3c405e9a 100644 --- a/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts +++ b/packages/replay-internal/test/unit/util/prepareReplayEvent.test.ts @@ -1,5 +1,5 @@ import type { ReplayEvent } from '@sentry/core'; -import { getClient, getCurrentScope, setCurrentClient } from '@sentry/core'; +import { getClient, getCurrentScope, Scope, setCurrentClient } from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { REPLAY_EVENT_NAME } from '../../../src/constants'; import { prepareReplayEvent } from '../../../src/util/prepareReplayEvent'; @@ -25,6 +25,29 @@ describe('Unit | util | prepareReplayEvent', () => { vi.clearAllMocks(); }); + it('resolves `null` and records a client report when an event processor throws', async () => { + const client = getClient()!; + const scope = new Scope(); + const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent'); + scope.addEventProcessor(() => { + throw new Error('sorry'); + }); + + const event: ReplayEvent = { + type: REPLAY_EVENT_NAME, + timestamp: 1670837008.634, + error_ids: [], + trace_ids: [], + urls: [], + replay_id: 'replay-ID', + replay_type: 'session', + segment_id: 0, + }; + + await expect(prepareReplayEvent({ scope, client, replayId: 'replay-ID', event })).resolves.toBeNull(); + expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'replay'); + }); + it('works', async () => { const client = getClient()!; const scope = getCurrentScope();