Skip to content

Commit 2b3d3e2

Browse files
Lms24claude
andcommitted
fix(core): Don't let a throwing beforeSendSpan callback escape into user code
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 962dbc1 commit 2b3d3e2

3 files changed

Lines changed: 79 additions & 2 deletions

File tree

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ module.exports = [
286286
path: createCDNPath('bundle.logs.metrics.min.js'),
287287
gzip: false,
288288
brotli: false,
289-
limit: '100 KB',
289+
limit: '101 KB',
290290
disablePlugins: ['@size-limit/esbuild'],
291291
},
292292
{

packages/core/src/tracing/spans/captureSpan.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { RawAttributes } from '../../attributes';
22
import type { Client } from '../../client';
3+
import { DEBUG_BUILD } from '../../debug-build';
34
import type { ScopeData } from '../../scope';
45
import {
56
SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT,
@@ -12,6 +13,7 @@ import {
1213
SEMANTIC_ATTRIBUTE_USER_USERNAME,
1314
} from '../../semanticAttributes';
1415
import type { SerializedStreamedSpan, Span, SpanJSON, StreamedSpanJSON } from '../../types/span';
16+
import { debug } from '../../utils/debug-logger';
1517
import { getCombinedScopeData } from '../../utils/scopeData';
1618
import {
1719
INTERNAL_getSegmentSpan,
@@ -183,7 +185,16 @@ export function applyBeforeSendSpanCallback<T extends StreamedSpanJSON | SpanJSO
183185
span: T,
184186
beforeSendSpan: (span: T) => T,
185187
): T {
186-
const modifedSpan = beforeSendSpan(span);
188+
let modifedSpan: T;
189+
try {
190+
modifedSpan = beforeSendSpan(span);
191+
} catch (error) {
192+
// Spans are captured synchronously when they end, so a throwing callback would otherwise
193+
// propagate into whatever user code ended the span.
194+
DEBUG_BUILD && debug.error('The `beforeSendSpan` callback threw an error, sending the span unmodified:', error);
195+
return span;
196+
}
197+
187198
if (!modifedSpan) {
188199
showSpanDropWarning();
189200
return span;

packages/core/test/lib/tracing/spans/captureSpan.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest';
22
import type { Contexts, SerializedStreamedSpan, SpanJSON, StreamedSpanJSON } from '../../../../src';
33
import {
44
captureSpan,
5+
debug,
56
SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT,
67
SEMANTIC_ATTRIBUTE_SENTRY_OP,
78
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -13,6 +14,7 @@ import {
1314
SEMANTIC_ATTRIBUTE_USER_ID,
1415
SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS,
1516
SEMANTIC_ATTRIBUTE_USER_USERNAME,
17+
spanStreamingIntegration,
1618
startInactiveSpan,
1719
startSpan,
1820
withStaticSpan,
@@ -547,6 +549,70 @@ describe('captureSpan', () => {
547549
consoleWarnSpy.mockRestore();
548550
});
549551

552+
it('keeps the span and logs an error if the beforeSendSpan callback throws', () => {
553+
const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined);
554+
const error = new Error('beforeSendSpan is broken');
555+
// A v10 callback that was not migrated to the streamed format throws like this, because
556+
// `data` doesn't exist on a `StreamedSpanJSON`.
557+
const beforeSendSpan = vi.fn(() => {
558+
throw error;
559+
});
560+
561+
const client = new TestClient(
562+
getDefaultTestClientOptions({
563+
dsn: 'https://dsn@ingest.f00.f00/1',
564+
tracesSampleRate: 1,
565+
traceLifecycle: 'stream',
566+
beforeSendSpan: beforeSendSpan as unknown as TestClientOptions['beforeSendSpan'],
567+
}),
568+
);
569+
570+
const span = withScope(scope => {
571+
scope.setClient(client);
572+
const span = startInactiveSpan({ name: 'my-span', attributes: { 'sentry.op': 'http.client' } });
573+
span.end();
574+
return span;
575+
});
576+
577+
const serialized = captureSpan(span, client);
578+
579+
expect(serialized.name).toBe('my-span');
580+
expect(serialized.attributes['sentry.op']).toEqual({ type: 'string', value: 'http.client' });
581+
expect(debugErrorSpy).toHaveBeenCalledWith(
582+
'The `beforeSendSpan` callback threw an error, sending the span unmodified:',
583+
error,
584+
);
585+
586+
debugErrorSpy.mockRestore();
587+
});
588+
589+
it("doesn't let a throwing beforeSendSpan callback propagate out of span.end()", () => {
590+
const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined);
591+
const client = new TestClient(
592+
getDefaultTestClientOptions({
593+
dsn: 'https://dsn@ingest.f00.f00/1',
594+
tracesSampleRate: 1,
595+
traceLifecycle: 'stream',
596+
integrations: [spanStreamingIntegration()],
597+
beforeSendSpan: (() => {
598+
throw new Error('beforeSendSpan is broken');
599+
}) as unknown as TestClientOptions['beforeSendSpan'],
600+
}),
601+
);
602+
603+
// Spans are captured synchronously from the `afterSpanEnd` hook, so a throwing callback
604+
// would otherwise surface in user code that ended the span.
605+
expect(() =>
606+
withScope(scope => {
607+
scope.setClient(client);
608+
client.init();
609+
startSpan({ name: 'my-span' }, () => undefined);
610+
}),
611+
).not.toThrow();
612+
613+
debugErrorSpy.mockRestore();
614+
});
615+
550616
// Standalone spans (INP web vital spans) are streamed even with the static trace lifecycle,
551617
// so a `withStaticSpan` callback has to see them in the v1 format it expects.
552618
describe('with the static trace lifecycle', () => {

0 commit comments

Comments
 (0)