diff --git a/packages/core/src/tracing/spans/captureSpan.ts b/packages/core/src/tracing/spans/captureSpan.ts index 47c6ee6ac021..c0fa74a55318 100644 --- a/packages/core/src/tracing/spans/captureSpan.ts +++ b/packages/core/src/tracing/spans/captureSpan.ts @@ -1,5 +1,6 @@ import type { RawAttributes } from '../../attributes'; import type { Client } from '../../client'; +import { DEBUG_BUILD } from '../../debug-build'; import type { ScopeData } from '../../scope'; import { SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT, @@ -12,6 +13,7 @@ import { SEMANTIC_ATTRIBUTE_USER_USERNAME, } from '../../semanticAttributes'; import type { SerializedStreamedSpan, Span, SpanJSON, StreamedSpanJSON } from '../../types/span'; +import { debug } from '../../utils/debug-logger'; import { getCombinedScopeData } from '../../utils/scopeData'; import { INTERNAL_getSegmentSpan, @@ -183,7 +185,16 @@ export function applyBeforeSendSpanCallback T, ): T { - const modifedSpan = beforeSendSpan(span); + let modifedSpan: T; + try { + modifedSpan = beforeSendSpan(span); + } catch (error) { + // Spans are captured synchronously when they end, so a throwing callback would otherwise + // propagate into whatever user code ended the span. + DEBUG_BUILD && debug.error('The `beforeSendSpan` callback threw an error, sending the span unmodified:', error); + return span; + } + if (!modifedSpan) { showSpanDropWarning(); return span; diff --git a/packages/core/test/lib/tracing/spans/captureSpan.test.ts b/packages/core/test/lib/tracing/spans/captureSpan.test.ts index fc2aab1a3e2c..9663edb70dcb 100644 --- a/packages/core/test/lib/tracing/spans/captureSpan.test.ts +++ b/packages/core/test/lib/tracing/spans/captureSpan.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import type { Contexts, SerializedStreamedSpan, SpanJSON, StreamedSpanJSON } from '../../../../src'; import { captureSpan, + debug, SEMANTIC_ATTRIBUTE_SENTRY_ENVIRONMENT, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -13,6 +14,7 @@ import { SEMANTIC_ATTRIBUTE_USER_ID, SEMANTIC_ATTRIBUTE_USER_IP_ADDRESS, SEMANTIC_ATTRIBUTE_USER_USERNAME, + spanStreamingIntegration, startInactiveSpan, startSpan, withStaticSpan, @@ -547,6 +549,70 @@ describe('captureSpan', () => { consoleWarnSpy.mockRestore(); }); + it('keeps the span and logs an error if the beforeSendSpan callback throws', () => { + const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined); + const error = new Error('beforeSendSpan is broken'); + // A v10 callback that was not migrated to the streamed format throws like this, because + // `data` doesn't exist on a `StreamedSpanJSON`. + const beforeSendSpan = vi.fn(() => { + throw error; + }); + + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + tracesSampleRate: 1, + traceLifecycle: 'stream', + beforeSendSpan: beforeSendSpan as unknown as TestClientOptions['beforeSendSpan'], + }), + ); + + const span = withScope(scope => { + scope.setClient(client); + const span = startInactiveSpan({ name: 'my-span', attributes: { 'sentry.op': 'http.client' } }); + span.end(); + return span; + }); + + const serialized = captureSpan(span, client); + + expect(serialized.name).toBe('my-span'); + expect(serialized.attributes['sentry.op']).toEqual({ type: 'string', value: 'http.client' }); + expect(debugErrorSpy).toHaveBeenCalledWith( + 'The `beforeSendSpan` callback threw an error, sending the span unmodified:', + error, + ); + + debugErrorSpy.mockRestore(); + }); + + it("doesn't let a throwing beforeSendSpan callback propagate out of span.end()", () => { + const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined); + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + tracesSampleRate: 1, + traceLifecycle: 'stream', + integrations: [spanStreamingIntegration()], + beforeSendSpan: (() => { + throw new Error('beforeSendSpan is broken'); + }) as unknown as TestClientOptions['beforeSendSpan'], + }), + ); + + // Spans are captured synchronously from the `afterSpanEnd` hook, so a throwing callback + // would otherwise surface in user code that ended the span. + expect(() => + withScope(scope => { + scope.setClient(client); + client.init(); + startSpan({ name: 'my-span' }, () => undefined); + }), + ).not.toThrow(); + + debugErrorSpy.mockRestore(); + }); + // Standalone spans (INP web vital spans) are streamed even with the static trace lifecycle, // so a `withStaticSpan` callback has to see them in the v1 format it expects. describe('with the static trace lifecycle', () => {