Refactor: JsErrorHandler: Rename handleJsError to handleFatalError

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

@@ -90,17 +90,23 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
 
 JsErrorHandler::JsErrorHandler(
     JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc)
-    : _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)){
+    : _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)),
+      _hasHandledFatalError(false){
 
       };
 
 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.
-  ParsedError parsedError = parseErrorStack(error, isFatal, false);
+  _hasHandledFatalError = true;
+  ParsedError parsedError = parseErrorStack(error, true, false);
   _jsErrorHandlingFunc(parsedError);
 }
 
+bool JsErrorHandler::hasHandledFatalError() {
+  return _hasHandledFatalError;
+}
+
 } // namespace facebook::react

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h

@@ -32,10 +32,12 @@ class JsErrorHandler {
   explicit JsErrorHandler(JsErrorHandlingFunc jsErrorHandlingFunc);
   ~JsErrorHandler();
 
-  void handleJsError(const jsi::JSError& error, bool isFatal);
+  void handleFatalError(const jsi::JSError& error);
+  bool hasHandledFatalError();
 
  private:
   JsErrorHandlingFunc _jsErrorHandlingFunc;
+  bool _hasHandledFatalError;
 };
 
 } // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp

@@ -13,7 +13,6 @@
 #include <react/featureflags/ReactNativeFeatureFlags.h>
 #include <react/renderer/debug/SystraceSection.h>
 #include <utility>
-#include "ErrorUtils.h"
 
 namespace facebook::react {
 

packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp

@@ -8,10 +8,10 @@
 #include "RuntimeScheduler_Legacy.h"
 #include "SchedulerPriorityUtils.h"
 
+#include <cxxreact/ErrorUtils.h>
 #include <react/renderer/consistency/ScopedShadowTreeRevisionLock.h>
 #include <react/renderer/debug/SystraceSection.h>
 #include <utility>
-#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;

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<RuntimeSchedulerTimePoint()> now =
-          RuntimeSchedulerClock::now);
+      std::function<RuntimeSchedulerTimePoint()> now);
 
   /*
    * Not copyable.

packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp

@@ -14,7 +14,6 @@
 #include <react/renderer/debug/SystraceSection.h>
 #include <react/utils/OnScopeExit.h>
 #include <utility>
-#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;

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<RuntimeSchedulerTimePoint()> now =
-          RuntimeSchedulerClock::now);
+      std::function<RuntimeSchedulerTimePoint()> now);
 
   /*
    * Not copyable.

packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp

@@ -36,25 +36,25 @@ ReactInstance::ReactInstance(
     : runtime_(std::move(runtime)),
       jsMessageQueueThread_(jsMessageQueueThread),
       timerManager_(std::move(timerManager)),
-      jsErrorHandler_(std::move(jsErrorHandlingFunc)),
-      hasFatalJsError_(std::make_shared<bool>(false)),
+      jsErrorHandler_(
+          std::make_shared<JsErrorHandler>(std::move(jsErrorHandlingFunc))),
       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<bool> 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;
     }
 
@@ -163,6 +163,8 @@ RuntimeExecutor ReactInstance::getBufferedRuntimeExecutor() noexcept {
   };
 }
 
+// TODO(T184010230): Should the RuntimeScheduler returned from this method be
+// buffered?
 std::shared_ptr<RuntimeScheduler>
 ReactInstance::getRuntimeScheduler() noexcept {
   return runtimeScheduler_;
@@ -220,9 +222,7 @@ void ReactInstance::loadScript(
             strongBufferedRuntimeExecuter->flush();
           }
         } catch (jsi::JSError& error) {
-          // Handle uncaught JS errors during loading JS bundle
-          *hasFatalJsError_ = true;
-          this->jsErrorHandler_.handleJsError(error, true);
+          jsErrorHandler_->handleFatalError(error);
         }
       });
 }

packages/react-native/ReactCommon/react/runtime/ReactInstance.h

@@ -80,10 +80,7 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate {
   std::shared_ptr<TimerManager> timerManager_;
   std::unordered_map<std::string, std::shared_ptr<CallableModule>> modules_;
   std::shared_ptr<RuntimeScheduler> runtimeScheduler_;
-  JsErrorHandler jsErrorHandler_;
-
-  // Whether there are errors caught during bundle loading
-  std::shared_ptr<bool> hasFatalJsError_;
+  std::shared_ptr<JsErrorHandler> jsErrorHandler_;
 
   jsinspector_modern::InstanceTarget* inspectorTarget_{nullptr};
   jsinspector_modern::RuntimeTarget* runtimeInspectorTarget_{nullptr};