diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d0d573808..288cea4f37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- SDK Gracefully downgrades when callback throws an error ([#2502](https://github.com/getsentry/sentry-react-native/pull/2502)) + ### Features - Add ClientReports ([#2496](https://github.com/getsentry/sentry-react-native/pull/2496)) diff --git a/src/js/client.ts b/src/js/client.ts index 2d34d213e5..d5e4c6093f 100644 --- a/src/js/client.ts +++ b/src/js/client.ts @@ -167,12 +167,13 @@ export class ReactNativeClient extends BaseClient { didCallNativeInit = await NATIVE.initNativeSdk(this._options); } catch (_) { this._showCannotConnectDialog(); - - this._options.onReady?.({ didCallNativeInit: false }); - - return; + } finally { + try { + this._options.onReady?.({ didCallNativeInit }); + } catch (error) { + logger.error('The OnReady callback threw an error: ', error); + } } - this._options.onReady?.({ didCallNativeInit }); } /** diff --git a/src/js/index.ts b/src/js/index.ts index b1848707b1..31e3ca61f1 100644 --- a/src/js/index.ts +++ b/src/js/index.ts @@ -29,7 +29,6 @@ export { setTags, setUser, startTransaction, - withScope, } from '@sentry/core'; // We need to import it so we patch the hub with global functions @@ -66,6 +65,7 @@ export { flush, close, captureUserFeedback, + withScope, } from './sdk'; export { TouchEventBoundary, withTouchEventBoundary } from './touchevents'; diff --git a/src/js/sdk.tsx b/src/js/sdk.tsx index 754acfc772..98ee7e5b0b 100644 --- a/src/js/sdk.tsx +++ b/src/js/sdk.tsx @@ -6,7 +6,7 @@ import { defaultStackParser, getCurrentHub, } from '@sentry/react'; -import { Integration, StackFrame, UserFeedback } from '@sentry/types'; +import { Integration, Scope, StackFrame, UserFeedback } from '@sentry/types'; import { getGlobalObject, logger, stackParserFromStackParserOptions } from '@sentry/utils'; import * as React from 'react'; @@ -25,6 +25,7 @@ import { TouchEventBoundary } from './touchevents'; import { ReactNativeProfiler, ReactNativeTracing } from './tracing'; import { makeReactNativeTransport } from './transports/native'; import { makeUtf8TextEncoder } from './transports/TextEncoder'; +import { safeFactory, safeTracesSampler } from './utils/safe'; const IGNORED_DEFAULT_INTEGRATIONS = [ 'GlobalHandlers', // We will use the react-native internal handlers @@ -54,13 +55,17 @@ export function init(passedOptions: ReactNativeOptions): void { const options: ReactNativeClientOptions = { ...DEFAULT_OPTIONS, ...passedOptions, + // If custom transport factory fails the SDK won't initialize transport: passedOptions.transport || makeReactNativeTransport, transportOptions: { ...DEFAULT_OPTIONS.transportOptions, ...(passedOptions.transportOptions ?? {}), }, integrations: [], - stackParser: stackParserFromStackParserOptions(passedOptions.stackParser || defaultStackParser) + stackParser: stackParserFromStackParserOptions(passedOptions.stackParser || defaultStackParser), + beforeBreadcrumb: safeFactory(passedOptions.beforeBreadcrumb, { loggerMessage: 'The beforeBreadcrumb threw an error' }), + initialScope: safeFactory(passedOptions.initialScope, { loggerMessage: 'The initialScope threw an error' }), + tracesSampler: safeTracesSampler(passedOptions.tracesSampler), }; // As long as tracing is opt in with either one of these options, then this is how we determine tracing is enabled. @@ -121,7 +126,7 @@ export function init(passedOptions: ReactNativeOptions): void { } options.integrations = getIntegrationsToSetup({ - integrations: passedOptions.integrations, + integrations: safeFactory(passedOptions.integrations, { loggerMessage: 'The integrations threw an error' }), defaultIntegrations, }); initAndBind(ReactNativeClient, options); @@ -232,4 +237,28 @@ export async function close(): Promise { */ export function captureUserFeedback(feedback: UserFeedback): void { getCurrentHub().getClient()?.captureUserFeedback(feedback); + } + +/** + * Creates a new scope with and executes the given operation within. + * The scope is automatically removed once the operation + * finishes or throws. + * + * This is essentially a convenience function for: + * + * pushScope(); + * callback(); + * popScope(); + * + * @param callback that will be enclosed into push/popScope. + */ +export function withScope(callback: (scope: Scope) => void): ReturnType { + const safeCallback = (scope: Scope): void => { + try { + callback(scope); + } catch (e) { + logger.error('Error while running withScope callback', e); + } + }; + getCurrentHub().withScope(safeCallback); } diff --git a/src/js/utils/safe.ts b/src/js/utils/safe.ts new file mode 100644 index 0000000000..c13fe9ac81 --- /dev/null +++ b/src/js/utils/safe.ts @@ -0,0 +1,65 @@ +import { logger } from '@sentry/utils'; + +import { ReactNativeOptions } from '../options'; + +type DangerTypesWithoutCallSignature = +// eslint-disable-next-line @typescript-eslint/ban-types + | 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. + */ +export function safeFactory( + danger: ((...args: A) => R) | T, + options: { + loggerMessage?: string; + } = {}, +): ((...args: A) => R) | T { + if (typeof danger === 'function') { + return (...args) => { + try { + return danger(...args); + } catch (error) { + logger.error( + options.loggerMessage + ? options.loggerMessage + : `The ${danger.name} callback threw an error`, + error, + ); + return args[0]; + } + }; + } else { + return danger; + } +} + +type TracesSampler = Required['tracesSampler']; + +/** + * Returns sage tracesSampler that returns 0 if the original failed. + */ +export function safeTracesSampler( + tracesSampler: ReactNativeOptions['tracesSampler'], +): ReactNativeOptions['tracesSampler'] { + if (tracesSampler) { + return ( + ...args: Parameters + ): ReturnType => { + try { + return tracesSampler(...args); + } catch (error) { + logger.error('The tracesSampler callback threw an error', error); + return 0; + } + } + } else { + return tracesSampler; + } +} diff --git a/test/client.test.ts b/test/client.test.ts index b744fbf445..694887d089 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -13,6 +13,7 @@ import { envelopeItemPayload, envelopeItems, firstArg, + flushPromises, getMockSession, getMockUserFeedback, getSyncPromiseRejectOnFirstCall, diff --git a/test/sdk.test.ts b/test/sdk.test.ts index 1514ffcbd2..6b3e49f6bc 100644 --- a/test/sdk.test.ts +++ b/test/sdk.test.ts @@ -4,6 +4,8 @@ interface MockedClient { flush: jest.Mock; } +let mockedGetCurrentHubWithScope: jest.Mock; + jest.mock('@sentry/react', () => { const actualModule = jest.requireActual('@sentry/react'); @@ -13,10 +15,14 @@ jest.mock('@sentry/react', () => { return { ...actualModule, - getCurrentHub: jest.fn(() => ({ - getClient: jest.fn(() => mockClient), - setTag: jest.fn(), - })), + getCurrentHub: jest.fn(() => { + mockedGetCurrentHubWithScope = jest.fn(); + return { + getClient: jest.fn(() => mockClient), + setTag: jest.fn(), + withScope: mockedGetCurrentHubWithScope, + }; + }), defaultIntegrations: [ { name: 'MockedDefaultReactIntegration', setupOnce: jest.fn() } ], }; }); @@ -53,10 +59,10 @@ jest.spyOn(logger, 'error'); import { initAndBind } from '@sentry/core'; import { getCurrentHub } from '@sentry/react'; -import { Integration } from '@sentry/types'; +import { Integration, Scope } from '@sentry/types'; import { ReactNativeClientOptions } from '../src/js/options'; -import { flush, init } from '../src/js/sdk'; +import { flush, init, withScope } from '../src/js/sdk'; import { ReactNativeTracing, ReactNavigationInstrumentation } from '../src/js/tracing'; import { firstArg, secondArg } from './testutils'; @@ -172,6 +178,62 @@ describe('Tests the SDK functionality', () => { }); }); + describe('initIsSafe', () => { + test('initialScope callback is safe after init', () => { + const mockInitialScope = jest.fn(() => { throw 'Test error' }); + + init({ initialScope: mockInitialScope }); + + expect(() => { + (mockedInitAndBind.mock.calls[0][secondArg].initialScope as (scope: Scope) => Scope)({} as any); + }).not.toThrow(); + expect(mockInitialScope).toBeCalledTimes(1); + }); + test('beforeBreadcrumb callback is safe after init', () => { + const mockBeforeBreadcrumb = jest.fn(() => { throw 'Test error' }); + + init({ beforeBreadcrumb: mockBeforeBreadcrumb }); + + expect(() => { + mockedInitAndBind.mock.calls[0][secondArg].beforeBreadcrumb?.({} as any); + }).not.toThrow(); + expect(mockBeforeBreadcrumb).toBeCalledTimes(1); + }); + + test('integrations callback should not crash init', () => { + const mockIntegrations = jest.fn(() => { throw 'Test error' }); + + expect(() => { + init({ integrations: mockIntegrations }); + }).not.toThrow(); + expect(mockIntegrations).toBeCalledTimes(1); + }); + + test('tracesSampler callback is safe after init', () => { + const mockTraceSampler = jest.fn(() => { throw 'Test error' }); + + init({ tracesSampler: mockTraceSampler }); + + expect(() => { + mockedInitAndBind.mock.calls[0][secondArg].tracesSampler?.({} as any); + }).not.toThrow(); + expect(mockTraceSampler).toBeCalledTimes(1); + }); + }); + + describe('withScope', () => { + test('withScope callback does not throw', () => { + const mockScopeCallback = jest.fn(() => { throw 'Test error' }); + + withScope(mockScopeCallback); + + expect(() => { + (mockedGetCurrentHubWithScope.mock.calls[0][firstArg] as (scope: Scope) => void)({} as any); + }).not.toThrow(); + expect(mockScopeCallback).toBeCalledTimes(1); + }); + }); + describe('integrations', () => { it('replaces default integrations', () => { const mockDefaultIntegration = getMockedIntegration(); diff --git a/test/testutils.ts b/test/testutils.ts index 51962daa05..b987f9486e 100644 --- a/test/testutils.ts +++ b/test/testutils.ts @@ -64,3 +64,5 @@ export const getSyncPromiseRejectOnFirstCall = (reason: unknown } }); }; + +export const flushPromises = (): Promise => new Promise(setImmediate); diff --git a/test/utils/safe.test.ts b/test/utils/safe.test.ts new file mode 100644 index 0000000000..40ef2454b8 --- /dev/null +++ b/test/utils/safe.test.ts @@ -0,0 +1,62 @@ +import { safeFactory, safeTracesSampler } from '../../src/js/utils/safe'; + +describe('safe', () => { + describe('safeFactory', () => { + test('calls given function with correct args', () => { + const mockFn = jest.fn(); + const actualSafeFunction = safeFactory(mockFn); + actualSafeFunction('foo', 'bar'); + expect(mockFn).toBeCalledTimes(1); + expect(mockFn).toBeCalledWith('foo', 'bar'); + }); + test('calls given function amd return its result', () => { + const mockFn = jest.fn(() => 'bar'); + const actualSafeFunction = safeFactory(mockFn); + const actualResult = actualSafeFunction('foo'); + expect(mockFn).toBeCalledTimes(1); + expect(actualResult).toBe('bar'); + }); + test('passes undefined trough', () => { + const actualSafeFunction = safeFactory(undefined); + expect(actualSafeFunction).not.toBeDefined(); + }); + test('passes object trough', () => { + const actualSafeFunction = safeFactory({ 'foo': 'bar' }); + expect(actualSafeFunction).toEqual({ 'foo': 'bar' }); + }); + test('returns input object if function failed', () => { + const mockFn = jest.fn(() => { throw 'Test error' }); + const actualSafeFunction = safeFactory(<(foo:string) => string>mockFn); + const actualResult = actualSafeFunction('foo'); + expect(mockFn).toBeCalledTimes(1); + expect(actualResult).toEqual('foo'); + }); + }); + describe('safeTracesSampler', () => { + test('calls given function with correct args', () => { + const mockFn = jest.fn(); + const actualSafeFunction = safeTracesSampler(mockFn); + actualSafeFunction?.({ transactionContext: { name: 'foo' } }); + expect(mockFn).toBeCalledTimes(1); + expect(mockFn).toBeCalledWith({ transactionContext: { name: 'foo' } }); + }); + test('calls given function amd return its result', () => { + const mockFn = jest.fn(() => 0.5); + const actualSafeFunction = safeTracesSampler(mockFn); + const actualResult = actualSafeFunction?.({ transactionContext: { name: 'foo' } }); + expect(mockFn).toBeCalledTimes(1); + expect(actualResult).toBe(0.5); + }); + test('passes undefined trough', () => { + const actualSafeFunction = safeTracesSampler(undefined); + expect(actualSafeFunction).not.toBeDefined(); + }); + test('returns input object if function failed', () => { + const mockFn = jest.fn(() => { throw 'Test error' }); + const actualSafeFunction = safeTracesSampler(mockFn); + const actualResult = actualSafeFunction?.({ transactionContext: { name: 'foo' } }); + expect(mockFn).toBeCalledTimes(1); + expect(actualResult).toEqual(0); + }); + }); +});