diff --git a/packages/scheduler/src/SchedulerPostTask.js b/packages/scheduler/src/SchedulerPostTask.js index b42f2114be2c3..5d986d5127cb0 100644 --- a/packages/scheduler/src/SchedulerPostTask.js +++ b/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( runTask.bind(null, priorityLevel, postTaskPriority, node, callback), postTaskOptions, ) - .catch(handlePostTaskError); + .catch(handleAbortError); return node; } @@ -150,30 +151,28 @@ function runTask( ), continuationOptions, ) - .catch(handlePostTaskError); + .catch(handleAbortError); } + } catch (error) { + // We're inside a `postTask` 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; } } -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) { diff --git a/packages/scheduler/src/__tests__/SchedulerPostTask-test.js b/packages/scheduler/src/__tests__/SchedulerPostTask-test.js index 8ad6228bb8a91..19b6e330b82fb 100644 --- a/packages/scheduler/src/__tests__/SchedulerPostTask-test.js +++ b/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', () => {