diff --git a/CHANGELOG.md b/CHANGELOG.md index 184207010c..69def9e635 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Encode envelopes using Base64, fix array length limit when transferring over Bridge. ([#2852](https://github.com/getsentry/sentry-react-native/pull/2852)) - This fix requires a rebuild of the native app - Symbolicate message and non-Error stacktraces locally in debug mode ([#3420](https://github.com/getsentry/sentry-react-native/pull/3420)) +- Remove Sentry SDK frames from rejected promise SyntheticError stack ([#3423](https://github.com/getsentry/sentry-react-native/pull/3423)) ## 5.14.1 diff --git a/src/js/integrations/debugsymbolicator.ts b/src/js/integrations/debugsymbolicator.ts index ef29bd9a3d..3a4b98bfa7 100644 --- a/src/js/integrations/debugsymbolicator.ts +++ b/src/js/integrations/debugsymbolicator.ts @@ -1,6 +1,7 @@ import type { Event, EventHint, EventProcessor, Hub, Integration, StackFrame as SentryStackFrame } from '@sentry/types'; import { addContextToFrame, logger } from '@sentry/utils'; +import { getFramesToPop, isErrorLike } from '../utils/error'; import type * as ReactNative from '../vendor/react-native'; const INTERNAL_CALLSITES_REGEX = new RegExp(['ReactNativeRenderer-dev\\.js$', 'MessageQueue\\.js$'].join('|')); @@ -37,24 +38,19 @@ export class DebugSymbolicator implements Integration { return event; } - if ( - event.exception && - hint.originalException && - typeof hint.originalException === 'object' && - 'stack' in hint.originalException && - typeof hint.originalException.stack === 'string' - ) { + if (event.exception && isErrorLike(hint.originalException)) { // originalException is ErrorLike object - const symbolicatedFrames = await this._symbolicate(hint.originalException.stack); + const symbolicatedFrames = await this._symbolicate( + hint.originalException.stack, + getFramesToPop(hint.originalException as Error), + ); symbolicatedFrames && this._replaceExceptionFramesInEvent(event, symbolicatedFrames); - } else if ( - hint.syntheticException && - typeof hint.syntheticException === 'object' && - 'stack' in hint.syntheticException && - typeof hint.syntheticException.stack === 'string' - ) { + } else if (hint.syntheticException && isErrorLike(hint.syntheticException)) { // syntheticException is Error object - const symbolicatedFrames = await this._symbolicate(hint.syntheticException.stack); + const symbolicatedFrames = await this._symbolicate( + hint.syntheticException.stack, + getFramesToPop(hint.syntheticException), + ); if (event.exception) { symbolicatedFrames && this._replaceExceptionFramesInEvent(event, symbolicatedFrames); @@ -73,7 +69,7 @@ export class DebugSymbolicator implements Integration { * Symbolicates the stack on the device talking to local dev server. * Mutates the passed event. */ - private async _symbolicate(rawStack: string): Promise { + private async _symbolicate(rawStack: string, skipFirstFrames: number = 0): Promise { const parsedStack = this._parseErrorStack(rawStack); try { @@ -86,7 +82,14 @@ export class DebugSymbolicator implements Integration { // This has been changed in an react-native version so stack is contained in here const newStack = prettyStack.stack || prettyStack; - const stackWithoutInternalCallsites = newStack.filter( + // https://github.com/getsentry/sentry-javascript/blob/739d904342aaf9327312f409952f14ceff4ae1ab/packages/utils/src/stacktrace.ts#L23 + // Match SentryParser which counts lines of stack (-1 for first line with the Error message) + const skipFirstAdjustedToSentryStackParser = Math.max(skipFirstFrames - 1, 0); + const stackWithoutPoppedFrames = skipFirstAdjustedToSentryStackParser + ? newStack.slice(skipFirstAdjustedToSentryStackParser) + : newStack; + + const stackWithoutInternalCallsites = stackWithoutPoppedFrames.filter( (frame: { file?: string }) => frame.file && frame.file.match(INTERNAL_CALLSITES_REGEX) === null, ); diff --git a/src/js/integrations/reactnativeerrorhandlers.ts b/src/js/integrations/reactnativeerrorhandlers.ts index 9a6a493def..158a9f143b 100644 --- a/src/js/integrations/reactnativeerrorhandlers.ts +++ b/src/js/integrations/reactnativeerrorhandlers.ts @@ -3,6 +3,7 @@ import type { EventHint, Integration, SeverityLevel } from '@sentry/types'; import { addExceptionMechanism, logger } from '@sentry/utils'; import type { ReactNativeClient } from '../client'; +import { createSyntheticError, isErrorLike } from '../utils/error'; import { RN_GLOBAL_OBJ } from '../utils/worldwide'; /** ReactNativeErrorHandlers Options */ @@ -100,11 +101,7 @@ export class ReactNativeErrorHandlers implements Integration { * Attach the unhandled rejection handler */ private _attachUnhandledRejectionHandler(): void { - const tracking: { - disable: () => void; - enable: (arg: unknown) => void; - // eslint-disable-next-line import/no-extraneous-dependencies,@typescript-eslint/no-var-requires - } = require('promise/setimmediate/rejection-tracking'); + const tracking = this._loadRejectionTracking(); const promiseRejectionTrackingOptions: PromiseRejectionTrackingOptions = { onUnhandled: (id, rejection = {}) => { @@ -123,7 +120,7 @@ export class ReactNativeErrorHandlers implements Integration { tracking.enable({ allRejections: true, - onUnhandled: (id: string, error: Error) => { + onUnhandled: (id: string, error: unknown) => { if (__DEV__) { promiseRejectionTrackingOptions.onUnhandled(id, error); } @@ -131,6 +128,7 @@ export class ReactNativeErrorHandlers implements Integration { getCurrentHub().captureException(error, { data: { id }, originalException: error, + syntheticException: isErrorLike(error) ? undefined : createSyntheticError(), }); }, onHandled: (id: string) => { @@ -251,4 +249,15 @@ export class ReactNativeErrorHandlers implements Integration { }); } } + + /** + * Loads and returns rejection tracking module + */ + private _loadRejectionTracking(): { + disable: () => void; + enable: (arg: unknown) => void; + } { + // eslint-disable-next-line @typescript-eslint/no-var-requires,import/no-extraneous-dependencies + return require('promise/setimmediate/rejection-tracking'); + } } diff --git a/src/js/utils/error.ts b/src/js/utils/error.ts new file mode 100644 index 0000000000..044e3a8516 --- /dev/null +++ b/src/js/utils/error.ts @@ -0,0 +1,38 @@ +export interface ExtendedError extends Error { + framesToPop?: number | undefined; +} + +// Sentry Stack Parser is skipping lines not frames +// https://github.com/getsentry/sentry-javascript/blob/739d904342aaf9327312f409952f14ceff4ae1ab/packages/utils/src/stacktrace.ts#L23 +// 1 for first line with the Error message +const SENTRY_STACK_PARSER_OFFSET = 1; +const REMOVE_ERROR_CREATION_FRAMES = 2 + SENTRY_STACK_PARSER_OFFSET; + +/** + * Creates synthetic trace. By default pops 2 frames - `createSyntheticError` and the caller + */ +export function createSyntheticError(framesToPop: number = 0): ExtendedError { + const error: ExtendedError = new Error(); + error.framesToPop = framesToPop + REMOVE_ERROR_CREATION_FRAMES; // Skip createSyntheticError's own stack frame. + return error; +} + +/** + * Returns the number of frames to pop from the stack trace. + * @param error ExtendedError + */ +export function getFramesToPop(error: ExtendedError): number { + return error.framesToPop !== undefined ? error.framesToPop : 0; +} + +/** + * Check if `potentialError` is an object with string stack property. + */ +export function isErrorLike(potentialError: unknown): potentialError is { stack: string } { + return ( + potentialError !== null && + typeof potentialError === 'object' && + 'stack' in potentialError && + typeof potentialError.stack === 'string' + ); +} diff --git a/test/error.test.ts b/test/error.test.ts new file mode 100644 index 0000000000..85fc3ee7ea --- /dev/null +++ b/test/error.test.ts @@ -0,0 +1,25 @@ +import { isErrorLike } from '../src/js/utils/error'; + +describe('error', () => { + describe('isErrorLike', () => { + test('returns true for Error object', () => { + expect(isErrorLike(new Error('test'))).toBe(true); + }); + + test('returns true for ErrorLike object', () => { + expect(isErrorLike({ stack: 'test' })).toBe(true); + }); + + test('returns false for non object', () => { + expect(isErrorLike('test')).toBe(false); + }); + + test('returns false for object without stack', () => { + expect(isErrorLike({})).toBe(false); + }); + + test('returns false for object with non string stack', () => { + expect(isErrorLike({ stack: 1 })).toBe(false); + }); + }); +}); diff --git a/test/integrations/debugsymbolicator.test.ts b/test/integrations/debugsymbolicator.test.ts index 34cebc688d..ab1465d8a0 100644 --- a/test/integrations/debugsymbolicator.test.ts +++ b/test/integrations/debugsymbolicator.test.ts @@ -247,6 +247,98 @@ describe('Debug Symbolicator Integration', () => { }, }); }); + + it('skips first frame (callee) for exception', async () => { + const symbolicatedEvent = await executeIntegrationFor( + { + exception: { + values: [ + { + type: 'Error', + value: 'Error: test', + stacktrace: { + frames: mockSentryParsedFrames, + }, + }, + ], + }, + }, + { + originalException: { + stack: mockRawStack, + framesToPop: 2, + // The current behavior matches https://github.com/getsentry/sentry-javascript/blob/739d904342aaf9327312f409952f14ceff4ae1ab/packages/utils/src/stacktrace.ts#L23 + // 2 for first line with the Error message + }, + }, + ); + + expect(symbolicatedEvent).toStrictEqual({ + exception: { + values: [ + { + type: 'Error', + value: 'Error: test', + stacktrace: { + frames: [ + { + function: 'bar', + filename: '/User/project/node_modules/bar/bar.js', + lineno: 2, + colno: 2, + in_app: false, + }, + ], + }, + }, + ], + }, + }); + }); + + it('skips first frame (callee) for message', async () => { + const symbolicatedEvent = await executeIntegrationFor( + { + threads: { + values: [ + { + stacktrace: { + frames: mockSentryParsedFrames, + }, + }, + ], + }, + }, + { + syntheticException: { + stack: mockRawStack, + framesToPop: 2, + // The current behavior matches https://github.com/getsentry/sentry-javascript/blob/739d904342aaf9327312f409952f14ceff4ae1ab/packages/utils/src/stacktrace.ts#L23 + // 2 for first line with the Error message + } as unknown as Error, + }, + ); + + expect(symbolicatedEvent).toStrictEqual({ + threads: { + values: [ + { + stacktrace: { + frames: [ + { + function: 'bar', + filename: '/User/project/node_modules/bar/bar.js', + lineno: 2, + colno: 2, + in_app: false, + }, + ], + }, + }, + ], + }, + }); + }); }); function executeIntegrationFor(mockedEvent: Event, hint: EventHint): Promise { diff --git a/test/integrations/reactnativeerrorhandlers.test.ts b/test/integrations/reactnativeerrorhandlers.test.ts index 6a7067eb44..3a4285f3e0 100644 --- a/test/integrations/reactnativeerrorhandlers.test.ts +++ b/test/integrations/reactnativeerrorhandlers.test.ts @@ -6,6 +6,8 @@ const mockBrowserClient: BrowserClient = new BrowserClient({ transport: jest.fn(), }); +let mockHubCaptureException: jest.Mock; + jest.mock('@sentry/core', () => { const core = jest.requireActual('@sentry/core'); @@ -23,8 +25,11 @@ jest.mock('@sentry/core', () => { getClient: () => client, getScope: () => scope, captureEvent: jest.fn(), + captureException: jest.fn(), }; + mockHubCaptureException = hub.captureException; + return { ...core, addGlobalEventProcessor: jest.fn(), @@ -45,10 +50,26 @@ jest.mock('@sentry/utils', () => { }); import { getCurrentHub } from '@sentry/core'; -import type { Event, EventHint, SeverityLevel } from '@sentry/types'; +import type { Event, EventHint, ExtendedError, Integration, SeverityLevel } from '@sentry/types'; import { ReactNativeErrorHandlers } from '../../src/js/integrations/reactnativeerrorhandlers'; +interface MockTrackingOptions { + allRejections: boolean; + onUnhandled: jest.Mock; + onHandled: jest.Mock; +} + +interface MockedReactNativeErrorHandlers extends Integration { + _loadRejectionTracking: jest.Mock< + { + disable: jest.Mock; + enable: jest.Mock; + }, + [] + >; +} + describe('ReactNativeErrorHandlers', () => { beforeEach(() => { ErrorUtils.getGlobalHandler = () => jest.fn(); @@ -103,6 +124,62 @@ describe('ReactNativeErrorHandlers', () => { expect(hint).toEqual(expect.objectContaining({ originalException: new Error('Test Error') })); }); }); + + describe('onUnhandledRejection', () => { + test('unhandled rejected promise is captured with synthetical error', async () => { + mockHubCaptureException.mockClear(); + const integration = new ReactNativeErrorHandlers(); + const mockDisable = jest.fn(); + const mockEnable = jest.fn(); + (integration as unknown as MockedReactNativeErrorHandlers)._loadRejectionTracking = jest.fn(() => ({ + disable: mockDisable, + enable: mockEnable, + })); + integration.setupOnce(); + + const [actualTrackingOptions] = mockEnable.mock.calls[0] || []; + actualTrackingOptions?.onUnhandled?.(1, 'Test Error'); + const actualSyntheticError = mockHubCaptureException.mock.calls[0][1].syntheticException; + + expect(mockDisable).not.toHaveBeenCalled(); + expect(mockEnable).toHaveBeenCalledWith( + expect.objectContaining({ + allRejections: true, + onUnhandled: expect.any(Function), + onHandled: expect.any(Function), + }), + ); + expect(mockEnable).toHaveBeenCalledTimes(1); + expect((actualSyntheticError as ExtendedError).framesToPop).toBe(3); + }); + + test('error like unhandled rejected promise is captured without synthetical error', async () => { + mockHubCaptureException.mockClear(); + const integration = new ReactNativeErrorHandlers(); + const mockDisable = jest.fn(); + const mockEnable = jest.fn(); + (integration as unknown as MockedReactNativeErrorHandlers)._loadRejectionTracking = jest.fn(() => ({ + disable: mockDisable, + enable: mockEnable, + })); + integration.setupOnce(); + + const [actualTrackingOptions] = mockEnable.mock.calls[0] || []; + actualTrackingOptions?.onUnhandled?.(1, new Error('Test Error')); + const actualSyntheticError = mockHubCaptureException.mock.calls[0][1].syntheticException; + + expect(mockDisable).not.toHaveBeenCalled(); + expect(mockEnable).toHaveBeenCalledWith( + expect.objectContaining({ + allRejections: true, + onUnhandled: expect.any(Function), + onHandled: expect.any(Function), + }), + ); + expect(mockEnable).toHaveBeenCalledTimes(1); + expect(actualSyntheticError).toBeUndefined(); + }); + }); }); function getActualCaptureEventArgs() {