diff --git a/packages/core/src/shared-exports.ts b/packages/core/src/shared-exports.ts index 31e4995db775..7cf1fcd69929 100644 --- a/packages/core/src/shared-exports.ts +++ b/packages/core/src/shared-exports.ts @@ -94,6 +94,7 @@ export { export { safeSetSpanJSONAttributes } from './tracing/spans/captureSpan'; export { isSentryRequestUrl } from './utils/isSentryRequestUrl'; export { handleCallbackErrors } from './utils/handleCallbackErrors'; +export { safeCallback } from './utils/safeCallback'; export { parameterize, fmt } from './utils/parameterize'; export type { HandleTunnelRequestOptions } from './utils/tunnel'; export { handleTunnelRequest } from './utils/tunnel'; diff --git a/packages/core/src/tracing/spans/beforeSendSpan.ts b/packages/core/src/tracing/spans/beforeSendSpan.ts index 1d1126d7f883..f5bcb7df854d 100644 --- a/packages/core/src/tracing/spans/beforeSendSpan.ts +++ b/packages/core/src/tracing/spans/beforeSendSpan.ts @@ -1,8 +1,8 @@ -import { DEBUG_BUILD } from '../../debug-build'; import type { BeforeSendStaticSpanCallback, BeforeSendStreamedSpanCallback } from '../../types/options'; import type { SpanJSON, StreamedSpanJSON } from '../../types/span'; import { addNonEnumerableProperty } from '../../utils/object'; -import { consoleSandbox, debug } from '../../utils/debug-logger'; +import { consoleSandbox } from '../../utils/debug-logger'; +import { safeCallback } from '../../utils/safeCallback'; /** * A wrapper to use the static, transaction-based span format in your `beforeSendSpan` callback. @@ -64,25 +64,25 @@ export function applyBeforeSendSpanCallback T, ): T { - try { - const modifedSpan = beforeSendSpan(span); - if (!modifedSpan) { - if (!hasShownSpanDropWarning) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - '[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.', - ); - }); - hasShownSpanDropWarning = true; - } - return span; - } - return modifedSpan; - } 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; + // Spans are captured synchronously when they end, so a throwing callback would otherwise + // propagate into whatever user code ended the span. + const modifiedSpan = safeCallback( + 'The `beforeSendSpan` callback threw an error, sending the span unmodified:', + () => beforeSendSpan(span), + () => span, + ); + if (modifiedSpan) { + return modifiedSpan; } + + if (!hasShownSpanDropWarning) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + '[Sentry] Returning null from `beforeSendSpan` is disallowed. To drop certain spans, configure the respective integrations directly or use `ignoreSpans`.', + ); + }); + hasShownSpanDropWarning = true; + } + return span; } diff --git a/packages/core/src/utils/safeCallback.ts b/packages/core/src/utils/safeCallback.ts new file mode 100644 index 000000000000..f9da628041ab --- /dev/null +++ b/packages/core/src/utils/safeCallback.ts @@ -0,0 +1,34 @@ +import { DEBUG_BUILD } from '../debug-build'; +import { debug } from './debug-logger'; +import { isThenable } from './is'; + +/** + * Invokes a user-provided callback (e.g. `beforeSend`, `tracesSampler`, an integration hook) so that + * neither a synchronous throw nor a rejected promise escapes into the caller. On failure the error is + * logged and `fallback(error)` supplies the result instead. + * + * Not for `startSpan` bodies: those must re-throw and are handled by `handleCallbackErrors`. + * + * @param message - Logged via `debug.error` together with the error, e.g. "The `beforeSend` callback threw an error, dropping the event:". + * @param fn - Invokes the callback. + * @param fallback - Produces the result to use when the callback throws or rejects. + */ +export function safeCallback(message: string, fn: () => T, fallback: (error: unknown) => T): T { + let result: T; + try { + result = fn(); + } catch (error) { + return recover(message, error, fallback); + } + + if (isThenable(result)) { + return result.then(undefined, (error: unknown) => recover(message, error, fallback)) as T; + } + + return result; +} + +function recover(message: string, error: unknown, fallback: (error: unknown) => T): T { + DEBUG_BUILD && debug.error(message, error); + return fallback(error); +} diff --git a/packages/core/test/lib/utils/safeCallback.test.ts b/packages/core/test/lib/utils/safeCallback.test.ts new file mode 100644 index 000000000000..dc5c4552d6ae --- /dev/null +++ b/packages/core/test/lib/utils/safeCallback.test.ts @@ -0,0 +1,70 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { debug } from '../../../src/utils/debug-logger'; +import { safeCallback } from '../../../src/utils/safeCallback'; + +describe('safeCallback', () => { + const debugErrorSpy = vi.spyOn(debug, 'error').mockImplementation(() => undefined); + + afterEach(() => { + debugErrorSpy.mockClear(); + }); + + it('returns the result of a sync callback', () => { + const fallback = vi.fn(() => 'fallback'); + + expect(safeCallback('callback threw:', () => 'value', fallback)).toBe('value'); + expect(fallback).not.toHaveBeenCalled(); + expect(debugErrorSpy).not.toHaveBeenCalled(); + }); + + it('returns the fallback and logs when a sync callback throws', () => { + const error = new Error('boom'); + const fallback = vi.fn(() => 'fallback'); + + expect( + safeCallback( + 'callback threw:', + () => { + throw error; + }, + fallback, + ), + ).toBe('fallback'); + expect(fallback).toHaveBeenCalledWith(error); + expect(debugErrorSpy).toHaveBeenCalledWith('callback threw:', error); + }); + + it('resolves to the result of an async callback', async () => { + const fallback = vi.fn(async () => 'fallback'); + + const result = safeCallback('callback threw:', async () => 'value', fallback); + + expect(result).toBeInstanceOf(Promise); + await expect(result).resolves.toBe('value'); + expect(fallback).not.toHaveBeenCalled(); + expect(debugErrorSpy).not.toHaveBeenCalled(); + }); + + it('resolves to the fallback and logs when an async callback rejects', async () => { + const error = new Error('boom'); + const fallback = vi.fn(async () => 'fallback'); + + const result = safeCallback('callback threw:', () => Promise.reject(error), fallback); + + await expect(result).resolves.toBe('fallback'); + expect(fallback).toHaveBeenCalledWith(error); + expect(debugErrorSpy).toHaveBeenCalledWith('callback threw:', error); + }); + + it('does not treat non-thenable objects as promises', () => { + const value = { then: 'not a function' }; + + expect( + safeCallback( + 'callback threw:', + () => value, + () => ({ then: 'fallback' }), + ), + ).toBe(value); + }); +}); diff --git a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 322a82dba495..7eea8ff59c42 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -29,6 +29,7 @@ import { isTracingSuppressed, LRUMap, parseUrl, + safeCallback, SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, @@ -110,16 +111,6 @@ export function instrumentUndici(config: NodeFetchOptions = {}): void { subscribeToChannel('undici:request:error', message => onError(message as RequestErrorMessage)); } -/** Replaces OTel's `safeExecuteInTheMiddle`: run `fn`, route any error to `onError`, and swallow it. */ -function safeExecute(fn: () => T, onError: (error: unknown) => void): T | undefined { - try { - return fn(); - } catch (error) { - onError(error); - return undefined; - } -} - function subscribeToChannel( diagnosticChannel: string, onMessage: (message: unknown, name: string | symbol) => void, @@ -177,9 +168,10 @@ function parseRequestHeaders(request: UndiciRequest): Map !!config.ignoreOutgoingRequests?.(url), - e => e && DEBUG_BUILD && debug.error('caught ignoreOutgoingRequests error: ', e), + () => false, ); // Breadcrumbs & span-less trace propagation are additionally skipped when tracing is suppressed. @@ -277,9 +269,10 @@ function onRequestCreated(config: NodeFetchOptions, { request }: RequestMessage) }); // Execute the request hook if defined - safeExecute( + safeCallback( + 'The `requestHook` callback threw an error:', () => config.requestHook?.(span, request), - e => e && DEBUG_BUILD && debug.error('caught requestHook error: ', e), + () => undefined, ); // Context propagation goes last so no hook can tamper the propagation headers. @@ -345,9 +338,10 @@ function onResponseHeaders(config: NodeFetchOptions, { request, response }: Resp }; // Execute the response hook if defined - safeExecute( + safeCallback( + 'The `responseHook` callback threw an error:', () => config.responseHook?.(span, { request, response }), - e => e && DEBUG_BUILD && debug.error('caught responseHook error: ', e), + () => undefined, ); if (config.headersToSpanAttributes?.responseHeaders) {