Skip to content

Commit

Permalink
Add additional checks to prevent throwing native java errors in UIMan…
Browse files Browse the repository at this point in the history
…agerBinding.cpp

Summary:
Right now there are places in UIManagerBinding.cpp where native java exceptions will be thrown if calling JSI functions results in errors, such as:

```Trying to report error getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
Error: getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
```

https://www.internalfb.com/intern/logview/details/facebook_android_javascripterrors/358181062b47b9561e60427bbb3816a9

In this diff I added LOG(ERROR) and checks because:
1, Throwing errors neither prevents the JSI errors nor handles them properly, checks prevent JSI errors from happening.
2, Errors are aggregated in Logview with other JSI errors as "Error: android_crash:com.facebook.react.devsupport.JSException:facebook::jni::throwNewJavaException" which keeps the SLA task open forever, checks can prevent JSI errors so they won't lead to exceptions, and  LOG(ERROR) will make sure we have enough info for debugging.

Changelog:
[General][Changed] -  Add checks and logs to for better error handling

Reviewed By: JoshuaGross

Differential Revision: D26885562

fbshipit-source-id: c0c1c057342e9efc0ff708188703f4332036e7a9
  • Loading branch information
luluwu2032 authored and facebook-github-bot committed Mar 15, 2021
1 parent 1bc06f1 commit ea1f953
Showing 1 changed file with 56 additions and 17 deletions.
73 changes: 56 additions & 17 deletions ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
namespace facebook {
namespace react {

static jsi::Object getModule(
static jsi::Value getModule(
jsi::Runtime &runtime,
std::string const &moduleName) {
auto batchedBridge =
Expand All @@ -31,7 +31,30 @@ static jsi::Object getModule(
LOG(ERROR) << "getModule of " << moduleName << " is not an object";
}
react_native_assert(moduleAsValue.isObject());
return moduleAsValue.asObject(runtime);
return moduleAsValue;
}

static bool checkBatchedBridgeIsActive(jsi::Runtime &runtime) {
if (!runtime.global().hasProperty(runtime, "__fbBatchedBridge")) {
LOG(ERROR)
<< "getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object";
return false;
}
return true;
}

static bool checkGetCallableModuleIsActive(jsi::Runtime &runtime) {
if (!checkBatchedBridgeIsActive(runtime)) {
return false;
}
auto batchedBridge =
runtime.global().getPropertyAsObject(runtime, "__fbBatchedBridge");
if (!batchedBridge.hasProperty(runtime, "getCallableModule")) {
LOG(ERROR)
<< "getPropertyAsFunction: function 'getCallableModule' is undefined, expected a Function";
return false;
}
return true;
}

std::shared_ptr<UIManagerBinding> UIManagerBinding::createAndInstallIfNeeded(
Expand Down Expand Up @@ -94,6 +117,27 @@ void UIManagerBinding::attach(std::shared_ptr<UIManager> const &uiManager) {
}
}

static void callMethodOfModule(
jsi::Runtime &runtime,
std::string const &moduleName,
std::string const &methodName,
std::initializer_list<jsi::Value> args) {
if (checkGetCallableModuleIsActive(runtime)) {
auto module = getModule(runtime, moduleName.c_str());
if (module.isObject()) {
jsi::Object object = module.asObject(runtime);
react_native_assert(object.hasProperty(runtime, methodName.c_str()));
if (object.hasProperty(runtime, methodName.c_str())) {
auto method = object.getPropertyAsFunction(runtime, methodName.c_str());
method.callWithThis(runtime, object, args);
} else {
LOG(ERROR) << "getPropertyAsFunction: property '" << methodName
<< "' is undefined, expected a Function";
}
}
}
}

void UIManagerBinding::startSurface(
jsi::Runtime &runtime,
SurfaceId surfaceId,
Expand All @@ -115,12 +159,10 @@ void UIManagerBinding::startSurface(
{jsi::String::createFromUtf8(runtime, moduleName),
jsi::valueFromDynamic(runtime, parameters)});
} else {
auto module = getModule(runtime, "AppRegistry");
auto method = module.getPropertyAsFunction(runtime, "runApplication");

method.callWithThis(
callMethodOfModule(
runtime,
module,
"AppRegistry",
"runApplication",
{jsi::String::createFromUtf8(runtime, moduleName),
jsi::valueFromDynamic(runtime, parameters)});
}
Expand All @@ -129,20 +171,17 @@ void UIManagerBinding::startSurface(
void UIManagerBinding::stopSurface(jsi::Runtime &runtime, SurfaceId surfaceId)
const {
auto global = runtime.global();
if (global.hasProperty(runtime, "RN$Bridgeless")) {
if (!global.hasProperty(runtime, "RN$stopSurface")) {
// ReactFabric module has not been loaded yet; there's no surface to stop.
return;
}
if (global.hasProperty(runtime, "RN$Bridgeless") &&
global.hasProperty(runtime, "RN$stopSurface")) {
// Bridgeless mode uses a custom JSI binding instead of callable module.
global.getPropertyAsFunction(runtime, "RN$stopSurface")
.call(runtime, {jsi::Value{surfaceId}});
} else {
auto module = getModule(runtime, "ReactFabric");
auto method =
module.getPropertyAsFunction(runtime, "unmountComponentAtNode");

method.callWithThis(runtime, module, {jsi::Value{surfaceId}});
callMethodOfModule(
runtime,
"ReactFabric",
"unmountComponentAtNode",
{jsi::Value{surfaceId}});
}
}

Expand Down

0 comments on commit ea1f953

Please sign in to comment.