Skip to content

feat(core): Record callback_error client reports for throwing user callbacks #23903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
msonnb wants to merge 4 commits into from
Open

feat(core): Record callback_error client reports for throwing user callbacks #23903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dcf7f40
feat(core): Record `callback_error` client reports for throwing user …
msonnb Sep 2, 2026
53d1f8d
revert to tuple instead of object
msonnb Sep 3, 2026
efbaddb
fix(core): Report callback errors without sentinel values
msonnb Sep 3, 2026
e8aff02
add comment
msonnb Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ages/node-integration-tests/suites/client-reports/drop-reasons/before-send-throws/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
},
Expand Down
4 changes: 2 additions & 2 deletions .../node-integration-tests/suites/client-reports/drop-reasons/event-processor-throws/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
},
Expand All @@ -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',
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion ...s/node-integration-tests/suites/client-reports/drop-reasons/traces-sampler-throws/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
],
},
Expand Down
33 changes: 19 additions & 14 deletions packages/core/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -1515,6 +1515,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
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
Expand All @@ -1525,7 +1526,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
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.');
}

Expand All @@ -1534,19 +1534,21 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
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();
Expand Down Expand Up @@ -1689,10 +1691,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
): PromiseLike<Event>;
}

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.
*/
Expand Down Expand Up @@ -1727,6 +1725,7 @@ function processBeforeSend(
options: ClientOptions,
event: Event,
hint: EventHint,
onCallbackError: () => void,
): PromiseLike<Event | null> | Event | null {
const {
beforeSend,
Expand All @@ -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;
},
);
}

Expand Down Expand Up @@ -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;
},
);
}
}
Expand Down
26 changes: 22 additions & 4 deletions packages/core/src/eventProcessors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*/
Expand All @@ -14,9 +16,10 @@ export function notifyEventProcessors(
event: Event | null,
hint: EventHint,
index: number = 0,
onDrop?: (reason: EventProcessorDropReason) => void,
): PromiseLike<Event | null> {
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);
Expand All @@ -28,6 +31,7 @@ function _notifyEventProcessors(
hint: EventHint,
processors: EventProcessor[],
index: number,
onDrop?: (reason: EventProcessorDropReason) => void,
): Event | null | PromiseLike<Event | null> {
const processor = processors[index];

Expand All @@ -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;
}
Comment thread
cursor[bot] marked this conversation as resolved.
return _notifyEventProcessors(result, hint, processors, index + 1, onDrop);
}
10 changes: 7 additions & 3 deletions packages/core/src/logs/internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -144,13 +144,17 @@ export function _INTERNAL_captureLog(
client.emit('beforeCaptureLog', processedLog);

const log = beforeSendLog
? safeCallback(
? safeCallback<Log | null | typeof CALLBACK_ERROR>(
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.');
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/metrics/internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -183,13 +183,18 @@ export function _INTERNAL_captureMetric(beforeMetric: Metric, options?: Internal
client.emit('processMetric', enrichedMetric);

const processedMetric = beforeSendMetric
? safeCallback(
? safeCallback<Metric | null | typeof CALLBACK_ERROR>(
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.');
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/tracing/sampling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,17 @@ export function sampleSpan(
options: Pick<CoreOptions, 'tracesSampleRate' | 'tracesSampler'>,
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];
}

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;

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tracing/trace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/types/clientreport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { DataCategory } from './datacategory';

export type EventDropReason =
| 'before_send'
| 'callback_error'
| 'event_processor'
| 'network_error'
| 'queue_overflow'
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/utils/envelope.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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';
}
31 changes: 22 additions & 9 deletions packages/core/src/utils/prepareEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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<Event | null> = 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);
Comment on lines +127 to +129

@sentry sentry Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: When a transaction is dropped by an event processor, its spans are not recorded as dropped because the code only checks for the 'callback_error' reason, not 'event_processor'.
Severity: MEDIUM

Suggested Fix

In prepareEvent.ts, update the conditional check to also record dropped spans for transactions when the drop reason is 'event_processor'. This will align the behavior with how beforeSend drops are handled and ensure consistent accounting of dropped spans.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/core/src/utils/prepareEvent.ts#L127-L129

Potential issue: When a transaction event is dropped by an event processor returning
`null`, the associated spans are not recorded as dropped. The new logic in
`prepareEvent.ts` only records dropped spans for transactions if the drop reason is
`'callback_error'`. This creates an inconsistency with how `beforeSend` drops are
handled, which do record spans for dropped transactions. This will lead to undercounting
of dropped spans in Sentry's reporting for users who have event processors that drop
transactions.

Did we get this right? 👍 / 👎 to inform future reviews.

@msonnb msonnb Sep 4, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lms24 this is true, but preexisting on develop. Just to be sure, I assume we do want to count dropped spans in all cases, right?

@Lms24 Lms24 Sep 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we want to also count spans (which IIUC we already do in most cases). Agree that this is preexisting, so feel free to follow up on separately or do it in this PR. whatever works best


if (typeof normalizeDepth === 'number' && normalizeDepth > 0) {
return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth);
}
Expand Down
Loading
Loading