Skip to content

Commit ea1f953

Browse files
luluwu2032facebook-github-bot
authored andcommitted
Add additional checks to prevent throwing native java errors in UIManagerBinding.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
1 parent 1bc06f1 commit ea1f953

File tree

1 file changed

+56
-17
lines changed

1 file changed

+56
-17
lines changed

ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp

+56-17
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
namespace facebook {
1717
namespace react {
1818

19-
static jsi::Object getModule(
19+
static jsi::Value getModule(
2020
jsi::Runtime &runtime,
2121
std::string const &moduleName) {
2222
auto batchedBridge =
@@ -31,7 +31,30 @@ static jsi::Object getModule(
3131
LOG(ERROR) << "getModule of " << moduleName << " is not an object";
3232
}
3333
react_native_assert(moduleAsValue.isObject());
34-
return moduleAsValue.asObject(runtime);
34+
return moduleAsValue;
35+
}
36+
37+
static bool checkBatchedBridgeIsActive(jsi::Runtime &runtime) {
38+
if (!runtime.global().hasProperty(runtime, "__fbBatchedBridge")) {
39+
LOG(ERROR)
40+
<< "getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object";
41+
return false;
42+
}
43+
return true;
44+
}
45+
46+
static bool checkGetCallableModuleIsActive(jsi::Runtime &runtime) {
47+
if (!checkBatchedBridgeIsActive(runtime)) {
48+
return false;
49+
}
50+
auto batchedBridge =
51+
runtime.global().getPropertyAsObject(runtime, "__fbBatchedBridge");
52+
if (!batchedBridge.hasProperty(runtime, "getCallableModule")) {
53+
LOG(ERROR)
54+
<< "getPropertyAsFunction: function 'getCallableModule' is undefined, expected a Function";
55+
return false;
56+
}
57+
return true;
3558
}
3659

3760
std::shared_ptr<UIManagerBinding> UIManagerBinding::createAndInstallIfNeeded(
@@ -94,6 +117,27 @@ void UIManagerBinding::attach(std::shared_ptr<UIManager> const &uiManager) {
94117
}
95118
}
96119

120+
static void callMethodOfModule(
121+
jsi::Runtime &runtime,
122+
std::string const &moduleName,
123+
std::string const &methodName,
124+
std::initializer_list<jsi::Value> args) {
125+
if (checkGetCallableModuleIsActive(runtime)) {
126+
auto module = getModule(runtime, moduleName.c_str());
127+
if (module.isObject()) {
128+
jsi::Object object = module.asObject(runtime);
129+
react_native_assert(object.hasProperty(runtime, methodName.c_str()));
130+
if (object.hasProperty(runtime, methodName.c_str())) {
131+
auto method = object.getPropertyAsFunction(runtime, methodName.c_str());
132+
method.callWithThis(runtime, object, args);
133+
} else {
134+
LOG(ERROR) << "getPropertyAsFunction: property '" << methodName
135+
<< "' is undefined, expected a Function";
136+
}
137+
}
138+
}
139+
}
140+
97141
void UIManagerBinding::startSurface(
98142
jsi::Runtime &runtime,
99143
SurfaceId surfaceId,
@@ -115,12 +159,10 @@ void UIManagerBinding::startSurface(
115159
{jsi::String::createFromUtf8(runtime, moduleName),
116160
jsi::valueFromDynamic(runtime, parameters)});
117161
} else {
118-
auto module = getModule(runtime, "AppRegistry");
119-
auto method = module.getPropertyAsFunction(runtime, "runApplication");
120-
121-
method.callWithThis(
162+
callMethodOfModule(
122163
runtime,
123-
module,
164+
"AppRegistry",
165+
"runApplication",
124166
{jsi::String::createFromUtf8(runtime, moduleName),
125167
jsi::valueFromDynamic(runtime, parameters)});
126168
}
@@ -129,20 +171,17 @@ void UIManagerBinding::startSurface(
129171
void UIManagerBinding::stopSurface(jsi::Runtime &runtime, SurfaceId surfaceId)
130172
const {
131173
auto global = runtime.global();
132-
if (global.hasProperty(runtime, "RN$Bridgeless")) {
133-
if (!global.hasProperty(runtime, "RN$stopSurface")) {
134-
// ReactFabric module has not been loaded yet; there's no surface to stop.
135-
return;
136-
}
174+
if (global.hasProperty(runtime, "RN$Bridgeless") &&
175+
global.hasProperty(runtime, "RN$stopSurface")) {
137176
// Bridgeless mode uses a custom JSI binding instead of callable module.
138177
global.getPropertyAsFunction(runtime, "RN$stopSurface")
139178
.call(runtime, {jsi::Value{surfaceId}});
140179
} else {
141-
auto module = getModule(runtime, "ReactFabric");
142-
auto method =
143-
module.getPropertyAsFunction(runtime, "unmountComponentAtNode");
144-
145-
method.callWithThis(runtime, module, {jsi::Value{surfaceId}});
180+
callMethodOfModule(
181+
runtime,
182+
"ReactFabric",
183+
"unmountComponentAtNode",
184+
{jsi::Value{surfaceId}});
146185
}
147186
}
148187

0 commit comments

Comments
 (0)