Do not suppress IllegalArgumentExceptions during TurboModule creation (…

…#41509)

Summary:
Pull Request resolved: #41509

We currently ignore `IllegalArgumentException` being thrown from TurboModule getters, as it was deemed acceptable to use that to signal a Package didn't have that module. This however can mask legitimate errors thrown during a TurboModule constructor.

TurboReactPackage#getModule is already marked as allowing nullable returns, so let's leave the use of exceptions for exceptional scenarios.

Changelog: [Android][Changed] Use null to signal a missing TurboModule instead of IllegalArgumentException.

Reviewed By: RSNara

Differential Revision: D51395165

fbshipit-source-id: 1eea1db6c7e3313a36d24e7837b36a3d0fccc718

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/DebugCorePackage.java

@@ -43,13 +43,12 @@ class DebugCorePackage extends TurboReactPackage implements ViewManagerOnDemandR
   public DebugCorePackage() {}
 
   @Override
-  public NativeModule getModule(String name, ReactApplicationContext reactContext) {
+  public @Nullable NativeModule getModule(String name, ReactApplicationContext reactContext) {
     switch (name) {
       case JSCHeapCapture.NAME:
         return new JSCHeapCapture(reactContext);
       default:
-        throw new IllegalArgumentException(
-            "In DebugCorePackage, could not find Native module for " + name);
+        return null;
     }
   }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java

@@ -8,11 +8,13 @@
 package com.facebook.react;
 
 import androidx.annotation.Nullable;
+import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.CxxModuleWrapper;
 import com.facebook.react.bridge.ModuleSpec;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.ReactApplicationContext;
+import com.facebook.react.common.ReactConstants;
 import com.facebook.react.config.ReactFeatureFlags;
 import com.facebook.react.internal.turbomodule.core.TurboModuleManagerDelegate;
 import com.facebook.react.internal.turbomodule.core.interfaces.TurboModule;
@@ -187,11 +189,12 @@ public TurboModule getModule(String moduleName) {
               return nativeModule;
             }
           } catch (IllegalArgumentException ex) {
-            /*
-             TurboReactPackages can throw an IllegalArgumentException when a module isn't found. If
-             this happens, it's safe to ignore the exception because a later TurboReactPackage could
-             provide the module.
-            */
+            // TODO T170570617: remove this catch statement and let exception bubble up
+            FLog.e(
+                ReactConstants.TAG,
+                ex,
+                "Caught exception while constructing module '%s'. This was previously ignored but will not be caught in the future.",
+                moduleName);
           }
         } else {
           throw new IllegalArgumentException(
@@ -217,11 +220,12 @@ public TurboModule getModule(String moduleName) {
         }
 
       } catch (IllegalArgumentException ex) {
-        /*
-         TurboReactPackages can throw an IllegalArgumentException when a module isn't found. If
-         this happens, it's safe to ignore the exception because a later TurboReactPackage could
-         provide the module.
-        */
+        // TODO T170570617: remove this catch statement and let exception bubble up
+        FLog.e(
+            ReactConstants.TAG,
+            ex,
+            "Caught exception while constructing module '%s'. This was previously ignored but will not be caught in the future.",
+            moduleName);
       }
     }
 
@@ -294,13 +298,13 @@ public NativeModule getLegacyModule(String moduleName) {
             resolvedModule = module;
           }
         }
-
       } catch (IllegalArgumentException ex) {
-        /*
-         * TurboReactPackages can throw an IllegalArgumentException when a module isn't found. If
-         * this happens, it's safe to ignore the exception because a later TurboReactPackage could
-         * provide the module.
-         */
+        // TODO T170570617: remove this catch statement and let exception bubble up
+        FLog.e(
+            ReactConstants.TAG,
+            ex,
+            "Caught exception while constructing module '%s'. This was previously ignored but will not be caught in the future.",
+            moduleName);
       }
     }
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java

@@ -19,6 +19,7 @@
 import com.facebook.debug.tags.ReactDebugOverlayTags;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.proguard.annotations.DoNotStrip;
+import com.facebook.react.common.ReactConstants;
 import com.facebook.react.internal.turbomodule.core.interfaces.TurboModule;
 import com.facebook.react.module.model.ReactModuleInfo;
 import com.facebook.systrace.SystraceMessage;
@@ -204,7 +205,7 @@ private NativeModule create() {
        *
        * @todo(T53311351)
        */
-      FLog.e("NativeModuleInitError", "Failed to create NativeModule \"" + getName() + "\"", ex);
+      FLog.e(ReactConstants.TAG, ex, "Failed to create NativeModule '%s'", mName);
       throw ex;
     } finally {
       ReactMarker.logMarker(CREATE_MODULE_END, mName, mInstanceKey);

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/CoreReactPackage.java

@@ -7,6 +7,7 @@
 
 package com.facebook.react.runtime;
 
+import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Nullsafe;
 import com.facebook.react.TurboReactPackage;
 import com.facebook.react.bridge.NativeModule;
@@ -51,7 +52,7 @@ public CoreReactPackage(
   }
 
   @Override
-  public NativeModule getModule(String name, ReactApplicationContext reactContext) {
+  public @Nullable NativeModule getModule(String name, ReactApplicationContext reactContext) {
     switch (name) {
       case AndroidInfoModule.NAME:
         return new AndroidInfoModule(reactContext);
@@ -68,8 +69,7 @@ public NativeModule getModule(String name, ReactApplicationContext reactContext)
       case ExceptionsManagerModule.NAME:
         return new ExceptionsManagerModule(mDevSupportManager);
       default:
-        throw new IllegalArgumentException(
-            "In BridgelessReactPackage, could not find Native module for " + name);
+        return null;
     }
   }