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

[krystofwoldrich]

Summary:

iOS change here

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

Test Plan:

Android

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:

console.log('Error in JS:', JSON.stringify({
  name: e.name,
  message: e.message,
  stack: e.stack,
  ...e,
}, null, 2));

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	17,657,478	+12,413
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,039,156	+12,418
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 98e7ecd
Branch: main

[javache]

I'm not 100% convinced we need to use the Error callback for this, given the overheads it requires.

Consider the JS equivalent of this type of module invocation to be something like this, which also will not call the original reject handler.

new Promise(() => {
  setTimeout(() => {
    throw new Error('foo');
  }, 0)
).catch(err => console.error('error' + err));

My perspective is that the behaviour for all async native module calls in RN should be a Logbox in development (which we should fix if that's not happening here!) and a RN fatal error in production (which apps can still decide to handle by restarting the React app - without crashing the entire app).

[javache]

Making native exceptions more actionable in development by capturing a JS stacktrace to associate with it is a great idea by the way. I just don't think we want to do it in production - potentially we could make it a runtime flag so app authors can decide?

[krystofwoldrich]

@RSNara @javache Thank you for the feedback. I understand the performance overhead. I would like to go the runtime flag way. Could you point me toward an existing runtime flag where I could add this one?

@javache Yes, it would be an improved experience compared to standard JS behavior.

[javache]

On Android, we use ReactFeatureFlags - which you'll need to use JNI to access - see getFeatureFlag - on iOS, we use global helpers like RCTEnableTurboModule to control feature flags.

[krystofwoldrich]

@javache Thank you for all the suggestions, corrections, and nits. I appreciate it.

I've removed iOS changes from this PR making it Android only. I'll link PR with iOS changes here.

Let me know if there is anything more! 🙏

iOS PR Here

[javache]

Is it expected that even "regular" rejects (eg the module implementation explicitly rejecting the promise) including the original caller's JS stack? Does that align with behaviour we see in other JS engines?

[krystofwoldrich]

@javache

Is it expected that even "regular" rejects (eg the module implementation explicitly rejecting the promise) including the original caller's JS stack? Does that align with behaviour we see in other JS engines?

I would say it's expected because to get the caller's JS stack the user had to enable the feature by the flag traceTurboModulePromiseRejections that explicitly says trace promise rejections (meaning bot explicit rejections and ones based on thrown exception)

[krystofwoldrich]

@javache I don't know how exactly to use the new AsyncCallback. I'm getting undefined in JS instead of the JSError.

#37484 (comment)

[krystofwoldrich]

@javache Thanks for the notes, I made the updates, let me know what you think. 🙏

[javache]

This is looking great! I love how much more readable this has become (I'll review the iOS one once this is good-to-go)

[facebook-github-bot]

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@javache merged this pull request in 7612e66.