feat(tm-android): Reject Promise if Turbo Module method throws an Error (#37484)

krystofwoldrich · facebook-github-bot · commit 7612e6601a1c · 2023-10-27T10:20:40.000-07:00
Summary: ### [iOS change here](#40764) This PR builds upon the previous work done in #36925, which introduced native stack traces to the JSError for synchronous functions. The current modifications concentrate on functions that return Promises. Prior to this PR, errors within Promise-returning functions would be thrown at the platform layer crashing the app without a link to the JS stack. After the implementation of this PR, errors thrown within Promise-returning functions are now captured and transformed into rejected Promises. These rejected Promises contain a JS Error object that contains both the JS stack trace and the cause, along with the platform stack trace. Additionally, this PR ensures that rejections from native functions are now linked to the JS stack trace, providing a more comprehensive view of the rejection flow. ## Changelog:  [GENERAL][ADDED] - Turbo Modules Promise-returning functions reject with JS and platform stack traces information Pull Request resolved: #37484 Test Plan: | Android | |--------| | ![function_promise_android](https://github.com/krystofwoldrich/react-native/assets/31292499/1d1a3adf-986a-47b4-b98b-9e766176b7ae) | Example of intentionally rejected promise on Android: ``` { "name": "Error", "message": "Exception in HostFunction: intentional promise rejection", "stack": "[native code]\ntryCallTwo@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:25844:9\ndoResolve@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:25975:25\nPromise@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:25863:14\n[native code]\nrejectPromise@http://10.0.2.2:8081/js/examples/TurboModule/SampleTurboModuleExample.bundle?platform=android&lazy=true&app=com.facebook.react.uiapp&modulesOnly=true&dev=true&minify=false&runModule=true&shallow=true:42:70\nonPress@http://10.0.2.2:8081/js/examples/TurboModule/SampleTurboModuleExample.bundle?platform=android&lazy=true&app=com.facebook.react.uiapp&modulesOnly=true&dev=true&minify=false&runModule=true&shallow=true:242:71\n_performTransitionSideEffects@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:51896:22\n_receiveSignal@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:51852:45\nonResponderRelease@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:51715:34\ninvokeGuardedCallbackProd@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:2962:21\ninvokeGuardedCallback@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:3048:42\ninvokeGuardedCallbackAndCatchFirstError@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:3051:36\nexecuteDispatch@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:3115:48\nexecuteDispatchesInOrder@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:3132:26\nexecuteDispatchesAndRelease@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:4856:35\nforEach@[native code]\nforEachAccumulated@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:3574:22\nrunEventsInBatch@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:4874:27\nrunExtractedPluginEventsInBatch@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:4896:25\nhttp://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:4914:42\nbatchedUpdates$1@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:14750:20\nbatchedUpdates@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:4845:36\ndispatchEvent@http://10.0.2.2:8081/js/RNTesterApp.android.bundle?platform=android&dev=true&lazy=true&minify=false&app=com.facebook.react.uiapp&modulesOnly=false&runModule=true:4907:23", "cause": { "nativeStackAndroid": [ { "lineNumber": 173, "file": "SampleTurboModule.java", "methodName": "getValueWithPromise", "class": "com.facebook.fbreact.specs.SampleTurboModule" }, { "lineNumber": -2, "file": "NativeRunnable.java", "methodName": "run", "class": "com.facebook.jni.NativeRunnable" }, { "lineNumber": 942, "file": "Handler.java", "methodName": "handleCallback", "class": "android.os.Handler" }, { "lineNumber": 99, "file": "Handler.java", "methodName": "dispatchMessage", "class": "android.os.Handler" }, { "lineNumber": 27, "file": "MessageQueueThreadHandler.java", "methodName": "dispatchMessage", "class": "com.facebook.react.bridge.queue.MessageQueueThreadHandler" }, { "lineNumber": 201, "file": "Looper.java", "methodName": "loopOnce", "class": "android.os.Looper" }, { "lineNumber": 288, "file": "Looper.java", "methodName": "loop", "class": "android.os.Looper" }, { "lineNumber": 228, "file": "MessageQueueThreadImpl.java", "methodName": "run", "class": "com.facebook.react.bridge.queue.MessageQueueThreadImpl$4" }, { "lineNumber": 1012, "file": "Thread.java", "methodName": "run", "class": "java.lang.Thread" } ], "userInfo": null, "message": "intentional promise rejection", "code": "code 1" } } ``` How I logged out the Errors: ```js console.log('Error in JS:', JSON.stringify({ name: e.name, message: e.message, stack: e.stack, ...e, }, null, 2)); ``` Reviewed By: RSNara Differential Revision: D50613349 Pulled By: javache fbshipit-source-id: b49c469118c8d8d27c43164f110dfe57ddd592d9
diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
@@ -8,6 +8,7 @@
 package com.facebook.react.config;
 
 import com.facebook.proguard.annotations.DoNotStripAny;
+import com.facebook.react.common.build.ReactBuildConfig;
 
 /**
  * Hi there, traveller! This configuration class is not meant to be used by end-users of RN. It
@@ -161,4 +162,16 @@ public class ReactFeatureFlags {
    * priorities from any thread.
    */
   public static boolean useModernRuntimeScheduler = false;
+
+  /**
+   * Enables storing js caller stack when creating promise in native module. This is useful in case
+   * of Promise rejection and tracing the cause.
+   */
+  public static boolean traceTurboModulePromiseRejections = ReactBuildConfig.DEBUG;
+
+  /**
+   * Enables auto rejecting promises from Turbo Modules method calls. If native error occurs Promise
+   * in JS will be rejected (The JS error will include native stack)
+   */
+  public static boolean rejectTurboModulePromiseOnNativeError = true;
 }
diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp
@@ -58,6 +58,28 @@ JavaTurboModule::~JavaTurboModule() {
 
 namespace {
 
+constexpr auto kReactFeatureFlagsJavaDescriptor =
+    "com/facebook/react/config/ReactFeatureFlags";
+
+bool getFeatureFlagBoolValue(const char* name) {
+  static const auto reactFeatureFlagsClass =
+      facebook::jni::findClassStatic(kReactFeatureFlagsJavaDescriptor);
+  const auto field = reactFeatureFlagsClass->getStaticField<jboolean>(name);
+  return reactFeatureFlagsClass->getStaticFieldValue(field);
+}
+
+bool traceTurboModulePromiseRejections() {
+  static bool traceRejections =
+      getFeatureFlagBoolValue("traceTurboModulePromiseRejections");
+  return traceRejections;
+}
+
+bool rejectTurboModulePromiseOnNativeError() {
+  static bool rejectOnError =
+      getFeatureFlagBoolValue("rejectTurboModulePromiseOnNativeError");
+  return rejectOnError;
+}
+
 struct JNIArgs {
   JNIArgs(size_t count) : args_(count) {}
   std::vector<jvalue> args_;
@@ -396,7 +418,7 @@ jsi::Value createJSRuntimeError(
  */
 jsi::JSError convertThrowableToJSError(
     jsi::Runtime& runtime,
-    jni::local_ref<jni::JThrowable> throwable) {
+    jni::alias_ref<jni::JThrowable> throwable) {
   auto stackTrace = throwable->getStackTrace();
 
   jsi::Array stackElements(runtime, stackTrace->size());
@@ -424,6 +446,22 @@ jsi::JSError convertThrowableToJSError(
   return {runtime, std::move(error)};
 }
 
+void rejectWithException(
+    AsyncCallback<>& reject,
+    std::exception_ptr exception,
+    std::optional<std::string>& jsInvocationStack) {
+  auto throwable = jni::getJavaExceptionForCppException(exception);
+  reject.call([jsInvocationStack, throwable = jni::make_global(throwable)](
+                  jsi::Runtime& rt, jsi::Function& jsFunction) {
+    auto jsError = convertThrowableToJSError(rt, throwable);
+    if (jsInvocationStack.has_value()) {
+      jsError.value().asObject(rt).setProperty(
+          rt, "stack", jsInvocationStack.value());
+    }
+    jsFunction.call(rt, jsError.value());
+  });
+}
+
 } // namespace
 
 jsi::Value JavaTurboModule::invokeJavaMethod(
@@ -783,6 +821,10 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
       jsi::Function Promise =
           runtime.global().getPropertyAsFunction(runtime, "Promise");
 
+      // The callback is used for auto rejecting if error is caught from method
+      // invocation
+      std::optional<AsyncCallback<>> nativeRejectCallback;
+
       // The promise constructor runs its arg immediately, so this is safe
       jobject javaPromise;
       jsi::Value jsPromise = Promise.callAsConstructor(
@@ -799,6 +841,13 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
                   throw jsi::JSError(runtime, "Incorrect number of arguments");
                 }
 
+                if (rejectTurboModulePromiseOnNativeError()) {
+                  nativeRejectCallback = AsyncCallback(
+                      runtime,
+                      args[1].getObject(runtime).getFunction(runtime),
+                      jsInvoker_);
+                }
+
                 auto resolve = createJavaCallback(
                     runtime,
                     args[0].getObject(runtime).getFunction(runtime),
@@ -817,14 +866,25 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
       env->DeleteLocalRef(javaPromise);
       jargs[argCount].l = globalPromise;
 
+      // JS Stack at the time when the promise is created.
+      std::optional<std::string> jsInvocationStack;
+      if (traceTurboModulePromiseRejections()) {
+        jsInvocationStack = createJSRuntimeError(runtime, "")
+                                .asObject(runtime)
+                                .getProperty(runtime, "stack")
+                                .toString(runtime)
+                                .utf8(runtime);
+      }
+
       const char* moduleName = name_.c_str();
       const char* methodName = methodNameStr.c_str();
       TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName);
-
       TMPL::asyncMethodCallDispatch(moduleName, methodName);
       nativeMethodCallInvoker_->invokeAsync(
           methodName,
           [jargs,
+           rejectCallback = std::move(nativeRejectCallback),
+           jsInvocationStack = std::move(jsInvocationStack),
            globalRefs,
            methodID,
            instance_ = jni::make_weak(instance_),
@@ -856,7 +916,14 @@ jsi::Value JavaTurboModule::invokeJavaMethod(
               FACEBOOK_JNI_THROW_PENDING_EXCEPTION();
             } catch (...) {
               TMPL::asyncMethodCallExecutionFail(moduleName, methodName, id);
-              throw;
+              if (rejectTurboModulePromiseOnNativeError() && rejectCallback) {
+                auto exception = std::current_exception();
+                rejectWithException(
+                    *rejectCallback, exception, jsInvocationStack);
+                rejectCallback = std::nullopt;
+              } else {
+                throw;
+              }
             }
 
             for (auto globalRef : globalRefs) {
diff --git a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js
@@ -54,7 +54,10 @@ class SampleTurboModuleExample extends React.Component<{||}, State> {
     rejectPromise: () =>
       NativeSampleTurboModule.getValueWithPromise(true)
         .then(() => {})
-        .catch(e => this._setResult('rejectPromise', e.message)),
+        .catch(e => {
+          console.error(e);
+          this._setResult('rejectPromise', e.message);
+        }),
     getConstants: () => NativeSampleTurboModule.getConstants(),
     voidFunc: () => NativeSampleTurboModule.voidFunc(),
     getBool: () => NativeSampleTurboModule.getBool(true),
@@ -81,45 +84,49 @@ class SampleTurboModuleExample extends React.Component<{||}, State> {
       try {
         NativeSampleTurboModule.voidFuncThrows?.();
       } catch (e) {
+        console.error(e);
         return e.message;
       }
     },
     getObjectThrows: () => {
       try {
         NativeSampleTurboModule.getObjectThrows?.({a: 1, b: 'foo', c: null});
       } catch (e) {
+        console.error(e);
         return e.message;
       }
     },
     promiseThrows: () => {
-      try {
-        // $FlowFixMe[unused-promise]
-        NativeSampleTurboModule.promiseThrows?.();
-      } catch (e) {
-        return e.message;
-      }
+      NativeSampleTurboModule.promiseThrows?.()
+        .then(() => {})
+        .catch(e => {
+          console.error(e);
+          this._setResult('promiseThrows', e.message);
+        });
     },
     voidFuncAssert: () => {
       try {
         NativeSampleTurboModule.voidFuncAssert?.();
       } catch (e) {
+        console.error(e);
         return e.message;
       }
     },
     getObjectAssert: () => {
       try {
         NativeSampleTurboModule.getObjectAssert?.({a: 1, b: 'foo', c: null});
       } catch (e) {
+        console.error(e);
         return e.message;
       }
     },
     promiseAssert: () => {
-      try {
-        // $FlowFixMe[unused-promise]
-        NativeSampleTurboModule.promiseAssert?.();
-      } catch (e) {
-        return e.message;
-      }
+      NativeSampleTurboModule.promiseAssert?.()
+        .then(() => {})
+        .catch(e => {
+          console.error(e);
+          this._setResult('promiseAssert', e.message);
+        });
     },
   };
 
