Refactor: JsErrorHandler: Rename JsErrorHandlingFunc -> OnJsError (#4…

…3985) Summary: Pull Request resolved: #43985 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 fbshipit-source-id: 4d20bc984e6633aeac6193b9276a88d76961df2c
facebook · Apr 9, 2024 · 2e3f226 · 2e3f226
1 parent c041b9f
commit 2e3f226
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 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
@@ -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
@@ -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
@@ -27,16 +27,16 @@ class JsErrorHandler {
     bool isFatal;
   };
 
-  using JsErrorHandlingFunc = std::function<void(const ParsedError& error)>;
+  using OnJsError = std::function<void(const ParsedError& error)>;
 
-  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
@@ -34,22 +34,20 @@ void ReactInstanceIntegrationTest::SetUp() {
   auto timerManager =
       std::make_shared<react::TimerManager>(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<int>(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<int>(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<react::HermesInstance>();
   std::unique_ptr<react::JSRuntime> 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
@@ -31,13 +31,12 @@ ReactInstance::ReactInstance(
     std::unique_ptr<JSRuntime> runtime,
     std::shared_ptr<MessageQueueThread> jsMessageQueueThread,
     std::shared_ptr<TimerManager> 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<JsErrorHandler>(std::move(jsErrorHandlingFunc))),
+      jsErrorHandler_(std::make_shared<JsErrorHandler>(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
@@ -34,7 +34,7 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate {
       std::unique_ptr<JSRuntime> runtime,
       std::shared_ptr<MessageQueueThread> jsMessageQueueThread,
       std::shared_ptr<TimerManager> 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
@@ -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<ReactInstance>(
       _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
@@ -123,15 +123,15 @@ class ReactInstanceTest : public ::testing::Test {
     auto mockRegistry = std::make_unique<MockTimerRegistry>();
     mockRegistry_ = mockRegistry.get();
     timerManager_ = std::make_shared<TimerManager>(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<ReactInstance>(
         std::move(runtime),
         messageQueueThread_,
         timerManager_,
-        std::move(jsErrorHandlingFunc));
+        std::move(onJsError));
     timerManager_->setRuntimeExecutor(instance_->getBufferedRuntimeExecutor());
 
     // Install a C++ error handler