Skip to content

Commit 71391db

Browse files
RSNarafacebook-github-bot
authored andcommitted
Refactor: ReactInstance: Pull "fatal error" bool into JsErrorHandler
Summary: I think we should try to centralize all things js error handling related inside JsErrorHandler. So, I moved this bool into JsErrorHandler. This makes ReactInstance easier to understand: it removes one member variable from ReactInstance. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D55547897
1 parent 8dbf114 commit 71391db

4 files changed

Lines changed: 21 additions & 17 deletions

File tree

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
9090

9191
JsErrorHandler::JsErrorHandler(
9292
JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc)
93-
: _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)){
93+
: _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)),
94+
_hasHandledFatalError(false){
9495

9596
};
9697

@@ -99,8 +100,15 @@ JsErrorHandler::~JsErrorHandler() {}
99100
void JsErrorHandler::handleJsError(const jsi::JSError& error, bool isFatal) {
100101
// TODO: Current error parsing works and is stable. Can investigate using
101102
// REGEX_HERMES to get additional Hermes data, though it requires JS setup.
103+
if (isFatal) {
104+
_hasHandledFatalError = true;
105+
}
102106
ParsedError parsedError = parseErrorStack(error, isFatal, false);
103107
_jsErrorHandlingFunc(parsedError);
104108
}
105109

110+
bool JsErrorHandler::hasHandledFatalError() {
111+
return _hasHandledFatalError;
112+
}
113+
106114
} // namespace facebook::react

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@ class JsErrorHandler {
3333
~JsErrorHandler();
3434

3535
void handleJsError(const jsi::JSError& error, bool isFatal);
36+
bool hasHandledFatalError();
3637

3738
private:
3839
JsErrorHandlingFunc _jsErrorHandlingFunc;
40+
bool _hasHandledFatalError;
3941
};
4042

4143
} // namespace facebook::react

packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,23 @@ ReactInstance::ReactInstance(
3838
timerManager_(std::move(timerManager)),
3939
jsErrorHandler_(
4040
std::make_shared<JsErrorHandler>(std::move(jsErrorHandlingFunc))),
41-
hasFatalJsError_(std::make_shared<bool>(false)),
4241
parentInspectorTarget_(parentInspectorTarget) {
4342
RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_),
4443
weakTimerManager =
4544
std::weak_ptr(timerManager_),
4645
weakJsMessageQueueThread =
4746
std::weak_ptr(jsMessageQueueThread_),
48-
weakHasFatalJsError = std::weak_ptr(
49-
hasFatalJsError_)](auto callback) {
50-
if (std::shared_ptr<bool> sharedHasFatalJsError =
51-
weakHasFatalJsError.lock()) {
52-
if (*sharedHasFatalJsError) {
53-
LOG(INFO)
54-
<< "Calling into JS using runtimeExecutor but hasFatalJsError_ is true";
55-
return;
56-
}
47+
weakJsErrorHander = std::weak_ptr(
48+
jsErrorHandler_)](auto callback) {
49+
auto jsErrorHandler = weakJsErrorHander.lock();
50+
if (weakRuntime.expired() || !jsErrorHandler) {
51+
return;
5752
}
58-
if (weakRuntime.expired()) {
53+
54+
if (jsErrorHandler->hasHandledFatalError()) {
55+
LOG(INFO)
56+
<< "RuntimeExecutor: Detected fatal js error. Dropping work on non-js thread."
57+
<< std::endl;
5958
return;
6059
}
6160

@@ -223,8 +222,6 @@ void ReactInstance::loadScript(
223222
strongBufferedRuntimeExecuter->flush();
224223
}
225224
} catch (jsi::JSError& error) {
226-
// Handle uncaught JS errors during loading JS bundle
227-
*hasFatalJsError_ = true;
228225
jsErrorHandler_->handleJsError(error, true);
229226
}
230227
});

packages/react-native/ReactCommon/react/runtime/ReactInstance.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,6 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate {
8282
std::shared_ptr<RuntimeScheduler> runtimeScheduler_;
8383
std::shared_ptr<JsErrorHandler> jsErrorHandler_;
8484

85-
// Whether there are errors caught during bundle loading
86-
std::shared_ptr<bool> hasFatalJsError_;
87-
8885
jsinspector_modern::InstanceTarget* inspectorTarget_{nullptr};
8986
jsinspector_modern::RuntimeTarget* runtimeInspectorTarget_{nullptr};
9087
jsinspector_modern::HostTarget* parentInspectorTarget_{nullptr};

0 commit comments

Comments
 (0)