add default early JS error handler (facebook#44884)

Summary: Pull Request resolved: facebook#44884 Removing JavaScript error handler supplied to ReactHostImpl.java which is just a stub and creating a default handler in ReactInstance.java which uses NativeExceptionHandler TurboModule to handle error. Changelog: [Android][BREAKING] Removing `ReactJsExceptionHandler` param from ReactHostImpl() constructor and providing a default private implementation Differential Revision: D58385767
alanleedev · Jun 13, 2024 · c365360 · c365360
1 parent b19bf2b
commit c365360
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 21 deletions.
diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api
@@ -3778,8 +3778,8 @@ public abstract class com/facebook/react/runtime/JSRuntimeFactory {
 }
 
 public class com/facebook/react/runtime/ReactHostImpl : com/facebook/react/ReactHost {
-	public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;Ljava/util/concurrent/Executor;Ljava/util/concurrent/Executor;Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;ZZ)V
-	public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;ZLcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;Z)V
+	public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;Ljava/util/concurrent/Executor;Ljava/util/concurrent/Executor;ZZ)V
+	public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;ZZ)V
 	public fun addBeforeDestroyListener (Lkotlin/jvm/functions/Function0;)V
 	public fun addReactInstanceEventListener (Lcom/facebook/react/ReactInstanceEventListener;)V
 	public fun createSurface (Landroid/content/Context;Ljava/lang/String;Landroid/os/Bundle;)Lcom/facebook/react/interfaces/fabric/ReactSurface;

diff --git a/...s/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHost.kt b/...s/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHost.kt
@@ -17,7 +17,6 @@ import com.facebook.react.bridge.ReactContext
 import com.facebook.react.common.annotations.UnstableReactNativeAPI
 import com.facebook.react.common.build.ReactBuildConfig
 import com.facebook.react.fabric.ComponentFactory
-import com.facebook.react.interfaces.exceptionmanager.ReactJsExceptionHandler
 import com.facebook.react.runtime.JSCInstance
 import com.facebook.react.runtime.ReactHostImpl
 import com.facebook.react.runtime.cxxreactpackage.CxxReactPackage
@@ -74,8 +73,6 @@ public object DefaultReactHost {
               reactPackages = packageList,
               jsRuntimeFactory = jsRuntimeFactory,
               turboModuleManagerDelegateBuilder = defaultTmmDelegateBuilder)
-      // TODO: T180971255 Improve default exception handler
-      val reactJsExceptionHandler = ReactJsExceptionHandler { _ -> }
       val componentFactory = ComponentFactory()
       DefaultComponentsRegistry.register(componentFactory)
       // TODO: T164788699 find alternative of accessing ReactHostImpl for initialising reactHost
@@ -85,7 +82,6 @@ public object DefaultReactHost {
                   defaultReactHostDelegate,
                   componentFactory,
                   true /* allowPackagerServerAccess */,
-                  reactJsExceptionHandler,
                   useDevSupport,
               )
               .apply {

diff --git a/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java
@@ -57,7 +57,6 @@
 import com.facebook.react.fabric.ComponentFactory;
 import com.facebook.react.fabric.FabricUIManager;
 import com.facebook.react.interfaces.TaskInterface;
-import com.facebook.react.interfaces.exceptionmanager.ReactJsExceptionHandler;
 import com.facebook.react.interfaces.fabric.ReactSurface;
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
 import com.facebook.react.modules.appearance.AppearanceModule;
@@ -102,7 +101,6 @@ public class ReactHostImpl implements ReactHost {
   private final Context mContext;
   private final ReactHostDelegate mReactHostDelegate;
   private final ComponentFactory mComponentFactory;
-  private final ReactJsExceptionHandler mReactJsExceptionHandler;
   private final DevSupportManager mDevSupportManager;
   private final Executor mBGExecutor;
   private final Executor mUIExecutor;
@@ -143,15 +141,13 @@ public ReactHostImpl(
       ReactHostDelegate delegate,
       ComponentFactory componentFactory,
       boolean allowPackagerServerAccess,
-      ReactJsExceptionHandler reactJsExceptionHandler,
       boolean useDevSupport) {
     this(
         context,
         delegate,
         componentFactory,
         Executors.newSingleThreadExecutor(),
         Task.UI_THREAD_EXECUTOR,
-        reactJsExceptionHandler,
         allowPackagerServerAccess,
         useDevSupport);
   }
@@ -162,15 +158,13 @@ public ReactHostImpl(
       ComponentFactory componentFactory,
       Executor bgExecutor,
       Executor uiExecutor,
-      ReactJsExceptionHandler reactJsExceptionHandler,
       boolean allowPackagerServerAccess,
       boolean useDevSupport) {
     mContext = context;
     mReactHostDelegate = delegate;
     mComponentFactory = componentFactory;
     mBGExecutor = bgExecutor;
     mUIExecutor = uiExecutor;
-    mReactJsExceptionHandler = reactJsExceptionHandler;
     mQueueThreadExceptionHandler = ReactHostImpl.this::handleHostException;
     mMemoryPressureRouter = new MemoryPressureRouter(context);
     mAllowPackagerServerAccess = allowPackagerServerAccess;
@@ -1067,7 +1061,6 @@ private Task<ReactInstance> getOrCreateReactInstanceTask() {
                             mComponentFactory,
                             devSupportManager,
                             mQueueThreadExceptionHandler,
-                            mReactJsExceptionHandler,
                             mUseDevSupport,
                             getOrCreateReactHostInspectorTarget());
 

diff --git a/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java b/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java
@@ -7,9 +7,16 @@
 
 package com.facebook.react.runtime;
 
+import static com.facebook.react.util.JSStackTrace.COLUMN_KEY;
+import static com.facebook.react.util.JSStackTrace.FILE_KEY;
+import static com.facebook.react.util.JSStackTrace.LINE_NUMBER_KEY;
+import static com.facebook.react.util.JSStackTrace.METHOD_NAME_KEY;
+
 import android.content.res.AssetManager;
 import android.view.View;
 import com.facebook.common.logging.FLog;
+import com.facebook.fbreact.specs.NativeExceptionsManagerSpec;
+import com.facebook.infer.annotation.Assertions;
 import com.facebook.infer.annotation.Nullsafe;
 import com.facebook.infer.annotation.ThreadConfined;
 import com.facebook.infer.annotation.ThreadSafe;
@@ -21,12 +28,15 @@
 import com.facebook.react.bridge.Arguments;
 import com.facebook.react.bridge.JSBundleLoader;
 import com.facebook.react.bridge.JSBundleLoaderDelegate;
+import com.facebook.react.bridge.JavaOnlyArray;
+import com.facebook.react.bridge.JavaOnlyMap;
 import com.facebook.react.bridge.JavaScriptContextHolder;
 import com.facebook.react.bridge.NativeArray;
 import com.facebook.react.bridge.NativeMap;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.ReactNoCrashSoftException;
 import com.facebook.react.bridge.ReactSoftExceptionLogger;
+import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.RuntimeExecutor;
 import com.facebook.react.bridge.RuntimeScheduler;
 import com.facebook.react.bridge.queue.MessageQueueThread;
@@ -110,7 +120,6 @@ final class ReactInstance {
       ComponentFactory componentFactory,
       DevSupportManager devSupportManager,
       QueueThreadExceptionHandler exceptionHandler,
-      ReactJsExceptionHandler reactExceptionManager,
       boolean useDevSupport,
       @Nullable ReactHostInspectorTarget reactHostInspectorTarget) {
     mBridgelessReactContext = bridgelessReactContext;
@@ -154,14 +163,15 @@ final class ReactInstance {
     // Notify JS if profiling is enabled
     boolean isProfiling =
         Systrace.isTracing(Systrace.TRACE_TAG_REACT_APPS | Systrace.TRACE_TAG_REACT_JS_VM_CALLS);
+
     mHybridData =
         initHybrid(
             jsRuntimeFactory,
             jsMessageQueueThread,
             nativeModulesMessageQueueThread,
             mJavaTimerManager,
             jsTimerExecutor,
-            reactExceptionManager,
+            new ReactJsExceptionHandlerImpl(nativeModulesMessageQueueThread),
             bindingsInstaller,
             isProfiling,
             reactHostInspectorTarget);
@@ -311,6 +321,44 @@ public ReactQueueConfiguration getReactQueueConfiguration() {
     return mQueueConfiguration;
   }
 
+  private class ReactJsExceptionHandlerImpl implements ReactJsExceptionHandler {
+    private final MessageQueueThread mNativemodulesmessagequeuethread;
+
+    ReactJsExceptionHandlerImpl(MessageQueueThread nativeModulesMessageQueueThread) {
+      this.mNativemodulesmessagequeuethread = nativeModulesMessageQueueThread;
+    }
+
+    @Override
+    public void reportJsException(ParsedError error) {
+      List<ReactJsExceptionHandler.ParsedError.StackFrame> frames = error.getFrames();
+      List<ReadableMap> readableMapList = new ArrayList<>();
+      for (ReactJsExceptionHandler.ParsedError.StackFrame frame : frames) {
+        JavaOnlyMap map = new JavaOnlyMap();
+        map.putDouble(COLUMN_KEY, frame.getColumnNumber());
+        map.putDouble(LINE_NUMBER_KEY, frame.getLineNumber());
+        map.putString(FILE_KEY, (String) frame.getFileName());
+        map.putString(METHOD_NAME_KEY, (String) frame.getMethodName());
+        readableMapList.add(map);
+      }
+
+      JavaOnlyMap data = new JavaOnlyMap();
+      data.putString("message", error.getMessage());
+      data.putArray("stack", JavaOnlyArray.from(readableMapList));
+      data.putInt("id", error.getExceptionId());
+      data.putBoolean("isFatal", error.isFatal());
+
+      // Simulate async native module method call
+      mNativemodulesmessagequeuethread.runOnQueue(
+          () -> {
+            NativeExceptionsManagerSpec exceptionsManager =
+                (NativeExceptionsManagerSpec)
+                    Assertions.assertNotNull(
+                        mTurboModuleManager.getModule(NativeExceptionsManagerSpec.NAME));
+            exceptionsManager.reportException(data);
+          });
+    }
+  }
+
   public void loadJSBundle(JSBundleLoader bundleLoader) {
     // Load the JS bundle
     Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstance.loadJSBundle");

diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostTest.kt
@@ -84,12 +84,7 @@ class ReactHostTest {
     Mockito.doReturn(jSBundleLoader).`when`(reactHostDelegate).jsBundleLoader
     reactHost =
         ReactHostImpl(
-            activityController.get().application,
-            reactHostDelegate,
-            componentFactory,
-            false,
-            {},
-            false)
+            activityController.get().application, reactHostDelegate, componentFactory, false, false)
     val taskCompletionSource = TaskCompletionSource<Boolean>().apply { setResult(true) }
     mockedTaskCompletionSourceCtor =
         Mockito.mockConstruction(