From 694f9d72f218309934eb1a38985728f00bef5ca0 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 1/7] [skip ci] Noop: ReactInstance: Add todo above getRuntimeScheduler() (#43952) Summary: getRuntimeScheduler() allows things to schedule work on the js thread by bypassing main bundle buffering. This is unsafe: almost everything should be using the buffered runtime executor, unless it sets up bindings used in the main bundle. I filed a task for the investigation to see if there's any problems. And added it to the code in this diff. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D55547899 --- .../react-native/ReactCommon/react/runtime/ReactInstance.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 3ed3b1342151..01083b78b501 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -163,6 +163,8 @@ RuntimeExecutor ReactInstance::getBufferedRuntimeExecutor() noexcept { }; } +// TODO(T184010230): Should the RuntimeScheduler returned from this method be +// buffered? std::shared_ptr ReactInstance::getRuntimeScheduler() noexcept { return runtimeScheduler_; From c86ce383942f3114f5e9cd87f168bb8b22280124 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 2/7] [skip ci] Refactor: RuntimeScheduler: Delete ErrorUtils.h (#43953) Summary: RuntimeScheduler's ErrorUtils.h is redundant. Let's just remove it. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D55547905 --- .../renderer/runtimescheduler/ErrorUtils.h | 36 ------------------- .../runtimescheduler/RuntimeScheduler.cpp | 1 - .../RuntimeScheduler_Legacy.cpp | 6 ++-- .../RuntimeScheduler_Modern.cpp | 3 +- 4 files changed, 4 insertions(+), 42 deletions(-) delete mode 100644 packages/react-native/ReactCommon/react/renderer/runtimescheduler/ErrorUtils.h diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/ErrorUtils.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/ErrorUtils.h deleted file mode 100644 index c15de49aac6b..000000000000 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/ErrorUtils.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include - -namespace facebook::react { - -inline static void handleFatalError( - jsi::Runtime& runtime, - const jsi::JSError& error) { - auto reportFatalError = "reportFatalError"; - auto errorUtils = runtime.global().getProperty(runtime, "ErrorUtils"); - if (errorUtils.isUndefined() || !errorUtils.isObject() || - !errorUtils.getObject(runtime).hasProperty(runtime, reportFatalError)) { - // ErrorUtils was not set up. This probably means the bundle didn't - // load properly. - throw jsi::JSError( - runtime, - "ErrorUtils is not set up properly. Something probably went wrong trying to load the JS bundle. Trying to report error " + - error.getMessage(), - error.getStack()); - } - - auto func = errorUtils.asObject(runtime).getPropertyAsFunction( - runtime, reportFatalError); - - func.call(runtime, error.value()); -} - -} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp index ffad447dc366..804e7cf10be9 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp @@ -13,7 +13,6 @@ #include #include #include -#include "ErrorUtils.h" namespace facebook::react { diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp index 2d13b9b8e645..e16188137413 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp @@ -8,10 +8,10 @@ #include "RuntimeScheduler_Legacy.h" #include "SchedulerPriorityUtils.h" +#include #include #include #include -#include "ErrorUtils.h" namespace facebook::react { @@ -139,7 +139,7 @@ void RuntimeScheduler_Legacy::callExpiredTasks(jsi::Runtime& runtime) { executeTask(runtime, topPriorityTask, didUserCallbackTimeout); } } catch (jsi::JSError& error) { - handleFatalError(runtime, error); + handleJSError(runtime, error, true); } currentPriority_ = previousPriority; @@ -191,7 +191,7 @@ void RuntimeScheduler_Legacy::startWorkLoop(jsi::Runtime& runtime) { executeTask(runtime, topPriorityTask, didUserCallbackTimeout); } } catch (jsi::JSError& error) { - handleFatalError(runtime, error); + handleJSError(runtime, error, true); } currentPriority_ = previousPriority; diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp index c07e2f8c2e9c..8cc6a6deebb0 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp @@ -14,7 +14,6 @@ #include #include #include -#include "ErrorUtils.h" namespace facebook::react { @@ -211,7 +210,7 @@ void RuntimeScheduler_Modern::startWorkLoop( executeTask(runtime, topPriorityTask, currentTime); } } catch (jsi::JSError& error) { - handleFatalError(runtime, error); + handleJSError(runtime, error, true); } currentPriority_ = previousPriority; From a0f3191ee25e396293af82e74cd873a2d17005ed Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 3/7] [skip ci] Refactor: RuntimeScheduler: Stop defaulting ctor args redundantly (#43954) Summary: Now, all the defaulting is in RuntimeScheduler.h. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D55547900 --- .../react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h | 3 +-- .../react/renderer/runtimescheduler/RuntimeScheduler_Modern.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h index d5f673d39b66..502c0def5ef0 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h @@ -22,8 +22,7 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase { public: explicit RuntimeScheduler_Legacy( RuntimeExecutor runtimeExecutor, - std::function now = - RuntimeSchedulerClock::now); + std::function now); /* * Not copyable. diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h index 372d8c5da19a..f730509928ba 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h @@ -23,8 +23,7 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { public: explicit RuntimeScheduler_Modern( RuntimeExecutor runtimeExecutor, - std::function now = - RuntimeSchedulerClock::now); + std::function now); /* * Not copyable. From baaaba6dcb2c81b162f82379e84d3466964e3ee9 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 4/7] [skip ci] Refactor: ReactInstance: Store JsErrorHandler inside a shared_ptr (#43955) Summary: Just makes it easier to pass around JsErrorHandler. We'll need this in D55547897, when we start storing the "has fataled" boolean inside the JsErrorHandler. Changelog: [internal] Reviewed By: cipolleschi Differential Revision: D55547898 --- .../react-native/ReactCommon/react/runtime/ReactInstance.cpp | 5 +++-- .../react-native/ReactCommon/react/runtime/ReactInstance.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 01083b78b501..2571fed723b0 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -36,7 +36,8 @@ ReactInstance::ReactInstance( : runtime_(std::move(runtime)), jsMessageQueueThread_(jsMessageQueueThread), timerManager_(std::move(timerManager)), - jsErrorHandler_(std::move(jsErrorHandlingFunc)), + jsErrorHandler_( + std::make_shared(std::move(jsErrorHandlingFunc))), hasFatalJsError_(std::make_shared(false)), parentInspectorTarget_(parentInspectorTarget) { RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), @@ -224,7 +225,7 @@ void ReactInstance::loadScript( } catch (jsi::JSError& error) { // Handle uncaught JS errors during loading JS bundle *hasFatalJsError_ = true; - this->jsErrorHandler_.handleJsError(error, true); + jsErrorHandler_->handleJsError(error, true); } }); } diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h index c785147f72bc..48393cb9a06d 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h @@ -80,7 +80,7 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate { std::shared_ptr timerManager_; std::unordered_map> modules_; std::shared_ptr runtimeScheduler_; - JsErrorHandler jsErrorHandler_; + std::shared_ptr jsErrorHandler_; // Whether there are errors caught during bundle loading std::shared_ptr hasFatalJsError_; From aa889a2e5b34372b21ebdc0b5aeca373481df123 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 5/7] [skip ci] Refactor: ReactInstance: Pull "fatal error" bool into JsErrorHandler (#43956) 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 --- .../jserrorhandler/JsErrorHandler.cpp | 10 +++++++- .../jserrorhandler/JsErrorHandler.h | 2 ++ .../react/runtime/ReactInstance.cpp | 23 ++++++++----------- .../ReactCommon/react/runtime/ReactInstance.h | 3 --- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp index 9b474c1e1391..1c00b15dc951 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp @@ -90,7 +90,8 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) { JsErrorHandler::JsErrorHandler( JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc) - : _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)){ + : _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)), + _hasHandledFatalError(false){ }; @@ -99,8 +100,15 @@ JsErrorHandler::~JsErrorHandler() {} void JsErrorHandler::handleJsError(const jsi::JSError& error, bool isFatal) { // TODO: Current error parsing works and is stable. Can investigate using // REGEX_HERMES to get additional Hermes data, though it requires JS setup. + if (isFatal) { + _hasHandledFatalError = true; + } ParsedError parsedError = parseErrorStack(error, isFatal, false); _jsErrorHandlingFunc(parsedError); } +bool JsErrorHandler::hasHandledFatalError() { + return _hasHandledFatalError; +} + } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h index 69f714d48536..d7e16eb9e6b8 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h @@ -33,9 +33,11 @@ class JsErrorHandler { ~JsErrorHandler(); void handleJsError(const jsi::JSError& error, bool isFatal); + bool hasHandledFatalError(); private: JsErrorHandlingFunc _jsErrorHandlingFunc; + bool _hasHandledFatalError; }; } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 2571fed723b0..1984ab3c3f12 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -38,24 +38,23 @@ ReactInstance::ReactInstance( timerManager_(std::move(timerManager)), jsErrorHandler_( std::make_shared(std::move(jsErrorHandlingFunc))), - hasFatalJsError_(std::make_shared(false)), parentInspectorTarget_(parentInspectorTarget) { RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), weakTimerManager = std::weak_ptr(timerManager_), weakJsMessageQueueThread = std::weak_ptr(jsMessageQueueThread_), - weakHasFatalJsError = std::weak_ptr( - hasFatalJsError_)](auto callback) { - if (std::shared_ptr sharedHasFatalJsError = - weakHasFatalJsError.lock()) { - if (*sharedHasFatalJsError) { - LOG(INFO) - << "Calling into JS using runtimeExecutor but hasFatalJsError_ is true"; - return; - } + weakJsErrorHander = std::weak_ptr( + jsErrorHandler_)](auto callback) { + auto jsErrorHandler = weakJsErrorHander.lock(); + if (weakRuntime.expired() || !jsErrorHandler) { + return; } - if (weakRuntime.expired()) { + + if (jsErrorHandler->hasHandledFatalError()) { + LOG(INFO) + << "RuntimeExecutor: Detected fatal js error. Dropping work on non-js thread." + << std::endl; return; } @@ -223,8 +222,6 @@ void ReactInstance::loadScript( strongBufferedRuntimeExecuter->flush(); } } catch (jsi::JSError& error) { - // Handle uncaught JS errors during loading JS bundle - *hasFatalJsError_ = true; jsErrorHandler_->handleJsError(error, true); } }); diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h index 48393cb9a06d..d44b539bd7eb 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h @@ -82,9 +82,6 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate { std::shared_ptr runtimeScheduler_; std::shared_ptr jsErrorHandler_; - // Whether there are errors caught during bundle loading - std::shared_ptr hasFatalJsError_; - jsinspector_modern::InstanceTarget* inspectorTarget_{nullptr}; jsinspector_modern::RuntimeTarget* runtimeInspectorTarget_{nullptr}; jsinspector_modern::HostTarget* parentInspectorTarget_{nullptr}; From 243f6bc3abe0e27fbf24556d513726fbea6bc6f7 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 6/7] [skip ci] Refactor: JsErrorHandler: Rename handleJsError to handleFatalError (#43957) Summary: Right now, JsErrorHandler is only used to handle fatal exceptions. So, let's just scope handleJsError down to handleFatalError. Changelog: [General][Breaking] - JsErrorHandler: Rename handleJsError to handleFatalError Reviewed By: cortinico Differential Revision: D55547901 --- .../ReactCommon/jserrorhandler/JsErrorHandler.cpp | 8 +++----- .../ReactCommon/jserrorhandler/JsErrorHandler.h | 2 +- .../ReactCommon/react/runtime/ReactInstance.cpp | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp index 1c00b15dc951..d70a4463761e 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp @@ -97,13 +97,11 @@ JsErrorHandler::JsErrorHandler( JsErrorHandler::~JsErrorHandler() {} -void JsErrorHandler::handleJsError(const jsi::JSError& error, bool isFatal) { +void JsErrorHandler::handleFatalError(const 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. - if (isFatal) { - _hasHandledFatalError = true; - } - ParsedError parsedError = parseErrorStack(error, isFatal, false); + _hasHandledFatalError = true; + ParsedError parsedError = parseErrorStack(error, true, false); _jsErrorHandlingFunc(parsedError); } diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h index d7e16eb9e6b8..53a03367989f 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h @@ -32,7 +32,7 @@ class JsErrorHandler { explicit JsErrorHandler(JsErrorHandlingFunc jsErrorHandlingFunc); ~JsErrorHandler(); - void handleJsError(const jsi::JSError& error, bool isFatal); + void handleFatalError(const jsi::JSError& error); bool hasHandledFatalError(); private: diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 1984ab3c3f12..440e9f5de576 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -222,7 +222,7 @@ void ReactInstance::loadScript( strongBufferedRuntimeExecuter->flush(); } } catch (jsi::JSError& error) { - jsErrorHandler_->handleJsError(error, true); + jsErrorHandler_->handleFatalError(error); } }); } From a4fc3652be1dadb92d5f3a43eacde20d9ffedfb1 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 8 Apr 2024 12:32:46 -0700 Subject: [PATCH 7/7] Refactor: JsErrorHandler: Rename JsErrorHandlingFunc -> OnJsError (#43985) Summary: This is just personal preference. The name "OnJsError" makes the intent of the abstraction clear: an instance of OnJsError is a function that gets called when a js error is caught. The name "JsErrorHandlingFunc" is not as good. Changelog: [General][Breaking] - JsErrorHandler: Rename JsErrorHandlingFunc to OnJsError Reviewed By: christophpurrer Differential Revision: D55563580 --- .../jni/react/runtime/jni/JReactInstance.cpp | 4 +-- .../jserrorhandler/JsErrorHandler.cpp | 7 ++-- .../jserrorhandler/JsErrorHandler.h | 6 ++-- .../tests/ReactInstanceIntegrationTest.cpp | 32 +++++++++---------- .../react/runtime/ReactInstance.cpp | 5 ++- .../ReactCommon/react/runtime/ReactInstance.h | 2 +- .../platform/ios/ReactCommon/RCTInstance.mm | 4 +-- .../runtime/tests/cxx/ReactInstanceTest.cpp | 10 +++--- 8 files changed, 33 insertions(+), 37 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp index 5c0f1bc678f1..e111d2c0a63f 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp @@ -51,7 +51,7 @@ JReactInstance::JReactInstance( jsTimerExecutor->cthis()->setTimerManager(timerManager); jReactExceptionManager_ = jni::make_global(jReactExceptionManager); - auto jsErrorHandlingFunc = + auto onJsError = [weakJReactExceptionManager = jni::make_weak(jReactExceptionManager)]( const JsErrorHandler::ParsedError& error) mutable noexcept { if (auto jReactExceptionManager = @@ -66,7 +66,7 @@ JReactInstance::JReactInstance( jsRuntimeFactory->cthis()->createJSRuntime(sharedJSMessageQueueThread), sharedJSMessageQueueThread, timerManager, - std::move(jsErrorHandlingFunc), + std::move(onJsError), jReactHostInspectorTarget ? jReactHostInspectorTarget->cthis()->getInspectorTarget() : nullptr); diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp index d70a4463761e..786996ba08ee 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp @@ -88,9 +88,8 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) { }; } -JsErrorHandler::JsErrorHandler( - JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc) - : _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)), +JsErrorHandler::JsErrorHandler(JsErrorHandler::OnJsError onJsError) + : _onJsError(std::move(onJsError)), _hasHandledFatalError(false){ }; @@ -102,7 +101,7 @@ void JsErrorHandler::handleFatalError(const jsi::JSError& error) { // REGEX_HERMES to get additional Hermes data, though it requires JS setup. _hasHandledFatalError = true; ParsedError parsedError = parseErrorStack(error, true, false); - _jsErrorHandlingFunc(parsedError); + _onJsError(parsedError); } bool JsErrorHandler::hasHandledFatalError() { diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h index 53a03367989f..92262d6fe871 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h @@ -27,16 +27,16 @@ class JsErrorHandler { bool isFatal; }; - using JsErrorHandlingFunc = std::function; + using OnJsError = std::function; - explicit JsErrorHandler(JsErrorHandlingFunc jsErrorHandlingFunc); + explicit JsErrorHandler(OnJsError onJsError); ~JsErrorHandler(); void handleFatalError(const jsi::JSError& error); bool hasHandledFatalError(); private: - JsErrorHandlingFunc _jsErrorHandlingFunc; + OnJsError _onJsError; bool _hasHandledFatalError; }; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp index 17ed7a7cce52..c49e72c0bb7c 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp @@ -34,22 +34,20 @@ void ReactInstanceIntegrationTest::SetUp() { auto timerManager = std::make_shared(std::move(mockRegistry)); - auto jsErrorHandlingFunc = - [](const JsErrorHandler::ParsedError& errorMap) noexcept { - LOG(INFO) << "[jsErrorHandlingFunc called]"; - LOG(INFO) << "message: " << errorMap.message; - LOG(INFO) << "exceptionId: " << std::to_string(errorMap.exceptionId); - LOG(INFO) << "isFatal: " - << std::to_string(static_cast(errorMap.isFatal)); - auto frames = errorMap.frames; - for (const auto& mapBuffer : frames) { - LOG(INFO) << "[Frame]" << std::endl - << "\tfile: " << mapBuffer.fileName; - LOG(INFO) << "\tmethodName: " << mapBuffer.methodName; - LOG(INFO) << "\tlineNumber: " << std::to_string(mapBuffer.lineNumber); - LOG(INFO) << "\tcolumn: " << std::to_string(mapBuffer.columnNumber); - } - }; + auto onJsError = [](const JsErrorHandler::ParsedError& errorMap) noexcept { + LOG(INFO) << "[jsErrorHandlingFunc called]"; + LOG(INFO) << "message: " << errorMap.message; + LOG(INFO) << "exceptionId: " << std::to_string(errorMap.exceptionId); + LOG(INFO) << "isFatal: " + << std::to_string(static_cast(errorMap.isFatal)); + auto frames = errorMap.frames; + for (const auto& mapBuffer : frames) { + LOG(INFO) << "[Frame]" << std::endl << "\tfile: " << mapBuffer.fileName; + LOG(INFO) << "\tmethodName: " << mapBuffer.methodName; + LOG(INFO) << "\tlineNumber: " << std::to_string(mapBuffer.lineNumber); + LOG(INFO) << "\tcolumn: " << std::to_string(mapBuffer.columnNumber); + } + }; auto jsRuntimeFactory = std::make_unique(); std::unique_ptr runtime_ = @@ -77,7 +75,7 @@ void ReactInstanceIntegrationTest::SetUp() { std::move(runtime_), messageQueueThread, timerManager, - std::move(jsErrorHandlingFunc), + std::move(onJsError), hostTargetIfModernCDP == nullptr ? nullptr : hostTargetIfModernCDP.get()); timerManager->setRuntimeExecutor(instance->getBufferedRuntimeExecutor()); diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 440e9f5de576..5014bdb1a14d 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -31,13 +31,12 @@ ReactInstance::ReactInstance( std::unique_ptr runtime, std::shared_ptr jsMessageQueueThread, std::shared_ptr timerManager, - JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc, + JsErrorHandler::OnJsError onJsError, jsinspector_modern::HostTarget* parentInspectorTarget) : runtime_(std::move(runtime)), jsMessageQueueThread_(jsMessageQueueThread), timerManager_(std::move(timerManager)), - jsErrorHandler_( - std::make_shared(std::move(jsErrorHandlingFunc))), + jsErrorHandler_(std::make_shared(std::move(onJsError))), parentInspectorTarget_(parentInspectorTarget) { RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), weakTimerManager = diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h index d44b539bd7eb..2a7ccea628bb 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.h +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.h @@ -34,7 +34,7 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate { std::unique_ptr runtime, std::shared_ptr jsMessageQueueThread, std::shared_ptr timerManager, - JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc, + JsErrorHandler::OnJsError onJsError, jsinspector_modern::HostTarget* parentInspectorTarget = nullptr); RuntimeExecutor getUnbufferedRuntimeExecutor() noexcept; diff --git a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm index 31a18620bd89..957ab73cceb6 100644 --- a/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm +++ b/packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm @@ -242,14 +242,14 @@ - (void)_start objCTimerRegistryRawPtr->setTimerManager(timerManager); __weak __typeof(self) weakSelf = self; - auto jsErrorHandlingFunc = [=](const JsErrorHandler::ParsedError &error) { [weakSelf _handleJSError:error]; }; + auto onJsError = [=](const JsErrorHandler::ParsedError &error) { [weakSelf _handleJSError:error]; }; // Create the React Instance _reactInstance = std::make_unique( _jsRuntimeFactory->createJSRuntime(_jsThreadManager.jsMessageThread), _jsThreadManager.jsMessageThread, timerManager, - jsErrorHandlingFunc, + onJsError, _parentInspectorTarget); _valid = true; diff --git a/packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp b/packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp index 42c3d32f4076..db2ec9255bf9 100644 --- a/packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp +++ b/packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp @@ -123,15 +123,15 @@ class ReactInstanceTest : public ::testing::Test { auto mockRegistry = std::make_unique(); mockRegistry_ = mockRegistry.get(); timerManager_ = std::make_shared(std::move(mockRegistry)); - auto jsErrorHandlingFunc = - [](const JsErrorHandler::ParsedError& errorMap) noexcept { - // Do nothing - }; + auto onJsError = [](const JsErrorHandler::ParsedError& errorMap) noexcept { + // Do nothing + }; + instance_ = std::make_unique( std::move(runtime), messageQueueThread_, timerManager_, - std::move(jsErrorHandlingFunc)); + std::move(onJsError)); timerManager_->setRuntimeExecutor(instance_->getBufferedRuntimeExecutor()); // Install a C++ error handler