fix: Linked errors processed before other integrations

CHANGELOG.md

@@ -28,6 +28,8 @@
 - Performance Tracing should be disabled by default ([#3533](https://github.com/getsentry/sentry-react-native/pull/3533))
 - Use `$NODE_BINARY` to execute Sentry CLI in Xcode scripts ([#3493](https://github.com/getsentry/sentry-react-native/pull/3493))
 - Return auto Release and Dist to source maps auto upload ([#3540](https://github.com/getsentry/sentry-react-native/pull/3540))
+- Linked errors processed before other integrations ([#3535](https://github.com/getsentry/sentry-react-native/pull/3535))
+  - This ensure their frames are correctly symbolicated
 
 ### Dependencies
 

android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

@@ -705,8 +705,8 @@ public void fetchNativeSdkInfo(Promise promise) {
         }
     }
 
-    public void fetchNativePackageName(Promise promise) {
-        promise.resolve(packageInfo.packageName);
+    public String fetchNativePackageName() {
+        return packageInfo.packageName;
     }
 
     private void setEventOriginTag(SentryEvent event) {

android/src/newarch/java/io/sentry/react/RNSentryModule.java

@@ -135,12 +135,13 @@ public WritableMap stopProfiling() {
     }
 
     @Override
-    public void fetchNativePackageName(Promise promise) {
-        this.impl.fetchNativePackageName(promise);
+    public String fetchNativePackageName() {
+        return this.impl.fetchNativePackageName();
     }
 
     @Override
-    public void fetchNativeStackFramesBy(ReadableArray instructionsAddr, Promise promise) {
+    public WritableMap fetchNativeStackFramesBy(ReadableArray instructionsAddr) {
         // Not used on Android
+        return null;
     }
 }

android/src/oldarch/java/io/sentry/react/RNSentryModule.java

@@ -133,13 +133,14 @@ public WritableMap stopProfiling() {
         return this.impl.stopProfiling();
     }
 
-    @ReactMethod
-    public void fetchNativePackageName(Promise promise) {
-        this.impl.fetchNativePackageName(promise);
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    public String fetchNativePackageName() {
+        return this.impl.fetchNativePackageName();
     }
 
-    @ReactMethod
-    public void fetchNativeStackFramesBy(ReadableArray instructionsAddr, Promise promise) {
+    @ReactMethod(isBlockingSynchronousMethod = true)
+    public WritableMap fetchNativeStackFramesBy(ReadableArray instructionsAddr) {
         // Not used on Android
+        return null;
     }
 }

ios/RNSentry.mm

@@ -201,11 +201,10 @@ - (void)setEventEnvironmentTag:(SentryEvent *)event
     resolve(modulesString);
 }
 
-RCT_EXPORT_METHOD(fetchNativePackageName:(RCTPromiseResolveBlock)resolve
-                  rejecter:(RCTPromiseRejectBlock)reject)
+RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSString *, fetchNativePackageName)
 {
     NSString *packageName = [[NSBundle mainBundle] executablePath];
-    resolve(packageName);
+    return packageName;
 }
 
 - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAddr
@@ -273,12 +272,10 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAdd
   }
 }
 
-RCT_EXPORT_METHOD(fetchNativeStackFramesBy:(NSArray *)instructionsAddr
-                  resolve:(RCTPromiseResolveBlock)resolve
-                  reject:(RCTPromiseRejectBlock)reject)
+RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, fetchNativeStackFramesBy:(NSArray *)instructionsAddr)
 {
-  resolve([self fetchNativeStackFramesBy:instructionsAddr
-                             symbolicate:dladdr]);
+  return [self fetchNativeStackFramesBy:instructionsAddr
+                             symbolicate:dladdr];
 }
 
 RCT_EXPORT_METHOD(fetchNativeDeviceContexts:(RCTPromiseResolveBlock)resolve

jest.config.js

@@ -1,7 +1,7 @@
 module.exports = {
   collectCoverage: true,
   preset: 'react-native',
-  setupFilesAfterEnv: ['<rootDir>/test/mockConsole.ts'],
+  setupFilesAfterEnv: ['jest-extended/all', '<rootDir>/test/mockConsole.ts'],
   globals: {
     __DEV__: true,
     'ts-jest': {

package.json

@@ -96,6 +96,7 @@
     "expo-module-scripts": "^3.1.0",
     "jest": "^29.6.2",
     "jest-environment-jsdom": "^29.6.2",
+    "jest-extended": "^4.0.2",
     "madge": "^6.1.0",
     "metro": "0.76",
     "prettier": "^2.0.5",

src/js/NativeRNSentry.ts

@@ -34,8 +34,8 @@ export interface Spec extends TurboModule {
   fetchViewHierarchy(): Promise<number[] | undefined | null>;
   startProfiling(): { started?: boolean; error?: string };
   stopProfiling(): { profile?: string; nativeProfile?: UnsafeObject; error?: string };
-  fetchNativePackageName(): Promise<string | undefined | null>;
-  fetchNativeStackFramesBy(instructionsAddr: number[]): Promise<NativeStackFrames | undefined | null>;
+  fetchNativePackageName(): string | undefined | null;
+  fetchNativeStackFramesBy(instructionsAddr: number[]): NativeStackFrames | undefined | null;
 }
 
 export type NativeStackFrame = {

src/js/integrations/nativelinkederrors.ts

@@ -1,5 +1,6 @@
 import { exceptionFromError } from '@sentry/browser';
 import type {
+  Client,
   DebugImage,
   Event,
   EventHint,
@@ -53,35 +54,29 @@ export class NativeLinkedErrors implements Integration {
   /**
    * @inheritDoc
    */
-  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
-    const client = getCurrentHub().getClient();
-    if (!client) {
-      return;
+  public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void {
+    /* noop */
+  }
+
+  /**
+   * @inheritDoc
+   */
+  public preprocessEvent(event: Event, hint: EventHint | undefined, client: Client): void {
+    if (this._nativePackage === null) {
+      this._nativePackage = this._fetchNativePackage();
     }
 
-    addGlobalEventProcessor(async (event: Event, hint?: EventHint) => {
-      if (this._nativePackage === null) {
-        this._nativePackage = await this._fetchNativePackage();
-      }
-      const self = getCurrentHub().getIntegration(NativeLinkedErrors);
-      return self ? this._handler(client.getOptions().stackParser, self._key, self._limit, event, hint) : event;
-    });
+    this._handler(client.getOptions().stackParser, this._key, this._limit, event, hint);
   }
 
   /**
    * Enriches passed event with linked exceptions and native debug meta images.
    */
-  private async _handler(
-    parser: StackParser,
-    key: string,
-    limit: number,
-    event: Event,
-    hint?: EventHint,
-  ): Promise<Event | null> {
+  private _handler(parser: StackParser, key: string, limit: number, event: Event, hint?: EventHint): void {
     if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) {
-      return event;
+      return;
     }
-    const { exceptions: linkedErrors, debugImages } = await this._walkErrorTree(
+    const { exceptions: linkedErrors, debugImages } = this._walkErrorTree(
       parser,
       limit,
       hint.originalException as ExtendedError,
@@ -92,25 +87,23 @@ export class NativeLinkedErrors implements Integration {
     event.debug_meta = event.debug_meta || {};
     event.debug_meta.images = event.debug_meta.images || [];
     event.debug_meta.images.push(...(debugImages || []));
-
-    return event;
   }
 
   /**
    * Walks linked errors and created Sentry exceptions chain.
    * Collects debug images from native errors stack frames.
    */
-  private async _walkErrorTree(
+  private _walkErrorTree(
     parser: StackParser,
     limit: number,
     error: ExtendedError,
     key: string,
     exceptions: Exception[] = [],
     debugImages: DebugImage[] = [],
-  ): Promise<{
+  ): {
     exceptions: Exception[];
     debugImages?: DebugImage[];
-  }> {
+  } {
     const linkedError = error[key];
     if (!linkedError || exceptions.length + 1 >= limit) {
       return {
@@ -126,7 +119,7 @@ export class NativeLinkedErrors implements Integration {
       exception = this._exceptionFromJavaStackElements(linkedError);
     } else if ('stackReturnAddresses' in linkedError) {
       // isObjCException
-      const { appleException, appleDebugImages } = await this._exceptionFromAppleStackReturnAddresses(linkedError);
+      const { appleException, appleDebugImages } = this._exceptionFromAppleStackReturnAddresses(linkedError);
       exception = appleException;
       exceptionDebugImages = appleDebugImages;
     } else if (isInstanceOf(linkedError, Error)) {
@@ -193,15 +186,15 @@ export class NativeLinkedErrors implements Integration {
   /**
    * Converts StackAddresses to a SentryException with DebugMetaImages
    */
-  private async _exceptionFromAppleStackReturnAddresses(objCException: {
+  private _exceptionFromAppleStackReturnAddresses(objCException: {
     name: string;
     message: string;
     stackReturnAddresses: number[];
-  }): Promise<{
+  }): {
     appleException: Exception;
     appleDebugImages: DebugImage[];
-  }> {
-    const nativeStackFrames = await this._fetchNativeStackFrames(objCException.stackReturnAddresses);
+  } {
+    const nativeStackFrames = this._fetchNativeStackFrames(objCException.stackReturnAddresses);
 
     return {
       appleException: {
@@ -218,14 +211,14 @@ export class NativeLinkedErrors implements Integration {
   /**
    * Fetches the native package/image name from the native layer
    */
-  private _fetchNativePackage(): Promise<string | null> {
+  private _fetchNativePackage(): string | null {
     return NATIVE.fetchNativePackageName();
   }
 
   /**
    * Fetches native debug image information on iOS
    */
-  private _fetchNativeStackFrames(instructionsAddr: number[]): Promise<NativeStackFrames | null> {
+  private _fetchNativeStackFrames(instructionsAddr: number[]): NativeStackFrames | null {
     return NATIVE.fetchNativeStackFramesBy(instructionsAddr);
   }
 }

src/js/wrapper.ts

@@ -85,12 +85,12 @@ interface SentryNativeWrapper {
   startProfiling(): boolean;
   stopProfiling(): { hermesProfile: Hermes.Profile; nativeProfile?: NativeProfileEvent } | null;
 
-  fetchNativePackageName(): Promise<string | null>;
+  fetchNativePackageName(): string | null;
 
   /**
    * Fetches native stack frames and debug images for the instructions addresses.
    */
-  fetchNativeStackFramesBy(instructionsAddr: number[]): Promise<NativeStackFrames | null>;
+  fetchNativeStackFramesBy(instructionsAddr: number[]): NativeStackFrames | null;
 }
 
 const EOL = utf8ToBytes('\n');
@@ -554,26 +554,26 @@ export const NATIVE: SentryNativeWrapper = {
     }
   },
 
-  async fetchNativePackageName(): Promise<string | null> {
+  fetchNativePackageName(): string | null {
     if (!this.enableNative) {
       return null;
     }
     if (!this._isModuleLoaded(RNSentry)) {
       return null;
     }
 
-    return (await RNSentry.fetchNativePackageName()) || null;
+    return RNSentry.fetchNativePackageName() || null;
   },
 
-  async fetchNativeStackFramesBy(instructionsAddr: number[]): Promise<NativeStackFrames | null> {
+  fetchNativeStackFramesBy(instructionsAddr: number[]): NativeStackFrames | null {
     if (!this.enableNative) {
       return null;
     }
     if (!this._isModuleLoaded(RNSentry)) {
       return null;
     }
 
-    return (await RNSentry.fetchNativeStackFramesBy(instructionsAddr)) || null;
+    return RNSentry.fetchNativeStackFramesBy(instructionsAddr) || null;
   },
 
   /**

test/integrations/integrationsexecutionorder.test.ts

@@ -0,0 +1,98 @@
+import * as mockWrapper from '../mockWrapper';
+jest.mock('../../src/js/wrapper', () => mockWrapper);
+jest.mock('../../src/js/utils/environment');
+
+import { defaultStackParser } from '@sentry/browser';
+import type { Integration } from '@sentry/types';
+
+import { ReactNativeClient } from '../../src/js/client';
+import { getDefaultIntegrations } from '../../src/js/integrations/default';
+import type { ReactNativeClientOptions } from '../../src/js/options';
+import { isHermesEnabled, notWeb } from '../../src/js/utils/environment';
+import { MOCK_DSN } from '../mockDsn';
+
+describe('Integration execution order', () => {
+  describe('mobile hermes', () => {
+    beforeEach(() => {
+      (notWeb as jest.Mock).mockReturnValue(true);
+      (isHermesEnabled as jest.Mock).mockReturnValue(true);
+    });
+
+    it('NativeLinkedErrors is before RewriteFrames', async () => {
+      // NativeLinkedErrors has to process event before RewriteFrames
+      // otherwise the linked errors stack trace frames won't be rewritten
+
+      const client = createTestClient();
+      const { integrations } = client.getOptions();
+
+      const nativeLinkedErrors = spyOnIntegrationById('NativeLinkedErrors', integrations);
+      const rewriteFrames = spyOnIntegrationById('RewriteFrames', integrations);
+
+      client.setupIntegrations();
+
+      client.captureException(new Error('test'));
+      jest.runAllTimers();
+
+      expect(nativeLinkedErrors.preprocessEvent).toHaveBeenCalledBefore(rewriteFrames.processEvent!);
+    });
+  });
+
+  describe('web', () => {
+    beforeEach(() => {
+      (notWeb as jest.Mock).mockReturnValue(false);
+      (isHermesEnabled as jest.Mock).mockReturnValue(false);
+    });
+
+    it('LinkedErrors is before RewriteFrames', async () => {
+      // LinkedErrors has to process event before RewriteFrames
+      // otherwise the linked errors stack trace frames won't be rewritten
+
+      const client = createTestClient();
+      const { integrations } = client.getOptions();
+
+      const linkedErrors = spyOnIntegrationById('LinkedErrors', integrations);
+      const rewriteFrames = spyOnIntegrationById('RewriteFrames', integrations);
+
+      client.setupIntegrations();
+
+      client.captureException(new Error('test'));
+      jest.runAllTimers();
+
+      expect(linkedErrors.preprocessEvent).toHaveBeenCalledBefore(rewriteFrames.processEvent!);
+    });
+  });
+});
+
+interface IntegrationSpy {
+  name: string;
+  setupOnce?: Integration['setupOnce'] & jest.Mock;
+  preprocessEvent?: Integration['preprocessEvent'] & jest.Mock;
+  processEvent?: Integration['processEvent'] & jest.Mock;
+}
+
+function spyOnIntegrationById(id: string, integrations: Integration[]): IntegrationSpy {
+  const candidate = integrations?.find(integration => integration.name === id);
+  if (!candidate) {
+    throw new Error(`Integration ${id} not found`);
+  }
+
+  jest.spyOn(candidate, 'setupOnce');
+  candidate.preprocessEvent && jest.spyOn(candidate, 'preprocessEvent');
+  candidate.processEvent && jest.spyOn(candidate, 'processEvent');
+  return candidate as IntegrationSpy;
+}
+
+function createTestClient(): ReactNativeClient {
+  const clientOptions: ReactNativeClientOptions = {
+    dsn: MOCK_DSN,
+    transport: () => ({
+      send: jest.fn().mockResolvedValue(undefined),
+      flush: jest.fn().mockResolvedValue(true),
+    }),
+    stackParser: defaultStackParser,
+    integrations: [],
+  };
+  clientOptions.integrations = getDefaultIntegrations(clientOptions);
+
+  return new ReactNativeClient(clientOptions);
+}