Skip to content

fix(promise-tracking): Remove Sentry frames from synthetic errors #3423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
37 changes: 20 additions & 17 deletions src/js/integrations/debugsymbolicator.ts
Original file line number Diff line number Diff line change
@@ -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('|'));
Expand Down Expand Up @@ -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);
Expand All @@ -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<SentryStackFrame[] | null> {
private async _symbolicate(rawStack: string, skipFirstFrames: number = 0): Promise<SentryStackFrame[] | null> {
const parsedStack = this._parseErrorStack(rawStack);

try {
Expand All @@ -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,
);

Expand Down
21 changes: 15 additions & 6 deletions src/js/integrations/reactnativeerrorhandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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 = {}) => {
Expand All @@ -123,14 +120,15 @@ 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);
}

getCurrentHub().captureException(error, {
data: { id },
originalException: error,
syntheticException: isErrorLike(error) ? undefined : createSyntheticError(),
});
},
onHandled: (id: string) => {
Expand Down Expand Up @@ -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');
}
}
38 changes: 38 additions & 0 deletions src/js/utils/error.ts
Original file line number Diff line number Diff line change
@@ -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'
);
}
25 changes: 25 additions & 0 deletions test/error.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
92 changes: 92 additions & 0 deletions test/integrations/debugsymbolicator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Event>{
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(<Event>{
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<Event | null> {
Expand Down
Loading