Skip to content

Commit 2af75e4

Browse files
Lms24claude
Lms24
and
claude
authored
fix(core): Record client outcomes when beforeSend callbacks return invalid values (#25049)
When `beforeSend` or `beforeSendTransaction` returns something other than an event or `null` (often `undefined` from a forgotten `return`), the event is dropped but no client outcome is recorded. It now records a `before_send` outcome for the event's category and, for transactions, the root `span` plus all child spans that weren't already recorded as `ignored`. --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 1f95be2 commit 2af75e4

2 files changed

Lines changed: 115 additions & 30 deletions

File tree

‎packages/core/src/client.ts‎

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import { consoleSandbox, debug } from './utils/debug-logger';
4343
import { dsnToString, makeDsn } from './utils/dsn';
4444
import { addItemToEnvelope, createAttachmentEnvelopeItem, getDataCategoryByType } from './utils/envelope';
4545
import { getPossibleEventMessages, isInternalException } from './utils/eventUtils';
46-
import { isObjectLike, isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils/is';
46+
import { isObjectLike, isParameterizedString, isPlainObject, isPrimitive } from './utils/is';
4747
import { merge } from './utils/merge';
4848
import { checkOrSetAlreadyCaught, uuid4 } from './utils/misc';
4949
import { parseSampleRate } from './utils/parseSampleRate';
@@ -1556,7 +1556,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
15561556
return prepared;
15571557
}
15581558

1559-
const result = processBeforeSend(
1559+
return processBeforeSend(
15601560
options,
15611561
prepared,
15621562
hint,
@@ -1567,19 +1567,23 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
15671567
ignoredSpanCount = count;
15681568
},
15691569
);
1570-
return _validateBeforeSendResult(result, beforeSendLabel);
15711570
})
15721571
.then(processedEvent => {
15731572
if (ignoredSpanCount) {
15741573
this.recordDroppedEvent('ignored', 'span', ignoredSpanCount);
15751574
}
15761575

1577-
if (processedEvent === null) {
1576+
// An annotated boolean keeps `isPlainObject` from narrowing `processedEvent` to a plain record
1577+
const isValidEvent: boolean = isPlainObject(processedEvent);
1578+
if (!isValidEvent || processedEvent === null) {
15781579
this.recordDroppedEvent(beforeSendDropReason, dataCategory);
15791580
if (isTransaction) {
15801581
// the transaction itself counts as one span, plus all the child spans that weren't ignored before
15811582
this.recordDroppedEvent(beforeSendDropReason, 'span', 1 + preparedSpanCount - ignoredSpanCount);
15821583
}
1584+
if (processedEvent !== null) {
1585+
throw _makeInternalError(`${beforeSendLabel} must return \`null\` or a valid event.`);
1586+
}
15831587
const dropMessage =
15841588
beforeSendDropReason === 'ignored'
15851589
? 'Transaction matched `ignoreSpans`'
@@ -1724,32 +1728,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
17241728
): PromiseLike<Event>;
17251729
}
17261730

1727-
/**
1728-
* Verifies that return value of configured `beforeSend` or `beforeSendTransaction` is of expected type, and returns the value if so.
1729-
*/
1730-
function _validateBeforeSendResult(
1731-
beforeSendResult: PromiseLike<Event | null> | Event | null,
1732-
beforeSendLabel: string,
1733-
): PromiseLike<Event | null> | Event | null {
1734-
const invalidValueError = `${beforeSendLabel} must return \`null\` or a valid event.`;
1735-
if (isThenable(beforeSendResult)) {
1736-
return beforeSendResult.then(
1737-
event => {
1738-
if (!isPlainObject(event) && event !== null) {
1739-
throw _makeInternalError(invalidValueError);
1740-
}
1741-
return event;
1742-
},
1743-
e => {
1744-
throw _makeInternalError(`${beforeSendLabel} rejected with ${e}`);
1745-
},
1746-
);
1747-
} else if (!isPlainObject(beforeSendResult) && beforeSendResult !== null) {
1748-
throw _makeInternalError(invalidValueError);
1749-
}
1750-
return beforeSendResult;
1751-
}
1752-
17531731
type BeforeSendDropReason = 'before_send' | 'callback_error' | 'ignored';
17541732

17551733
/**

‎packages/core/test/lib/client.test.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2759,6 +2759,113 @@ describe('Client', () => {
27592759
);
27602760
});
27612761

2762+
test('drops the transaction and its non-ignored spans when `beforeSendTransaction` rejects', async () => {
2763+
vi.useFakeTimers();
2764+
2765+
const exception = new Error('beforeSendTransaction failed');
2766+
const beforeSendTransaction = vi.fn(() => Promise.reject(exception));
2767+
const options = getDefaultTestClientOptions({
2768+
dsn: PUBLIC_DSN,
2769+
beforeSendTransaction,
2770+
ignoreSpans: ['ignored span'],
2771+
});
2772+
const client = new TestClient(options);
2773+
const captureExceptionSpy = vi.spyOn(client, 'captureException');
2774+
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
2775+
const debugErrorSpy = vi.spyOn(debugLoggerModule.debug, 'error');
2776+
2777+
client.captureEvent({
2778+
transaction: '/dogs/are/great',
2779+
type: 'transaction',
2780+
spans: ['first span', 'ignored span', 'second span'].map((description, i) => ({
2781+
description,
2782+
span_id: `${i}`.padStart(16, '0'),
2783+
start_timestamp: 1591603196.637835,
2784+
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
2785+
data: {},
2786+
status: 'ok',
2787+
})),
2788+
});
2789+
await vi.runOnlyPendingTimersAsync();
2790+
2791+
expect(TestClient.instance!.event).toBeUndefined();
2792+
expect(captureExceptionSpy).not.toHaveBeenCalled();
2793+
expect(recordDroppedEventSpy).toHaveBeenCalledTimes(3);
2794+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('ignored', 'span', 1);
2795+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'transaction');
2796+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'span', 3);
2797+
expect(debugErrorSpy).toHaveBeenCalledWith(
2798+
'The `beforeSendTransaction` callback threw an error, dropping the event:',
2799+
exception,
2800+
);
2801+
});
2802+
2803+
test.each([
2804+
['sync', () => undefined],
2805+
['async', () => Promise.resolve(undefined)],
2806+
])('records a `before_send` outcome when `beforeSend` returns an invalid value (%s)', async (_, beforeSend) => {
2807+
vi.useFakeTimers();
2808+
2809+
// @ts-expect-error we need to test regular-js behavior
2810+
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend });
2811+
const client = new TestClient(options);
2812+
const captureExceptionSpy = vi.spyOn(client, 'captureException');
2813+
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
2814+
const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn');
2815+
2816+
client.captureEvent({ message: 'hello' });
2817+
await vi.runOnlyPendingTimersAsync();
2818+
2819+
expect(TestClient.instance!.event).toBeUndefined();
2820+
expect(captureExceptionSpy).not.toHaveBeenCalled();
2821+
expect(recordDroppedEventSpy).toHaveBeenCalledTimes(1);
2822+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'error');
2823+
expect(loggerWarnSpy).toHaveBeenCalledWith('before send for type `error` must return `null` or a valid event.');
2824+
});
2825+
2826+
test.each([
2827+
['sync', () => undefined],
2828+
['async', () => Promise.resolve(undefined)],
2829+
])(
2830+
'records `before_send` outcomes for the transaction and its spans when `beforeSendTransaction` returns an invalid value (%s)',
2831+
async (_, beforeSendTransaction) => {
2832+
vi.useFakeTimers();
2833+
2834+
const options = getDefaultTestClientOptions({
2835+
dsn: PUBLIC_DSN,
2836+
// @ts-expect-error we need to test regular-js behavior
2837+
beforeSendTransaction,
2838+
ignoreSpans: ['ignored span'],
2839+
});
2840+
const client = new TestClient(options);
2841+
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
2842+
const loggerWarnSpy = vi.spyOn(debugLoggerModule.debug, 'warn');
2843+
2844+
client.captureEvent({
2845+
transaction: '/dogs/are/great',
2846+
type: 'transaction',
2847+
spans: ['first span', 'ignored span'].map((description, i) => ({
2848+
description,
2849+
span_id: `${i}`.padStart(16, '0'),
2850+
start_timestamp: 1591603196.637835,
2851+
trace_id: '86f39e84263a4de99c326acab3bfe3bd',
2852+
data: {},
2853+
status: 'ok',
2854+
})),
2855+
});
2856+
await vi.runOnlyPendingTimersAsync();
2857+
2858+
expect(TestClient.instance!.event).toBeUndefined();
2859+
expect(recordDroppedEventSpy).toHaveBeenCalledTimes(3);
2860+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('ignored', 'span', 1);
2861+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'transaction');
2862+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'span', 2);
2863+
expect(loggerWarnSpy).toHaveBeenCalledWith(
2864+
'before send for type `transaction` must return `null` or a valid event.',
2865+
);
2866+
},
2867+
);
2868+
27622869
test('captures an internal event when the event processing pipeline itself throws', async () => {
27632870
vi.useFakeTimers();
27642871

0 commit comments

Comments
 (0)