Skip to content

Commit

Permalink
Route all js errors caught in C++ through JsErrorHandler (#45619)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #45619

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 @facebook-github-bot
RSNara authored and facebook-github-bot committed Aug 13, 2024
1 parent 98b0c25 commit ff217c9
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 30 deletions.
33 changes: 32 additions & 1 deletion packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,30 @@
*/

#include "JsErrorHandler.h"
#include <cxxreact/ErrorUtils.h>
#include <glog/logging.h>
#include <regex>
#include <sstream>
#include <string>
#include <vector>

namespace facebook::react {

// TODO(T198763073): Migrate away from std::regex in this function
static JsErrorHandler::ParsedError
parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
/**
* This parses the different stack traces and puts them into one format
* This borrows heavily from TraceKit (https://github.com/occ/TraceKit)
* This is the same regex from stacktrace-parser.js.
*/
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
const std::regex REGEX_CHROME(
R"(^\s*at (?:(?:(?:Anonymous function)?|((?:\[object object\])?\S+(?: \[as \S+\])?)) )?\(?((?:file|http|https):.*?):(\d+)(?::(\d+))?\)?\s*$)");
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
const std::regex REGEX_GECKO(
R"(^(?:\s*([^@]*)(?:\((.*?)\))?@)?(\S.*?):(\d+)(?::(\d+))?\s*$)");
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
const std::regex REGEX_NODE(
R"(^\s*at (?:((?:\[object object\])?\S+(?: \[as \S+\])?) )?\(?(.*?):(\d+)(?::(\d+))?\)?\s*$)");

Expand All @@ -34,6 +40,7 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
// 4. source URL (filename)
// 5. line number (1 based)
// 6. column number (1 based) or virtual offset (0 based)
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
const std::regex REGEX_HERMES(
R"(^ {4}at (.+?)(?: \((native)\)?| \((address at )?(.*?):(\d+):(\d+)\))$)");

Expand All @@ -46,6 +53,7 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
auto searchResults = std::smatch{};

if (isHermes) {
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
if (std::regex_search(line, searchResults, REGEX_HERMES)) {
std::string str2 = std::string(searchResults[2]);
if (str2.compare("native")) {
Expand All @@ -58,6 +66,7 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
}
}
} else {
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
if (std::regex_search(line, searchResults, REGEX_GECKO)) {
frames.push_back({
.fileName = std::string(searchResults[3]),
Expand All @@ -66,7 +75,9 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
.columnNumber = std::stoi(searchResults[5]),
});
} else if (
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
std::regex_search(line, searchResults, REGEX_CHROME) ||
// NOLINTNEXTLINE(facebook-hte-StdRegexIsAwful)
std::regex_search(line, searchResults, REGEX_NODE)) {
frames.push_back({
.fileName = std::string(searchResults[2]),
Expand Down Expand Up @@ -96,10 +107,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 +135,8 @@ bool JsErrorHandler::hasHandledFatalError() {
return _hasHandledFatalError;
}

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

} // namespace facebook::react
11 changes: 10 additions & 1 deletion packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h
View file
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
View file
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 ff217c9

Please sign in to comment.