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

krystofwoldrich · web-flow · commit df12f4572760 · 2023-12-01T15:47:04.000Z
diff --git 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
@@ -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<SentryStackFrame[] | null> {
+  private async _symbolicate(rawStack: string, skipFirstFrames: number = 0): Promise<SentryStackFrame[] | null> {
     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
@@ -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,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) => {
@@ -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
@@ -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
@@ -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
@@ -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> {
diff --git a/test/integrations/reactnativeerrorhandlers.test.ts b/test/integrations/reactnativeerrorhandlers.test.ts
