fix(react-native): call native client synchronously when turbo module…

… is enabled
joycesmartbear · Jun 10, 2024 · 115eeff · 115eeff
1 parent 334ca8c
commit 115eeff
Show file tree

Hide file tree

Showing 12 changed files with 269 additions and 73 deletions.
diff --git a/packages/delivery-react-native/delivery.js b/packages/delivery-react-native/delivery.js
@@ -11,7 +11,10 @@ module.exports = (client, NativeClient) => ({
         nativeStack = event.originalError.nativeStackAndroid
       }
     }
-    NativeClient.dispatch({
+
+    const isTurboModuleEnabled = global.__turboModuleProxy != null
+
+    const eventPayload = {
       errors: event.errors,
       severity: event.severity,
       severityReason: event._handledState.severityReason,
@@ -27,7 +30,14 @@ module.exports = (client, NativeClient) => ({
       apiKey: event.apiKey,
       featureFlags: event.toJSON().featureFlags,
       nativeStack: nativeStack
-    }).then(() => cb()).catch(cb)
+    }
+
+    if (isTurboModuleEnabled) {
+      NativeClient.dispatch(eventPayload)
+      cb()
+    } else {
+      NativeClient.dispatchAsync(eventPayload).then(() => cb()).catch(cb)
+    }
   },
   sendSession: () => {
     client._logger.warn('@bugsnag/delivery-react-native sendSession() should never be called', new Error().stack)

diff --git a/packages/delivery-react-native/test/delivery.test.ts b/packages/delivery-react-native/test/delivery.test.ts
@@ -1,5 +1,3 @@
-/* eslint-disable jest/no-disabled-tests */
-
 import Client from '@bugsnag/core/client'
 import delivery from '../'
 import EventWithInternals from '@bugsnag/core/event'
@@ -37,10 +35,10 @@ class ReactNativeError extends Error {
 }
 
 describe('delivery: react native', () => {
-  it('sends the correct payload using the native client’s dispatch() method', done => {
+  it('sends the correct payload using the native client’s dispatchAsync() method', done => {
     const sent: NativeClientEvent[] = []
     const NativeClient = {
-      dispatch: (event: NativeClientEvent) => {
+      dispatchAsync: (event: NativeClientEvent) => {
         sent.push(event)
         return new Promise((resolve) => resolve(true))
       }
@@ -86,10 +84,10 @@ describe('delivery: react native', () => {
     })
   })
 
-  it.skip('extracts nativeStackIOS', done => {
+  it('extracts nativeStackIOS', done => {
     const sent: NativeClientEvent[] = []
     const NativeClient = {
-      dispatch: (event: NativeClientEvent) => {
+      dispatchAsync: (event: NativeClientEvent) => {
         sent.push(event)
         return new Promise((resolve) => resolve(true))
       }
@@ -114,10 +112,10 @@ describe('delivery: react native', () => {
     })
   })
 
-  it.skip('extracts nativeStackAndroid', done => {
+  it('extracts nativeStackAndroid', done => {
     const sent: NativeClientEvent[] = []
     const NativeClient = {
-      dispatch: (event: NativeClientEvent) => {
+      dispatchAsync: (event: NativeClientEvent) => {
         sent.push(event)
         return new Promise((resolve) => resolve(true))
       }
@@ -139,4 +137,56 @@ describe('delivery: react native', () => {
       done()
     })
   })
+
+  it('uses the synchronous dispatch() method when Turbo Modules are enabled', done => {
+    // @ts-expect-error __turboModuleProxy does not exist on type 'Global'
+    global.__turboModuleProxy = {}
+
+    const sent: NativeClientEvent[] = []
+    const NativeClient = {
+      dispatch: (event: NativeClientEvent) => {
+        sent.push(event)
+        return true
+      }
+    }
+    const c = new Client({ apiKey: 'api_key' })
+
+    const metaData: any = {
+      from: 'javascript'
+    }
+
+    // ensure that circular references in metadata are safely handled
+    metaData.circle = metaData
+
+    c._setDelivery(client => delivery(client, NativeClient))
+    c.leaveBreadcrumb('hi', metaData, 'state')
+    c.setContext('test screen')
+    c.setUser('123')
+    c.notify(new Error('oh no'), (e) => {
+      e.groupingHash = 'ER_GRP_098'
+      e.apiKey = 'abcdef123456abcdef123456abcdef123456'
+    }, (err, event) => {
+      expect(err).not.toBeTruthy()
+      expect(sent.length).toBe(1)
+      expect(sent[0].errors[0].errorMessage).toBe('oh no')
+      expect(sent[0].severity).toBe('warning')
+      expect(sent[0].severityReason.type).toBe('handledException')
+      expect(sent[0].unhandled).toBe(false)
+      expect(sent[0].app).toEqual({ releaseStage: 'production', version: undefined, type: undefined })
+      expect(sent[0].device).toEqual({})
+      expect(sent[0].threads).toEqual([])
+      expect(sent[0].breadcrumbs.length).toBe(1)
+      expect(sent[0].breadcrumbs[0].message).toBe('hi')
+      expect(sent[0].breadcrumbs[0].metadata).toStrictEqual({
+        from: 'javascript',
+        circle: '[Circular]'
+      })
+      expect(sent[0].context).toBe('test screen')
+      expect(sent[0].user).toEqual({ id: '123', email: undefined, name: undefined })
+      expect(sent[0].metadata).toEqual({})
+      expect(sent[0].groupingHash).toEqual('ER_GRP_098')
+      expect(sent[0].apiKey).toBe('abcdef123456abcdef123456abcdef123456')
+      done()
+    })
+  })
 })
diff --git a/packages/plugin-react-native-event-sync/event-sync.js b/packages/plugin-react-native-event-sync/event-sync.js
@@ -1,21 +1,35 @@
 module.exports = (NativeClient) => ({
   load: (client) => {
-    client.addOnError(async event => {
-      const {
-        threads,
-        breadcrumbs,
-        app,
-        device,
-        deviceMetadata,
-        appMetadata
-      } = await NativeClient.getPayloadInfo({ unhandled: event.unhandled })
+    const isTurboModuleEnabled = global.__turboModuleProxy != null
 
-      event.breadcrumbs = breadcrumbs
-      event.app = { ...event.app, ...app }
-      event.device = { ...event.device, ...device }
-      event.threads = threads
-      event.addMetadata('device', deviceMetadata)
-      event.addMetadata('app', appMetadata)
-    }, true)
+    if (isTurboModuleEnabled) {
+      client.addOnError(event => {
+        const payloadInfo = NativeClient.getPayloadInfo({ unhandled: event.unhandled })
+        syncEvent(payloadInfo, event)
+      }, true)
+    } else {
+      client.addOnError(async event => {
+        const payloadInfo = await NativeClient.getPayloadInfoAsync({ unhandled: event.unhandled })
+        syncEvent(payloadInfo, event)
+      }, true)
+    }
   }
 })
+
+const syncEvent = (payloadInfo, event) => {
+  const {
+    threads,
+    breadcrumbs,
+    app,
+    device,
+    deviceMetadata,
+    appMetadata
+  } = payloadInfo
+
+  event.breadcrumbs = breadcrumbs
+  event.app = { ...event.app, ...app }
+  event.device = { ...event.device, ...device }
+  event.threads = threads
+  event.addMetadata('device', deviceMetadata)
+  event.addMetadata('app', appMetadata)
+}
diff --git a/packages/plugin-react-native-event-sync/test/event-sync.test.ts b/packages/plugin-react-native-event-sync/test/event-sync.test.ts
@@ -8,7 +8,54 @@ describe('plugin: react native event sync', () => {
       apiKey: 'api_key',
       plugins: [
         plugin({
-          getPayloadInfo: async () => {
+          getPayloadInfoAsync: async () => {
+            return {
+              device: { osName: 'android', manufacturer: 'google' },
+              app: { versionCode: '1.0' },
+              breadcrumbs: [
+                {
+                  message: 'Test',
+                  type: 'manual',
+                  metadata: { meta: 'data' },
+                  timestamp: ts.toISOString()
+                }
+              ],
+              threads: [
+                {
+                  id: '123',
+                  name: 'main',
+                  errorReportingThread: false,
+                  type: 'android',
+                  stacktrace: []
+                }
+              ]
+            }
+          }
+        })
+      ]
+    })
+
+    c.notify(new Error('blah'), (event) => {
+      expect(event.device.osName).toBe('android')
+      expect(event.device.manufacturer).toBe('google')
+      expect(event.app.versionCode).toBe('1.0')
+      expect(event.threads.length).toBe(1)
+      expect(event.threads[0].name).toBe('main')
+      expect(event.breadcrumbs.length).toBe(1)
+      expect(event.breadcrumbs[0].timestamp).toBe(ts.toISOString())
+      done()
+    })
+  })
+
+  it('uses the synchronous getPayloadInfo method when Turbo Modules are enabled', done => {
+    // @ts-expect-error __turboModuleProxy does not exist on type 'Global'
+    global.__turboModuleProxy = {}
+    const ts = new Date()
+    const c = new Client({
+      apiKey: 'api_key',
+      plugins: [
+        plugin({
+          getPayloadInfo: () => {
             return {
               device: { osName: 'android', manufacturer: 'google' },
               app: { versionCode: '1.0' },

diff --git a/packages/react-native/android/src/main/java/com/bugsnag/android/BugsnagReactNativeImpl.java b/packages/react-native/android/src/main/java/com/bugsnag/android/BugsnagReactNativeImpl.java
@@ -185,27 +185,35 @@ void updateUser(@Nullable String id, @Nullable String email, @Nullable String na
     }
   }
 
-  void dispatch(@NonNull ReadableMap payload, @NonNull Promise promise) {
+  boolean dispatch(@NonNull ReadableMap payload) {
     try {
       plugin.dispatch(payload.toHashMap());
-      promise.resolve(true);
+      return true;
     } catch (Throwable exc) {
       logFailure("dispatch", exc);
-      promise.resolve(false);
+      return false;
     }
   }
 
-  void getPayloadInfo(@NonNull ReadableMap payload, @NonNull Promise promise) {
+  void dispatchAsync(@NonNull ReadableMap payload, @NonNull Promise promise) {
+    promise.resolve(dispatch(payload));
+  }
+
+  WritableMap getPayloadInfo(@NonNull ReadableMap payload) {
     try {
       boolean unhandled = payload.getBoolean("unhandled");
       Map<String, Object> info = plugin.getPayloadInfo(unhandled);
-      promise.resolve(ReactNativeCompat.toWritableMap(info));
+      return ReactNativeCompat.toWritableMap(info);
     } catch (Throwable exc) {
       logFailure("dispatch", exc);
-      promise.resolve(null);
+      return new WritableNativeMap();
     }
   }
 
+  void getPayloadInfoAsync(@NonNull ReadableMap payload, @NonNull Promise promise) {
+    promise.resolve(getPayloadInfo(payload));
+  }
+
   void addFeatureFlag(@NonNull String name, @Nullable String variant) {
     try {
       plugin.addFeatureFlag(name, variant);

diff --git a/packages/react-native/android/src/newarch/java/com/bugsnag/android/NativeBugsnagImpl.java b/packages/react-native/android/src/newarch/java/com/bugsnag/android/NativeBugsnagImpl.java
@@ -87,13 +87,23 @@ public void updateUser(@Nullable String id, @Nullable String email, @Nullable St
   }
 
   @Override
-  public void dispatch(ReadableMap payload, Promise promise) {
-    impl.dispatch(payload, promise);
+  public boolean dispatch(ReadableMap payload) {
+    return impl.dispatch(payload);
   }
 
   @Override
-  public void getPayloadInfo(ReadableMap payload, Promise promise) {
-    impl.getPayloadInfo(payload, promise);
+  public void dispatchAsync(ReadableMap payload, Promise promise) {
+    impl.dispatchAsync(payload, promise);
+  }
+
+  @Override
+  public WritableMap getPayloadInfo(ReadableMap payload) {
+    return impl.getPayloadInfo(payload);
+  }
+
+  @Override
+  public void getPayloadInfoAsync(ReadableMap payload, Promise promise) {
+    impl.getPayloadInfoAsync(payload, promise);
   }
 
   @Override

diff --git a/packages/react-native/android/src/oldarch/java/com/bugsnag/android/BugsnagReactNative.java b/packages/react-native/android/src/oldarch/java/com/bugsnag/android/BugsnagReactNative.java
@@ -93,13 +93,23 @@ void updateUser(@Nullable String id, @Nullable String email, @Nullable String na
   }
 
   @ReactMethod
-  void dispatch(@NonNull ReadableMap payload, @NonNull Promise promise) {
-    impl.dispatch(payload, promise);
+  boolean dispatch(@NonNull ReadableMap payload) {
+    return impl.dispatch(payload);
   }
 
   @ReactMethod
-  void getPayloadInfo(@NonNull ReadableMap payload, @NonNull Promise promise) {
-    impl.getPayloadInfo(payload, promise);
+  void dispatchAsync(@NonNull ReadableMap payload, @NonNull Promise promise) {
+    impl.dispatchAsync(payload, promise);
+  }
+
+  @ReactMethod
+  WritableMap getPayloadInfo(@NonNull ReadableMap payload) {
+    return impl.getPayloadInfo(payload);
+  }
+
+  @ReactMethod
+  void getPayloadInfoAsync(@NonNull ReadableMap payload, @NonNull Promise promise) {
+    impl.getPayloadInfoAsync(payload, promise);
   }
 
   @ReactMethod

diff --git a/packages/react-native/ios/BugsnagReactNative/BugsnagReactNative.h b/packages/react-native/ios/BugsnagReactNative/BugsnagReactNative.h
@@ -44,11 +44,15 @@
 - (void)clearFeatureFlag:(NSString *)name;
 - (void)clearFeatureFlags;
 
-- (void)dispatch:(NSDictionary *)payload
+- (NSNumber *)dispatch:(NSDictionary *)payload;
+
+- (void)dispatchAsync:(NSDictionary *)payload
          resolve:(RCTPromiseResolveBlock)resolve
           reject:(RCTPromiseRejectBlock)reject;
 
-- (void)getPayloadInfo:(NSDictionary *)payloadInfo
+- (NSDictionary *)getPayloadInfo:(NSDictionary *)payloadInfo;
+
+- (void)getPayloadInfoAsync:(NSDictionary *)payloadInfo
                resolve:(RCTPromiseResolveBlock)resolve
                 reject:(RCTPromiseRejectBlock)reject;