diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4bc796cdc6..29a3f35e8e 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` ([#6675](https://github.com/getsentry/sentry-react-native/pull/6675))
+
### 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();
});
});
});