From 224983811515bb712015f7d7269155bfd823c73e Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Sep 2026 11:09:27 +0200 Subject: [PATCH 1/3] fix(core): Align beforeBreadcrumb and tracesSampler error fallbacks with spec Per the Callback Error Isolation spec: - A throwing `beforeBreadcrumb` now drops the breadcrumb (returns null) instead of keeping the unmodified one. - A throwing `tracesSampler` now falls back to the configured `tracesSampleRate` instead of substituting a hardcoded 0. Fixes #6668 Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 4 +++ packages/core/src/js/sdk.tsx | 4 ++- packages/core/src/js/utils/safe.ts | 24 ++++++++++++----- packages/core/test/sdk.test.ts | 17 ++++++++---- packages/core/test/utils/safe.test.ts | 38 ++++++++++++++++++++++----- 5 files changed, 69 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bc796cdc6..5fa007228a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ## Unreleased +### Fixes + +- A throwing `beforeBreadcrumb` now drops the breadcrumb, and a throwing `tracesSampler` now falls back to the configured `tracesSampleRate` instead of forcing 0% sampling ([#6668](https://github.com/getsentry/sentry-react-native/issues/6668)) + ### Dependencies - Bump Cocoa SDK from v9.26.1 to v9.27.0 ([#6670](https://github.com/getsentry/sentry-react-native/pull/6670)) diff --git a/packages/core/src/js/sdk.tsx b/packages/core/src/js/sdk.tsx index a5f00c4f05..3b78309d95 100644 --- a/packages/core/src/js/sdk.tsx +++ b/packages/core/src/js/sdk.tsx @@ -111,6 +111,8 @@ export function init(passedOptions: ReactNativeOptions): void { const userBeforeBreadcrumb = safeFactory(userOptions.beforeBreadcrumb, { loggerMessage: 'The beforeBreadcrumb threw an error', + // Per the Callback Error Isolation spec, drop the breadcrumb when the callback throws. + onError: () => null, }); // Exclude Dev Server and Sentry Dsn request from Breadcrumbs @@ -175,7 +177,7 @@ export function init(passedOptions: ReactNativeOptions): void { } if ('tracesSampler' in options) { - options.tracesSampler = safeTracesSampler(options.tracesSampler); + options.tracesSampler = safeTracesSampler(options.tracesSampler, options.tracesSampleRate); } if (!('environment' in options)) { diff --git a/packages/core/src/js/utils/safe.ts b/packages/core/src/js/utils/safe.ts index 6c40101bcb..f4db1456b0 100644 --- a/packages/core/src/js/utils/safe.ts +++ b/packages/core/src/js/utils/safe.ts @@ -8,13 +8,19 @@ type DangerTypesWithoutCallSignature = object | null | undefined; * Returns callback factory wrapped with try/catch * or the original passed value is it's not a function. * - * If the factory fails original data are returned as it. - * They might be partially modified by the failed function. + * If the factory fails the original data are returned as is (they might be + * partially modified by the failed function), unless an `onError` handler is + * provided to compute a different fallback value. */ export function safeFactory( danger: ((...args: A) => R) | T, options: { loggerMessage?: string; + /** + * Computes the value returned when the wrapped function throws. + * Defaults to returning the first argument (the unmodified input). + */ + onError?: (...args: A) => R; } = {}, ): ((...args: A) => R) | T { if (typeof danger === 'function') { @@ -26,7 +32,7 @@ export function safeFactory['tracesSampler']; /** - * Returns sage tracesSampler that returns 0 if the original failed. + * Returns a safe tracesSampler that falls back to the configured `tracesSampleRate` + * if the original callback throws. + * + * Per the Callback Error Isolation spec the fallback MUST NOT substitute a hardcoded + * `0` or `1`; when no `tracesSampleRate` is configured the returned `undefined` lets + * the core sampling pipeline discard the transaction. */ export function safeTracesSampler( tracesSampler: ReactNativeOptions['tracesSampler'], + tracesSampleRate: ReactNativeOptions['tracesSampleRate'], ): ReactNativeOptions['tracesSampler'] { if (tracesSampler) { return (...args: Parameters): ReturnType => { try { return tracesSampler(...args); } catch (error) { - debug.error('The tracesSampler callback threw an error', error); - return 0; + debug.error('The tracesSampler callback threw an error, falling back to tracesSampleRate', error); + return tracesSampleRate as ReturnType; } }; } else { diff --git a/packages/core/test/sdk.test.ts b/packages/core/test/sdk.test.ts index f22052ace2..1dd33cc1cb 100644 --- a/packages/core/test/sdk.test.ts +++ b/packages/core/test/sdk.test.ts @@ -408,17 +408,20 @@ describe('Tests the SDK functionality', () => { }).not.toThrow(); expect(mockInitialScope).toHaveBeenCalledTimes(1); }); - test('beforeBreadcrumb callback is safe after init', () => { + test('beforeBreadcrumb callback is safe after init and drops the breadcrumb on error', () => { const mockBeforeBreadcrumb = jest.fn(() => { throw 'Test error'; }); init({ beforeBreadcrumb: mockBeforeBreadcrumb }); + let result: ReturnType>['beforeBreadcrumb']> | undefined; expect(() => { - usedOptions()?.beforeBreadcrumb?.({} as any); + result = usedOptions()?.beforeBreadcrumb?.({ message: 'test' } as any); }).not.toThrow(); expect(mockBeforeBreadcrumb).toHaveBeenCalledTimes(1); + // Per the Callback Error Isolation spec the breadcrumb is dropped (null) on error. + expect(result).toBeNull(); }); test('integrations callback should not crash init', () => { @@ -432,17 +435,21 @@ describe('Tests the SDK functionality', () => { expect(mockIntegrations).toHaveBeenCalledTimes(1); }); - test('tracesSampler callback is safe after init', () => { + test('tracesSampler callback is safe after init and falls back to tracesSampleRate on error', () => { const mockTraceSampler = jest.fn(() => { throw 'Test error'; }); - init({ tracesSampler: mockTraceSampler }); + init({ tracesSampler: mockTraceSampler, tracesSampleRate: 0.42 }); + let result: number | boolean | undefined; expect(() => { - usedOptions()?.tracesSampler?.({} as any); + result = usedOptions()?.tracesSampler?.({} as any); }).not.toThrow(); expect(mockTraceSampler).toHaveBeenCalledTimes(1); + // Per the Callback Error Isolation spec the sampler falls back to the configured + // tracesSampleRate rather than substituting a hardcoded 0. + expect(result).toBe(0.42); }); }); diff --git a/packages/core/test/utils/safe.test.ts b/packages/core/test/utils/safe.test.ts index d023ea3517..1c7bcd00eb 100644 --- a/packages/core/test/utils/safe.test.ts +++ b/packages/core/test/utils/safe.test.ts @@ -33,11 +33,22 @@ describe('safe', () => { expect(mockFn).toHaveBeenCalledTimes(1); expect(actualResult).toEqual('foo'); }); + test('returns onError result if function failed and onError is provided', () => { + const mockFn = jest.fn(() => { + throw 'Test error'; + }); + const actualSafeFunction = safeFactory(<(foo: string) => string | null>mockFn, { + onError: () => null, + }); + const actualResult = actualSafeFunction('foo'); + expect(mockFn).toHaveBeenCalledTimes(1); + expect(actualResult).toBeNull(); + }); }); describe('safeTracesSampler', () => { test('calls given function with correct args', () => { const mockFn = jest.fn(); - const actualSafeFunction = safeTracesSampler(mockFn); + const actualSafeFunction = safeTracesSampler(mockFn, undefined); const expectedInheritOrSampleWith = function (fallbackSampleRate: number): number { return fallbackSampleRate; }; @@ -55,7 +66,7 @@ describe('safe', () => { }); test('calls given function amd return its result', () => { const mockFn = jest.fn(() => 0.5); - const actualSafeFunction = safeTracesSampler(mockFn); + const actualSafeFunction = safeTracesSampler(mockFn, undefined); const actualResult = actualSafeFunction?.({ name: 'foo', transactionContext: { name: 'foo' }, @@ -67,14 +78,29 @@ describe('safe', () => { expect(actualResult).toBe(0.5); }); test('passes undefined trough', () => { - const actualSafeFunction = safeTracesSampler(undefined); + const actualSafeFunction = safeTracesSampler(undefined, undefined); expect(actualSafeFunction).not.toBeDefined(); }); - test('returns input object if function failed', () => { + test('falls back to the configured tracesSampleRate if the function failed', () => { + const mockFn = jest.fn(() => { + throw 'Test error'; + }); + const actualSafeFunction = safeTracesSampler(mockFn, 0.25); + const actualResult = actualSafeFunction?.({ + name: 'foo', + transactionContext: { name: 'foo' }, + inheritOrSampleWith: function (fallbackSampleRate: number): number { + return fallbackSampleRate; + }, + }); + expect(mockFn).toHaveBeenCalledTimes(1); + expect(actualResult).toEqual(0.25); + }); + test('returns undefined if the function failed and no tracesSampleRate is configured', () => { const mockFn = jest.fn(() => { throw 'Test error'; }); - const actualSafeFunction = safeTracesSampler(mockFn); + const actualSafeFunction = safeTracesSampler(mockFn, undefined); const actualResult = actualSafeFunction?.({ name: 'foo', transactionContext: { name: 'foo' }, @@ -83,7 +109,7 @@ describe('safe', () => { }, }); expect(mockFn).toHaveBeenCalledTimes(1); - expect(actualResult).toEqual(0); + expect(actualResult).toBeUndefined(); }); }); }); From 728ce2fc95580defc6723f974d0b8046b2b2378f Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Sep 2026 11:10:55 +0200 Subject: [PATCH 2/3] docs: Reference PR in changelog entry Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa007228a..ebcfef4196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Fixes -- A throwing `beforeBreadcrumb` now drops the breadcrumb, and a throwing `tracesSampler` now falls back to the configured `tracesSampleRate` instead of forcing 0% sampling ([#6668](https://github.com/getsentry/sentry-react-native/issues/6668)) +- A throwing `beforeBreadcrumb` now drops the breadcrumb, and a throwing `tracesSampler` now falls back to the configured `tracesSampleRate` instead of forcing 0% sampling ([#6675](https://github.com/getsentry/sentry-react-native/pull/6675)) ### Dependencies From f52b6c81df8223ad6000ec22d83125464c5c7a99 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 4 Sep 2026 11:14:34 +0200 Subject: [PATCH 3/3] docs: Simplify changelog entry wording Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebcfef4196..29a3f35e8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ ### Fixes -- A throwing `beforeBreadcrumb` now drops the breadcrumb, and a throwing `tracesSampler` now falls back to the configured `tracesSampleRate` instead of forcing 0% sampling ([#6675](https://github.com/getsentry/sentry-react-native/pull/6675)) +- A throwing `beforeBreadcrumb` now drops the breadcrumb, and a throwing `tracesSampler` now falls back to the configured `tracesSampleRate` ([#6675](https://github.com/getsentry/sentry-react-native/pull/6675)) ### Dependencies