Skip to content

fix(core): Record client outcomes when beforeSend callbacks return invalid values #25049

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

Draft
Lms24 wants to merge 2 commits into from
Draft

fix(core): Record client outcomes when beforeSend callbacks return invalid values #25049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50c3264
fix(core): Record client outcomes when beforeSend callbacks return in…
Lms24 Oct 5, 2026
8064da8
fix(core): Remove unnecessary type assertion flagged by lint
Lms24 Oct 5, 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
38 changes: 8 additions & 30 deletions packages/core/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import { consoleSandbox, debug } from './utils/debug-logger';
import { dsnToString, makeDsn } from './utils/dsn';
import { addItemToEnvelope, createAttachmentEnvelopeItem, getDataCategoryByType } from './utils/envelope';
import { getPossibleEventMessages } from './utils/eventUtils';
import { isObjectLike, isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils/is';
import { isObjectLike, isParameterizedString, isPlainObject, isPrimitive } from './utils/is';
import { merge } from './utils/merge';
import { checkOrSetAlreadyCaught, uuid4 } from './utils/misc';
import { parseSampleRate } from './utils/parseSampleRate';
Expand Down Expand Up @@ -1540,7 +1540,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
return prepared;
}

const result = processBeforeSend(
return processBeforeSend(
options,
prepared,
hint,
Expand All @@ -1551,19 +1551,23 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
ignoredSpanCount = count;
},
);
return _validateBeforeSendResult(result, beforeSendLabel);
})
.then(processedEvent => {
if (ignoredSpanCount) {
this.recordDroppedEvent('ignored', 'span', ignoredSpanCount);
}

if (processedEvent === null) {
// An annotated boolean keeps `isPlainObject` from narrowing `processedEvent` to a plain record
const isValidEvent: boolean = isPlainObject(processedEvent);
if (!isValidEvent || processedEvent === null) {
this.recordDroppedEvent(beforeSendDropReason, dataCategory);
if (isTransaction) {
// the transaction itself counts as one span, plus all the child spans that weren't ignored before
this.recordDroppedEvent(beforeSendDropReason, 'span', 1 + preparedSpanCount - ignoredSpanCount);
}
if (processedEvent !== null) {
throw _makeInternalError(`${beforeSendLabel} must return \`null\` or a valid event.`);
}
const dropMessage =
beforeSendDropReason === 'ignored'
? 'Transaction matched `ignoreSpans`'
Expand Down Expand Up @@ -1708,32 +1712,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
): PromiseLike<Event>;
}

/**
* Verifies that return value of configured `beforeSend` or `beforeSendTransaction` is of expected type, and returns the value if so.
*/
function _validateBeforeSendResult(

@Lms24 Lms24 Oct 5, 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.

this function can conveniently be removed because the invalidValueError is now directly thrown in line 1569 and the ${beforeSendLabel} rejected with ${e} wasn't reached already because of safeCallback handling throws now. So this was mostly just dead code that could be simplified.

beforeSendResult: PromiseLike<Event | null> | Event | null,
beforeSendLabel: string,
): PromiseLike<Event | null> | Event | null {
const invalidValueError = `${beforeSendLabel} must return \`null\` or a valid event.`;
if (isThenable(beforeSendResult)) {
return beforeSendResult.then(
event => {
if (!isPlainObject(event) && event !== null) {
throw _makeInternalError(invalidValueError);
}
return event;
},
e => {
throw _makeInternalError(`${beforeSendLabel} rejected with ${e}`);
},
);
} else if (!isPlainObject(beforeSendResult) && beforeSendResult !== null) {
throw _makeInternalError(invalidValueError);
}
return beforeSendResult;
}

type BeforeSendDropReason = 'before_send' | 'callback_error' | 'ignored';

/**
Expand Down
107 changes: 107 additions & 0 deletions packages/core/test/lib/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2757,6 +2757,113 @@ describe('Client', () => {
);
});

test('drops the transaction and its non-ignored spans when `beforeSendTransaction` rejects', async () => {
vi.useFakeTimers();

const exception = new Error('beforeSendTransaction failed');
const beforeSendTransaction = vi.fn(() => Promise.reject(exception));
const options = getDefaultTestClientOptions({
dsn: PUBLIC_DSN,
beforeSendTransaction,
ignoreSpans: ['ignored span'],
});
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: ['first span', 'ignored span', 'second span'].map((description, i) => ({
description,
span_id: `${i}`.padStart(16, '0'),
start_timestamp: 1591603196.637835,
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
data: {},
status: 'ok',
})),
});
await vi.runOnlyPendingTimersAsync();

expect(TestClient.instance!.event).toBeUndefined();
expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(recordDroppedEventSpy).toHaveBeenCalledTimes(3);
expect(recordDroppedEventSpy).toHaveBeenCalledWith('ignored', 'span', 1);
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,
);
});

test.each([
['sync', () => undefined],
['async', () => Promise.resolve(undefined)],
])('records a `before_send` outcome when `beforeSend` returns an invalid value (%s)', async (_, beforeSend) => {
vi.useFakeTimers();

// @ts-expect-error we need to test regular-js behavior
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 loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn');

client.captureEvent({ message: 'hello' });
await vi.runOnlyPendingTimersAsync();

expect(TestClient.instance!.event).toBeUndefined();
expect(captureExceptionSpy).not.toHaveBeenCalled();
expect(recordDroppedEventSpy).toHaveBeenCalledTimes(1);
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'error');
expect(loggerWarnSpy).toHaveBeenCalledWith('before send for type `error` must return `null` or a valid event.');
});

test.each([
['sync', () => undefined],
['async', () => Promise.resolve(undefined)],
])(
'records `before_send` outcomes for the transaction and its spans when `beforeSendTransaction` returns an invalid value (%s)',
async (_, beforeSendTransaction) => {
vi.useFakeTimers();

const options = getDefaultTestClientOptions({
dsn: PUBLIC_DSN,
// @ts-expect-error we need to test regular-js behavior
beforeSendTransaction,
ignoreSpans: ['ignored span'],
});
const client = new TestClient(options);
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn');

client.captureEvent({
transaction: '/dogs/are/great',
type: 'transaction',
spans: ['first span', 'ignored span'].map((description, i) => ({
description,
span_id: `${i}`.padStart(16, '0'),
start_timestamp: 1591603196.637835,
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
data: {},
status: 'ok',
})),
});
await vi.runOnlyPendingTimersAsync();

expect(TestClient.instance!.event).toBeUndefined();
expect(recordDroppedEventSpy).toHaveBeenCalledTimes(3);
expect(recordDroppedEventSpy).toHaveBeenCalledWith('ignored', 'span', 1);
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'transaction');
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span', 2);
expect(loggerWarnSpy).toHaveBeenCalledWith(
'before send for type `transaction` must return `null` or a valid event.',
);
},
);

test('captures an internal event when the event processing pipeline itself throws', async () => {
vi.useFakeTimers();

Expand Down
Loading