Skip to content

Commit

Permalink
feat(tm-android): Reject Promise if Turbo Module method throws an Err…
Browse files Browse the repository at this point in the history 
…or (facebook#37484)

Summary:
### [iOS change here](facebook#40764)

This PR builds upon the previous work done in facebook#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:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[GENERAL][ADDED] - Turbo Modules Promise-returning functions reject with JS and platform stack traces information

Pull Request resolved: facebook#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
  • Loading branch information
@krystofwoldrich @Othinn
krystofwoldrich authored and Othinn committed Oct 30, 2023
1 parent ebea300 commit 9b50ad4
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 16 deletions.
13 changes: 13 additions & 0 deletions .../react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
73 changes: 70 additions & 3 deletions ...tive/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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),
Expand All @@ -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_),
Expand Down Expand Up @@ -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) {
Expand Down
33 changes: 20 additions & 13 deletions packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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);
});
},
};

Expand Down

0 comments on commit 9b50ad4

Please sign in to comment.