[Scheduler] Re-throw unhandled errors

Because `postTask` returns a promise, errors inside a `postTask`
callback result in the promise being rejected.

If we don't catch those errors, then the browser will report an
"Unhandled promise rejection" error. This is a confusing message to see
in the console, because the fact that `postTask` is a promise-based API
is an implementation detail from the perspective of the developer.
"Promise rejection" is a red herring.

On the other hand, if we do catch those errors, then we need to report
the error to the user in some other way.

What we really want is the default error reporting behavior that a
normal, non-Promise browser event gets.

So, we'll re-throw inside `setTimeout`.

Author: acdlite

packages/scheduler/src/SchedulerPostTask.js

@@ -39,6 +39,7 @@ export {
 
 // Capture local references to native APIs, in case a polyfill overrides them.
 const perf = window.performance;
+const setTimeout = window.setTimeout;
 
 // Use experimental Chrome Scheduler postTask API.
 const scheduler = global.scheduler;
@@ -112,7 +113,7 @@ export function unstable_scheduleCallback<T>(
       runTask.bind(null, priorityLevel, postTaskPriority, node, callback),
       postTaskOptions,
     )
-    .catch(handlePostTaskError);
+    .catch(handleAbortError);
 
   return node;
 }
@@ -124,56 +125,49 @@ function runTask<T>(
   callback: SchedulerCallback<T>,
 ) {
   deadline = getCurrentTime() + yieldInterval;
+  let result;
   try {
     currentPriorityLevel_DEPRECATED = priorityLevel;
     const didTimeout_DEPRECATED = false;
-    const result = callback(didTimeout_DEPRECATED);
-    if (typeof result === 'function') {
-      // Assume this is a continuation
-      const continuation: SchedulerCallback<T> = (result: any);
-      const continuationController = new TaskController();
-      const continuationOptions = {
-        priority: postTaskPriority,
-        signal: continuationController.signal,
-      };
-      // Update the original callback node's controller, since even though we're
-      // posting a new task, conceptually it's the same one.
-      node._controller = continuationController;
-      scheduler
-        .postTask(
-          runTask.bind(
-            null,
-            priorityLevel,
-            postTaskPriority,
-            node,
-            continuation,
-          ),
-          continuationOptions,
-        )
-        .catch(handlePostTaskError);
-    }
+    result = callback(didTimeout_DEPRECATED);
+  } catch (error) {
+    // We're inside a promise. If we don't handle this error, then it will
+    // trigger an "Unhandled promise rejection" error. We don't want that, but
+    // we do want the default error reporting behavior that normal (non-Promise)
+    // tasks get for unhandled errors.
+    //
+    // So we'll re-throw the error inside a regular browser task.
+    setTimeout(() => {
+      throw error;
+    });
   } finally {
     currentPriorityLevel_DEPRECATED = NormalPriority;
   }
+  if (typeof result === 'function') {
+    // Assume this is a continuation
+    const continuation: SchedulerCallback<T> = (result: any);
+    const continuationController = new TaskController();
+    const continuationOptions = {
+      priority: postTaskPriority,
+      signal: continuationController.signal,
+    };
+    // Update the original callback node's controller, since even though we're
+    // posting a new task, conceptually it's the same one.
+    node._controller = continuationController;
+    scheduler
+      .postTask(
+        runTask.bind(null, priorityLevel, postTaskPriority, node, continuation),
+        continuationOptions,
+      )
+      .catch(handleAbortError);
+  }
 }
 
-function handlePostTaskError(error) {
-  // This error is either a user error thrown by a callback, or an AbortError
-  // as a result of a cancellation.
-  //
-  // User errors trigger a global `error` event even if we don't rethrow them.
-  // In fact, if we do rethrow them, they'll get reported to the console twice.
-  // I'm not entirely sure the current `postTask` spec makes sense here. If I
-  // catch a `postTask` call, it shouldn't trigger a global error.
-  //
+function handleAbortError(error) {
   // Abort errors are an implementation detail. We don't expose the
   // TaskController to the user, nor do we expose the promise that is returned
-  // from `postTask`. So we shouldn't rethrow those, either, since there's no
-  // way to handle them. (If we did return the promise to the user, then it
-  // should be up to them to handle the AbortError.)
-  //
-  // In either case, we can suppress the error, barring changes to the spec
-  // or the Scheduler API.
+  // from `postTask`. So we should suppress them, since there's no way for the
+  // user to handle them.
 }
 
 export function unstable_cancelCallback(node: CallbackNode) {

packages/scheduler/src/__tests__/SchedulerPostTask-test.js

@@ -70,6 +70,15 @@ describe('SchedulerPostTask', () => {
       },
     };
 
+    // Note: setTimeout is used to report errors and nothing else
+    window.setTimeout = cb => {
+      try {
+        cb();
+      } catch (error) {
+        runtime.log(`Error: ${error.message}`);
+      }
+    };
+
     // Mock browser scheduler.
     const scheduler = {};
     global.scheduler = scheduler;
@@ -116,16 +125,10 @@ describe('SchedulerPostTask', () => {
       // delete the continuation task.
       const prevTaskQueue = taskQueue;
       taskQueue = new Map();
-      for (const [, {id, callback, resolve, reject}] of prevTaskQueue) {
-        try {
-          log(`Task ${id} Fired`);
-          callback(false);
-          resolve();
-        } catch (error) {
-          log(`Task ${id} errored [${error.message}]`);
-          reject(error);
-          continue;
-        }
+      for (const [, {id, callback, resolve}] of prevTaskQueue) {
+        log(`Task ${id} Fired`);
+        callback(false);
+        resolve();
       }
     }
     function log(val) {
@@ -219,12 +222,7 @@ describe('SchedulerPostTask', () => {
       'Post Task 1 [user-visible]',
     ]);
     runtime.flushTasks();
-    runtime.assertLog([
-      'Task 0 Fired',
-      'Task 0 errored [Oops!]',
-      'Task 1 Fired',
-      'Yay',
-    ]);
+    runtime.assertLog(['Task 0 Fired', 'Error: Oops!', 'Task 1 Fired', 'Yay']);
   });
 
   it('schedule new task after queue has emptied', () => {