From 2139b7f4f1a4af214d35dae9a365488624ee969e Mon Sep 17 00:00:00 2001
From: Pieter De Baets <pieterdb@meta.com>
Date: Thu, 12 Dec 2024 04:33:15 -0800
Subject: [PATCH] Error when using runTask in a re-entrant way

Summary:
Recursively calling runTask is not supported, as the inner call will no-op since we're already executing the eventloop.

Currently errors are not correctly propagated, but this at least makes it so that we don't attempt to schedule the task either, which could lead to incorrect assumptions being made.

Changelog: [internal]

Reviewed By: rubennorte

Differential Revision: D67107664
---
 .../src/__tests__/Fantom-itest.js             | 27 +++++++++++++++++++
 packages/react-native-fantom/src/index.js     | 16 ++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/packages/react-native-fantom/src/__tests__/Fantom-itest.js b/packages/react-native-fantom/src/__tests__/Fantom-itest.js
index e16fdeffc33408..d4a2f97216bd82 100644
--- a/packages/react-native-fantom/src/__tests__/Fantom-itest.js
+++ b/packages/react-native-fantom/src/__tests__/Fantom-itest.js
@@ -10,6 +10,7 @@
  */
 
 import 'react-native/Libraries/Core/InitializeCore';
+
 import {createRoot, runTask} from '..';
 import * as React from 'react';
 import {Text, View} from 'react-native';
@@ -70,6 +71,32 @@ describe('Fantom', () => {
 
       expect(completed).toBe(true);
     });
+
+    // TODO: when error handling is fixed, this should verify using `toThrow`
+    it('should throw when running a task inside another task', () => {
+      let lastCallbackExecuted = 0;
+      runTask(() => {
+        lastCallbackExecuted = 1;
+        runTask(() => {
+          lastCallbackExecuted = 2;
+          throw new Error('Recursive runTask should be unreachable');
+        });
+      });
+      expect(lastCallbackExecuted).toBe(1);
+
+      runTask(() => {
+        queueMicrotask(() => {
+          lastCallbackExecuted = 3;
+          runTask(() => {
+            lastCallbackExecuted = 4;
+            throw new Error(
+              'Recursive runTask from micro-task should be unreachable',
+            );
+          });
+        });
+      });
+      expect(lastCallbackExecuted).toBe(3);
+    });
   });
 
   describe('getRenderedOutput', () => {
diff --git a/packages/react-native-fantom/src/index.js b/packages/react-native-fantom/src/index.js
index 606be08ad430de..0c3b79f7dbcd3b 100644
--- a/packages/react-native-fantom/src/index.js
+++ b/packages/react-native-fantom/src/index.js
@@ -60,17 +60,31 @@ class Root {
   // TODO: add an API to check if all surfaces were deallocated when tests are finished.
 }
 
+let flushingQueue = false;
+
 /*
  * Runs a task on on the event loop. To be used together with root.render.
  *
  * React must run inside of event loop to ensure scheduling environment is closer to production.
  */
 export function runTask(task: () => void | Promise<void>) {
+  if (flushingQueue) {
+    throw new Error(
+      'Nested runTask calls are not allowed. If you want to schedule a task from inside another task, use scheduleTask instead.',
+    );
+  }
+
   nativeRuntimeScheduler.unstable_scheduleCallback(
     schedulerPriorityImmediate,
     task,
   );
-  global.$$JSTesterModuleName$$.flushMessageQueue();
+
+  try {
+    flushingQueue = true;
+    global.$$JSTesterModuleName$$.flushMessageQueue();
+  } finally {
+    flushingQueue = false;
+  }
 }
 
 // TODO: Add option to define surface props and pass it to startSurface