Continue running microtasks when parent task throws

Summary:
Changelog: [General][Fixed] Fixed LogBox not showing correctly on the New Architecture

We found an incorrect behavior in the event loop, where an error in a task would prevent its microtasks from running. This isn't spec compliant and should be fixed.

This caused LogBox to not work correctly, as error reporting is implemented via microtasks that would never execute.

Reviewed By: sammy-SC

Differential Revision: D58010521

fbshipit-source-id: 7901c5d6e83fb63af148e12ad6c32be490a3999d

Author: rubennorte

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

@@ -229,21 +229,17 @@ void RuntimeScheduler_Modern::startWorkLoop(
 
   auto previousPriority = currentPriority_;
 
-  try {
-    while (syncTaskRequests_ == 0) {
-      auto currentTime = now_();
-      auto topPriorityTask = selectTask(currentTime, onlyExpired);
-
-      if (!topPriorityTask) {
-        // No pending work to do.
-        // Events will restart the loop when necessary.
-        break;
-      }
-
-      executeTask(runtime, *topPriorityTask, currentTime);
+  while (syncTaskRequests_ == 0) {
+    auto currentTime = now_();
+    auto topPriorityTask = selectTask(currentTime, onlyExpired);
+
+    if (!topPriorityTask) {
+      // No pending work to do.
+      // Events will restart the loop when necessary.
+      break;
     }
-  } catch (jsi::JSError& error) {
-    handleJSError(runtime, error, true);
+
+    executeTask(runtime, *topPriorityTask, currentTime);
   }
 
   currentPriority_ = previousPriority;
@@ -330,12 +326,16 @@ void RuntimeScheduler_Modern::executeMacrotask(
     bool didUserCallbackTimeout) const {
   SystraceSection s("RuntimeScheduler::executeMacrotask");
 
-  auto result = task.execute(runtime, didUserCallbackTimeout);
+  try {
+    auto result = task.execute(runtime, didUserCallbackTimeout);
 
-  if (result.isObject() && result.getObject(runtime).isFunction(runtime)) {
-    // If the task returned a continuation callback, we re-assign it to the task
-    // and keep the task in the queue.
-    task.callback = result.getObject(runtime).getFunction(runtime);
+    if (result.isObject() && result.getObject(runtime).isFunction(runtime)) {
+      // If the task returned a continuation callback, we re-assign it to the
+      // task and keep the task in the queue.
+      task.callback = result.getObject(runtime).getFunction(runtime);
+    }
+  } catch (jsi::JSError& error) {
+    handleJSError(runtime, error, true);
   }
 }
 

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

@@ -32,20 +32,22 @@ jsi::Value Task::execute(jsi::Runtime& runtime, bool didUserCallbackTimeout) {
     return result;
   }
 
-  auto& cbVal = callback.value();
+  // We get the value of the callback and reset it immediately to avoid it being
+  // called more than once (including when the callback throws).
+  auto originalCallback = std::move(*callback);
+  callback.reset();
 
-  if (cbVal.index() == 0) {
+  if (originalCallback.index() == 0) {
     // Callback in JavaScript is expecting a single bool parameter.
     // React team plans to remove it in the future when a scheduler bug on web
     // is resolved.
-    result =
-        std::get<jsi::Function>(cbVal).call(runtime, {didUserCallbackTimeout});
+    result = std::get<jsi::Function>(originalCallback)
+                 .call(runtime, {didUserCallbackTimeout});
   } else {
     // Calling a raw callback
-    std::get<RawCallback>(cbVal)(runtime);
+    std::get<RawCallback>(originalCallback)(runtime);
   }
-  // Destroying callback to prevent calling it twice.
-  callback.reset();
+
   return result;
 }
 

packages/react-native/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp

@@ -1021,6 +1021,52 @@ TEST_P(RuntimeSchedulerTest, modernTwoThreadsRequestAccessToTheRuntime) {
   EXPECT_EQ(stubQueue_->size(), 0);
 }
 
+TEST_P(RuntimeSchedulerTest, errorInTaskShouldNotStopMicrotasks) {
+  // Only for modern runtime scheduler
+  if (!GetParam()) {
+    return;
+  }
+
+  auto microtaskRan = false;
+  auto taskRan = false;
+
+  auto callback = createHostFunctionFromLambda([&](bool /* unused */) {
+    taskRan = true;
+
+    auto microtaskCallback = jsi::Function::createFromHostFunction(
+        *runtime_,
+        jsi::PropNameID::forUtf8(*runtime_, "microtask1"),
+        3,
+        [&](jsi::Runtime& /*unused*/,
+            const jsi::Value& /*unused*/,
+            const jsi::Value* /*arguments*/,
+            size_t /*unused*/) -> jsi::Value {
+          microtaskRan = true;
+          return jsi::Value::undefined();
+        });
+
+    runtime_->queueMicrotask(microtaskCallback);
+
+    throw jsi::JSError(*runtime_, "Test error");
+
+    return jsi::Value::undefined();
+  });
+
+  runtimeScheduler_->scheduleTask(
+      SchedulerPriority::NormalPriority, std::move(callback));
+
+  EXPECT_EQ(taskRan, false);
+  EXPECT_EQ(microtaskRan, false);
+  EXPECT_EQ(stubQueue_->size(), 1);
+
+  stubQueue_->tick();
+
+  EXPECT_EQ(taskRan, 1);
+  EXPECT_EQ(microtaskRan, 1);
+  EXPECT_EQ(stubQueue_->size(), 0);
+  EXPECT_EQ(stubErrorUtils_->getReportFatalCallCount(), 1);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     UseModernRuntimeScheduler,
     RuntimeSchedulerTest,