From ff217c9e05a08493fe387594f1c7202e00ecc9be Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Tue, 13 Aug 2024 12:32:28 -0700 Subject: [PATCH] Route all js errors caught in C++ through JsErrorHandler (#45619) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../jserrorhandler/JsErrorHandler.cpp | 33 ++++++++- .../jserrorhandler/JsErrorHandler.h | 11 ++- .../react/runtime/ReactInstance.cpp | 72 +++++++++++-------- 3 files changed, 86 insertions(+), 30 deletions(-) diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp index 786996ba08ee59..ca2b95ae1d7b40 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp @@ -6,6 +6,8 @@ */ #include "JsErrorHandler.h" +#include +#include #include #include #include @@ -13,6 +15,7 @@ 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) { /** @@ -20,10 +23,13 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) { * 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*$)"); @@ -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+)\))$)"); @@ -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")) { @@ -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]), @@ -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]), @@ -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); } @@ -108,4 +135,8 @@ bool JsErrorHandler::hasHandledFatalError() { return _hasHandledFatalError; } +void JsErrorHandler::setJsPipelineReady() { + _isJsPipelineReady = true; +} + } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h index 92262d6fe871e0..d6099584556bb9 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h @@ -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 diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 041c774919c3ca..bd9659095d4446 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -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); + } + }); } }; @@ -118,7 +119,13 @@ ReactInstance::ReactInstance( }; } - runtimeScheduler_ = std::make_shared(runtimeExecutor); + runtimeScheduler_ = std::make_shared( + 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. @@ -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()); @@ -224,7 +240,7 @@ void ReactInstance::loadScript( strongBufferedRuntimeExecuter->flush(); } } catch (jsi::JSError& error) { - jsErrorHandler_->handleFatalError(error); + jsErrorHandler_->handleFatalError(runtime, error); } }); }