Skip to content

Commit

Permalink
Route all js errors caught in C++ through JsErrorHandler
Browse files Browse the repository at this point in the history
Summary:
If any fatal js error is caught in c++, just route it through js error handler.

Then, make js error handler call into the right pipeline:
1. After the js pipeline is ready: Route the errors through the js pipeline
2. Otherwise: Route errors through the c++ pipeline.

Changelog: [Internal]

Reviewed By: javache

Differential Revision: D60138417
  • Loading branch information
RSNara authored and facebook-github-bot committed Aug 13, 2024
1 parent 98b0c25 commit 2e8ea71
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/

#include "JsErrorHandler.h"
#include <cxxreact/ErrorUtils.h>

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester (JSC, OldArch)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester_dynamic_frameworks (JSC)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester (Hermes, OldArch)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester_ruby_3_2_0

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester (JSC, NewArch)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester_dynamic_frameworks (Hermes)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester_dynamic_frameworks (Hermes)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester (Hermes, NewArch)

'cxxreact/ErrorUtils.h' file not found

Check failure on line 9 in packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

View workflow job for this annotation

GitHub Actions / test_ios_rntester (Hermes, NewArch)

'cxxreact/ErrorUtils.h' file not found
#include <glog/logging.h>
#include <regex>
#include <sstream>
#include <string>
Expand Down Expand Up @@ -96,10 +98,26 @@ JsErrorHandler::JsErrorHandler(JsErrorHandler::OnJsError onJsError)

JsErrorHandler::~JsErrorHandler() {}

void JsErrorHandler::handleFatalError(const jsi::JSError& error) {
void JsErrorHandler::handleFatalError(
jsi::Runtime& runtime,
jsi::JSError& error) {
// TODO: Current error parsing works and is stable. Can investigate using
// REGEX_HERMES to get additional Hermes data, though it requires JS setup.
_hasHandledFatalError = true;

if (_isJsPipelineReady) {
try {
handleJSError(runtime, error, true);
return;
} catch (jsi::JSError& e) {
LOG(ERROR)
<< "JsErrorHandler: Failed to report js error using js pipeline. Using C++ pipeline instead."
<< std::endl
<< "Reporting failure: " << e.getMessage() << std::endl
<< "Original js error: " << error.getMessage() << std::endl;
}
}
// This is a hacky way to get Hermes stack trace.
ParsedError parsedError = parseErrorStack(error, true, false);
_onJsError(parsedError);
}
Expand All @@ -108,4 +126,8 @@ bool JsErrorHandler::hasHandledFatalError() {
return _hasHandledFatalError;
}

void JsErrorHandler::setJsPipelineReady() {
_isJsPipelineReady = true;
}

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,21 @@ class JsErrorHandler {
explicit JsErrorHandler(OnJsError onJsError);
~JsErrorHandler();

void handleFatalError(const jsi::JSError& error);
void handleFatalError(jsi::Runtime& runtime, jsi::JSError& error);
bool hasHandledFatalError();
void setJsPipelineReady();

private:
/**
* This callback:
* 1. Shouldn't retain the ReactInstance. So that we don't get retain cycles.
* 2. Should be implemented by something that can outlive the react instance
* (both before init and after teardown). So that errors during init and
* teardown get reported properly.
**/
OnJsError _onJsError;
bool _hasHandledFatalError;
bool _isJsPipelineReady{};
};

} // namespace facebook::react
72 changes: 44 additions & 28 deletions packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,44 +43,45 @@ ReactInstance::ReactInstance(
std::weak_ptr(timerManager_),
weakJsThread =
std::weak_ptr(jsMessageQueueThread_),
weakJsErrorHander = std::weak_ptr(
jsErrorHandler_)](auto callback) {
auto jsErrorHandler = weakJsErrorHander.lock();
if (weakRuntime.expired() || !jsErrorHandler) {
jsErrorHandler =
jsErrorHandler_](auto callback) {
if (weakRuntime.expired()) {
return;
}

if (jsErrorHandler->hasHandledFatalError()) {
LOG(INFO)
<< "RuntimeExecutor: Detected fatal js error. Dropping work on non-js thread."
<< "RuntimeExecutor: Detected fatal error. Dropping work on non-js thread."
<< std::endl;
return;
}

if (auto jsThread = weakJsThread.lock()) {
jsThread->runOnQueue(
[weakRuntime, weakTimerManager, callback = std::move(callback)]() {
auto runtime = weakRuntime.lock();
if (!runtime) {
return;
}
jsThread->runOnQueue([jsErrorHandler,
weakRuntime,
weakTimerManager,
callback = std::move(callback)]() {
auto runtime = weakRuntime.lock();
if (!runtime) {
return;
}

jsi::Runtime& jsiRuntime = runtime->getRuntime();
SystraceSection s("ReactInstance::_runtimeExecutor[Callback]");
try {
callback(jsiRuntime);

// If we have first-class support for microtasks,
// they would've been called as part of the previous callback.
if (!ReactNativeFeatureFlags::enableMicrotasks()) {
if (auto timerManager = weakTimerManager.lock()) {
timerManager->callReactNativeMicrotasks(jsiRuntime);
}
}
} catch (jsi::JSError& originalError) {
handleJSError(jsiRuntime, originalError, true);
jsi::Runtime& jsiRuntime = runtime->getRuntime();
SystraceSection s("ReactInstance::_runtimeExecutor[Callback]");
try {
callback(jsiRuntime);

// If we have first-class support for microtasks,
// they would've been called as part of the previous callback.
if (!ReactNativeFeatureFlags::enableMicrotasks()) {
if (auto timerManager = weakTimerManager.lock()) {
timerManager->callReactNativeMicrotasks(jsiRuntime);
}
});
}
} catch (jsi::JSError& originalError) {
jsErrorHandler->handleFatalError(jsiRuntime, originalError);
}
});
}
};

Expand Down Expand Up @@ -118,7 +119,13 @@ ReactInstance::ReactInstance(
};
}

runtimeScheduler_ = std::make_shared<RuntimeScheduler>(runtimeExecutor);
runtimeScheduler_ = std::make_shared<RuntimeScheduler>(
runtimeExecutor,
RuntimeSchedulerClock::now,
[jsErrorHandler = jsErrorHandler_](
jsi::Runtime& runtime, jsi::JSError& error) {
jsErrorHandler->handleFatalError(runtime, error);
});
runtimeScheduler_->setPerformanceEntryReporter(
// FIXME: Move creation of PerformanceEntryReporter to here and guarantee
// that its lifetime is the same as the runtime.
Expand Down Expand Up @@ -212,6 +219,15 @@ void ReactInstance::loadScript(
}

runtime.evaluateJavaScript(buffer, sourceURL);

/**
* TODO(T183610671): We need a safe/reliable way to enable the js
* pipeline from javascript. Remove this after we figure that out, or
* after we just remove the js pipeline.
*/
if (!jsErrorHandler_->hasHandledFatalError()) {
jsErrorHandler_->setJsPipelineReady();
}
if (hasLogger) {
ReactMarker::logTaggedMarkerBridgeless(
ReactMarker::RUN_JS_BUNDLE_STOP, scriptName.c_str());
Expand All @@ -224,7 +240,7 @@ void ReactInstance::loadScript(
strongBufferedRuntimeExecuter->flush();
}
} catch (jsi::JSError& error) {
jsErrorHandler_->handleFatalError(error);
jsErrorHandler_->handleFatalError(runtime, error);
}
});
}
Expand Down

0 comments on commit 2e8ea71

Please sign in to comment.