From 013b7ad117834cbb99b4fc0a3d08fdb8622597c9 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 23 Sep 2019 11:23:28 -0700 Subject: [PATCH] [suspense][error handling] Inline renderRoot and fix error handling bug (#16801) * Outline push/pop logic in `renderRoot` I want to get rid of the the `isSync` argument to `renderRoot`, and instead use separate functions for concurrent and synchronous render. As a first step, this extracts the push/pop logic that happens before and after the render phase into helper functions. * Extract `catch` block into helper function Similar to previous commit. Extract error handling logic into a separate function so it can be reused. * Fork `renderRoot` for sync and concurrent Removes `isSync` argument in favor of separate functions. * Extra "root completion" logic to separate function Moving this out to avoid an accidental early return, which would bypass the call to `ensureRootIsScheduled` and freeze the UI. * Inline `renderRoot` Inlines `renderRoot` into `performConcurrentWorkOnRoot` and `performSyncWorkOnRoot`. This lets me remove the `isSync` argument and also get rid of a redundant try-catch wrapper. * [suspense][error handling] Add failing unit test Covers an edge case where an error is thrown inside the complete phase of a component that is in the return path of a component that suspends. The second error should also be handled (i.e. able to be captured by an error boundary. The test is currently failing because there's a call to `completeUnitOfWork` inside the main render phase `catch` block. That call is not itself wrapped in try-catch, so anything that throws is treated as a fatal/unhandled error. I believe this bug is only observable if something in the host config throws; and, only in legacy mode, because in concurrent/batched mode, `completeUnitOfWork` on fiber that throws follows the "unwind" path only, not the "complete" path, and the "unwind" path does not call any host config methods. * [scheduler][profiler] Start time of delayed tasks Fixes a bug in the Scheduler profiler where the start time of a delayed tasks is always 0. * Remove ad hoc `throw` Fatal errors (errors that are not captured by an error boundary) are currently rethrown from directly inside the render phase's `catch` block. This is a refactor hazard because the code in this branch has to mirror the code that happens at the end of the function, when exiting the render phase in the normal case. This commit moves the throw to the end, using a new root exit status. * Handle errors that occur on unwind --- .../src/ReactFiberWorkLoop.js | 871 ++++++++++-------- ...tSuspenseWithNoopRenderer-test.internal.js | 34 + scripts/error-codes/codes.json | 3 +- 3 files changed, 504 insertions(+), 404 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index be45086f2f7f..50b3d2aaefac 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -200,13 +200,14 @@ const LegacyUnbatchedContext = /* */ 0b001000; const RenderContext = /* */ 0b010000; const CommitContext = /* */ 0b100000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootIncomplete = 0; -const RootErrored = 1; -const RootSuspended = 2; -const RootSuspendedWithDelay = 3; -const RootCompleted = 4; -const RootLocked = 5; +const RootFatalErrored = 1; +const RootErrored = 2; +const RootSuspended = 3; +const RootSuspendedWithDelay = 4; +const RootCompleted = 5; +const RootLocked = 6; export type Thenable = { then(resolve: () => mixed, reject?: () => mixed): Thenable | void, @@ -225,6 +226,8 @@ let workInProgress: Fiber | null = null; let renderExpirationTime: ExpirationTime = NoWork; // Whether to root completed, errored, suspended, etc. let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; +// A fatal error, if one is thrown +let workInProgressRootFatalError: mixed = null; // Most recent event time among processed updates during this render. // This is conceptually a time stamp but expressed in terms of an ExpirationTime // because we deal mostly with expiration times in the hot path, so this avoids @@ -655,8 +658,55 @@ function performConcurrentWorkOnRoot(root, didTimeout) { const expirationTime = getNextRootExpirationTimeToWorkOn(root); if (expirationTime !== NoWork) { const originalCallbackNode = root.callbackNode; - try { - renderRoot(root, expirationTime, false); + invariant( + (executionContext & (RenderContext | CommitContext)) === NoContext, + 'Should not already be working.', + ); + + flushPassiveEffects(); + + // If the root or expiration time have changed, throw out the existing stack + // and prepare a fresh one. Otherwise we'll continue where we left off. + if ( + root !== workInProgressRoot || + expirationTime !== renderExpirationTime + ) { + prepareFreshStack(root, expirationTime); + startWorkOnPendingInteractions(root, expirationTime); + } + + // If we have a work-in-progress fiber, it means there's still work to do + // in this root. + if (workInProgress !== null) { + const prevExecutionContext = executionContext; + executionContext |= RenderContext; + const prevDispatcher = pushDispatcher(root); + const prevInteractions = pushInteractions(root); + startWorkLoopTimer(workInProgress); + do { + try { + workLoopConcurrent(); + break; + } catch (thrownValue) { + handleError(root, thrownValue); + } + } while (true); + resetContextDependencies(); + executionContext = prevExecutionContext; + popDispatcher(prevDispatcher); + if (enableSchedulerTracing) { + popInteractions(((prevInteractions: any): Set)); + } + + if (workInProgressRootExitStatus === RootFatalErrored) { + const fatalError = workInProgressRootFatalError; + stopInterruptedWorkLoopTimer(); + prepareFreshStack(root, expirationTime); + markRootSuspendedAtTime(root, expirationTime); + ensureRootIsScheduled(root); + throw fatalError; + } + if (workInProgress !== null) { // There's still work left over. Exit without committing. stopInterruptedWorkLoopTimer(); @@ -668,281 +718,274 @@ function performConcurrentWorkOnRoot(root, didTimeout) { const finishedWork: Fiber = ((root.finishedWork = root.current.alternate): any); root.finishedExpirationTime = expirationTime; - resolveLocksOnRoot(root, expirationTime); + finishConcurrentRender( + root, + finishedWork, + workInProgressRootExitStatus, + expirationTime, + ); + } + + ensureRootIsScheduled(root); + if (root.callbackNode === originalCallbackNode) { + // The task node scheduled for this root is the same one that's + // currently executed. Need to return a continuation. + return performConcurrentWorkOnRoot.bind(null, root); + } + } + } + return null; +} - // Set this to null to indicate there's no in-progress render. - workInProgressRoot = null; +function finishConcurrentRender( + root, + finishedWork, + exitStatus, + expirationTime, +) { + // Set this to null to indicate there's no in-progress render. + workInProgressRoot = null; - switch (workInProgressRootExitStatus) { - case RootIncomplete: { - invariant(false, 'Should have a work-in-progress.'); - } - // Flow knows about invariant, so it complains if I add a break - // statement, but eslint doesn't know about invariant, so it complains - // if I do. eslint-disable-next-line no-fallthrough - case RootErrored: { - if (expirationTime !== Idle) { - // If this was an async render, the error may have happened due to - // a mutation in a concurrent event. Try rendering one more time, - // synchronously, to see if the error goes away. If there are - // lower priority updates, let's include those, too, in case they - // fix the inconsistency. Render at Idle to include all updates. - markRootExpiredAtTime(root, Idle); + switch (exitStatus) { + case RootIncomplete: + case RootFatalErrored: { + invariant(false, 'Root did not complete. This is a bug in React.'); + } + // Flow knows about invariant, so it complains if I add a break + // statement, but eslint doesn't know about invariant, so it complains + // if I do. eslint-disable-next-line no-fallthrough + case RootErrored: { + if (expirationTime !== Idle) { + // If this was an async render, the error may have happened due to + // a mutation in a concurrent event. Try rendering one more time, + // synchronously, to see if the error goes away. If there are + // lower priority updates, let's include those, too, in case they + // fix the inconsistency. Render at Idle to include all updates. + markRootExpiredAtTime(root, Idle); + break; + } + // Commit the root in its errored state. + commitRoot(root); + break; + } + case RootSuspended: { + markRootSuspendedAtTime(root, expirationTime); + const lastSuspendedTime = root.lastSuspendedTime; + if (expirationTime === lastSuspendedTime) { + root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); + } + flushSuspensePriorityWarningInDEV(); + + // We have an acceptable loading state. We need to figure out if we + // should immediately commit it or wait a bit. + + // If we have processed new updates during this render, we may now + // have a new loading state ready. We want to ensure that we commit + // that as soon as possible. + const hasNotProcessedNewUpdates = + workInProgressRootLatestProcessedExpirationTime === Sync; + if ( + hasNotProcessedNewUpdates && + // do not delay if we're inside an act() scope + !( + __DEV__ && + flushSuspenseFallbacksInTests && + IsThisRendererActing.current + ) + ) { + // If we have not processed any new updates during this pass, then + // this is either a retry of an existing fallback state or a + // hidden tree. Hidden trees shouldn't be batched with other work + // and after that's fixed it can only be a retry. We're going to + // throttle committing retries so that we don't show too many + // loading states too quickly. + let msUntilTimeout = + globalMostRecentFallbackTime + FALLBACK_THROTTLE_MS - now(); + // Don't bother with a very short suspense time. + if (msUntilTimeout > 10) { + if (workInProgressRootHasPendingPing) { + const lastPingedTime = root.lastPingedTime; + if (lastPingedTime === NoWork || lastPingedTime >= expirationTime) { + // This render was pinged but we didn't get to restart + // earlier so try restarting now instead. + root.lastPingedTime = expirationTime; + prepareFreshStack(root, expirationTime); break; } - // Commit the root in its errored state. - commitRoot(root); + } + + const nextTime = getNextRootExpirationTimeToWorkOn(root); + if (nextTime !== NoWork && nextTime !== expirationTime) { + // There's additional work on this root. + break; + } + if ( + lastSuspendedTime !== NoWork && + lastSuspendedTime !== expirationTime + ) { + // We should prefer to render the fallback of at the last + // suspended level. Ping the last suspended level to try + // rendering it again. + root.lastPingedTime = lastSuspendedTime; break; } - case RootSuspended: { - markRootSuspendedAtTime(root, expirationTime); - const lastSuspendedTime = root.lastSuspendedTime; - if (expirationTime === lastSuspendedTime) { - root.nextKnownPendingLevel = getRemainingExpirationTime( - finishedWork, - ); - } - flushSuspensePriorityWarningInDEV(); - - // We have an acceptable loading state. We need to figure out if we - // should immediately commit it or wait a bit. - - // If we have processed new updates during this render, we may now - // have a new loading state ready. We want to ensure that we commit - // that as soon as possible. - const hasNotProcessedNewUpdates = - workInProgressRootLatestProcessedExpirationTime === Sync; - if ( - hasNotProcessedNewUpdates && - // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) - ) { - // If we have not processed any new updates during this pass, then - // this is either a retry of an existing fallback state or a - // hidden tree. Hidden trees shouldn't be batched with other work - // and after that's fixed it can only be a retry. We're going to - // throttle committing retries so that we don't show too many - // loading states too quickly. - let msUntilTimeout = - globalMostRecentFallbackTime + FALLBACK_THROTTLE_MS - now(); - // Don't bother with a very short suspense time. - if (msUntilTimeout > 10) { - if (workInProgressRootHasPendingPing) { - const lastPingedTime = root.lastPingedTime; - if ( - lastPingedTime === NoWork || - lastPingedTime >= expirationTime - ) { - // This render was pinged but we didn't get to restart - // earlier so try restarting now instead. - root.lastPingedTime = expirationTime; - prepareFreshStack(root, expirationTime); - break; - } - } - const nextTime = getNextRootExpirationTimeToWorkOn(root); - if (nextTime !== NoWork && nextTime !== expirationTime) { - // There's additional work on this root. - break; - } - if ( - lastSuspendedTime !== NoWork && - lastSuspendedTime !== expirationTime - ) { - // We should prefer to render the fallback of at the last - // suspended level. Ping the last suspended level to try - // rendering it again. - root.lastPingedTime = lastSuspendedTime; - break; - } + // The render is suspended, it hasn't timed out, and there's no + // lower priority work to do. Instead of committing the fallback + // immediately, wait for more data to arrive. + root.timeoutHandle = scheduleTimeout( + commitRoot.bind(null, root), + msUntilTimeout, + ); + break; + } + } + // The work expired. Commit immediately. + commitRoot(root); + break; + } + case RootSuspendedWithDelay: { + markRootSuspendedAtTime(root, expirationTime); + const lastSuspendedTime = root.lastSuspendedTime; + if (expirationTime === lastSuspendedTime) { + root.nextKnownPendingLevel = getRemainingExpirationTime(finishedWork); + } + flushSuspensePriorityWarningInDEV(); - // The render is suspended, it hasn't timed out, and there's no - // lower priority work to do. Instead of committing the fallback - // immediately, wait for more data to arrive. - root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), - msUntilTimeout, - ); - break; - } - } - // The work expired. Commit immediately. - commitRoot(root); + if ( + // do not delay if we're inside an act() scope + !( + __DEV__ && + flushSuspenseFallbacksInTests && + IsThisRendererActing.current + ) + ) { + // We're suspended in a state that should be avoided. We'll try to + // avoid committing it for as long as the timeouts let us. + if (workInProgressRootHasPendingPing) { + const lastPingedTime = root.lastPingedTime; + if (lastPingedTime === NoWork || lastPingedTime >= expirationTime) { + // This render was pinged but we didn't get to restart earlier + // so try restarting now instead. + root.lastPingedTime = expirationTime; + prepareFreshStack(root, expirationTime); break; } - case RootSuspendedWithDelay: { - markRootSuspendedAtTime(root, expirationTime); - const lastSuspendedTime = root.lastSuspendedTime; - if (expirationTime === lastSuspendedTime) { - root.nextKnownPendingLevel = getRemainingExpirationTime( - finishedWork, - ); - } - flushSuspensePriorityWarningInDEV(); - - if ( - // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) - ) { - // We're suspended in a state that should be avoided. We'll try to - // avoid committing it for as long as the timeouts let us. - if (workInProgressRootHasPendingPing) { - const lastPingedTime = root.lastPingedTime; - if ( - lastPingedTime === NoWork || - lastPingedTime >= expirationTime - ) { - // This render was pinged but we didn't get to restart earlier - // so try restarting now instead. - root.lastPingedTime = expirationTime; - prepareFreshStack(root, expirationTime); - break; - } - } - - const nextTime = getNextRootExpirationTimeToWorkOn(root); - if (nextTime !== NoWork && nextTime !== expirationTime) { - // There's additional work on this root. - break; - } - if ( - lastSuspendedTime !== NoWork && - lastSuspendedTime !== expirationTime - ) { - // We should prefer to render the fallback of at the last - // suspended level. Ping the last suspended level to try - // rendering it again. - root.lastPingedTime = lastSuspendedTime; - break; - } + } - let msUntilTimeout; - if (workInProgressRootLatestSuspenseTimeout !== Sync) { - // We have processed a suspense config whose expiration time we - // can use as the timeout. - msUntilTimeout = - expirationTimeToMs(workInProgressRootLatestSuspenseTimeout) - - now(); - } else if ( - workInProgressRootLatestProcessedExpirationTime === Sync - ) { - // This should never normally happen because only new updates - // cause delayed states, so we should have processed something. - // However, this could also happen in an offscreen tree. - msUntilTimeout = 0; - } else { - // If we don't have a suspense config, we're going to use a - // heuristic to determine how long we can suspend. - const eventTimeMs: number = inferTimeFromExpirationTime( - workInProgressRootLatestProcessedExpirationTime, - ); - const currentTimeMs = now(); - const timeUntilExpirationMs = - expirationTimeToMs(expirationTime) - currentTimeMs; - let timeElapsed = currentTimeMs - eventTimeMs; - if (timeElapsed < 0) { - // We get this wrong some time since we estimate the time. - timeElapsed = 0; - } + const nextTime = getNextRootExpirationTimeToWorkOn(root); + if (nextTime !== NoWork && nextTime !== expirationTime) { + // There's additional work on this root. + break; + } + if ( + lastSuspendedTime !== NoWork && + lastSuspendedTime !== expirationTime + ) { + // We should prefer to render the fallback of at the last + // suspended level. Ping the last suspended level to try + // rendering it again. + root.lastPingedTime = lastSuspendedTime; + break; + } - msUntilTimeout = jnd(timeElapsed) - timeElapsed; + let msUntilTimeout; + if (workInProgressRootLatestSuspenseTimeout !== Sync) { + // We have processed a suspense config whose expiration time we + // can use as the timeout. + msUntilTimeout = + expirationTimeToMs(workInProgressRootLatestSuspenseTimeout) - now(); + } else if (workInProgressRootLatestProcessedExpirationTime === Sync) { + // This should never normally happen because only new updates + // cause delayed states, so we should have processed something. + // However, this could also happen in an offscreen tree. + msUntilTimeout = 0; + } else { + // If we don't have a suspense config, we're going to use a + // heuristic to determine how long we can suspend. + const eventTimeMs: number = inferTimeFromExpirationTime( + workInProgressRootLatestProcessedExpirationTime, + ); + const currentTimeMs = now(); + const timeUntilExpirationMs = + expirationTimeToMs(expirationTime) - currentTimeMs; + let timeElapsed = currentTimeMs - eventTimeMs; + if (timeElapsed < 0) { + // We get this wrong some time since we estimate the time. + timeElapsed = 0; + } - // Clamp the timeout to the expiration time. TODO: Once the - // event time is exact instead of inferred from expiration time - // we don't need this. - if (timeUntilExpirationMs < msUntilTimeout) { - msUntilTimeout = timeUntilExpirationMs; - } - } + msUntilTimeout = jnd(timeElapsed) - timeElapsed; - // Don't bother with a very short suspense time. - if (msUntilTimeout > 10) { - // The render is suspended, it hasn't timed out, and there's no - // lower priority work to do. Instead of committing the fallback - // immediately, wait for more data to arrive. - root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), - msUntilTimeout, - ); - break; - } - } - // The work expired. Commit immediately. - commitRoot(root); - break; - } - case RootCompleted: { - // The work completed. Ready to commit. - if ( - // do not delay if we're inside an act() scope - !( - __DEV__ && - flushSuspenseFallbacksInTests && - IsThisRendererActing.current - ) && - workInProgressRootLatestProcessedExpirationTime !== Sync && - workInProgressRootCanSuspendUsingConfig !== null - ) { - // If we have exceeded the minimum loading delay, which probably - // means we have shown a spinner already, we might have to suspend - // a bit longer to ensure that the spinner is shown for - // enough time. - const msUntilTimeout = computeMsUntilSuspenseLoadingDelay( - workInProgressRootLatestProcessedExpirationTime, - expirationTime, - workInProgressRootCanSuspendUsingConfig, - ); - if (msUntilTimeout > 10) { - markRootSuspendedAtTime(root, expirationTime); - root.timeoutHandle = scheduleTimeout( - commitRoot.bind(null, root), - msUntilTimeout, - ); - break; - } - } - commitRoot(root); - break; - } - case RootLocked: { - // This root has a lock that prevents it from committing. Exit. If - // we begin work on the root again, without any intervening updates, - // it will finish without doing additional work. - markRootSuspendedAtTime(root, expirationTime); - break; - } - default: { - invariant(false, 'Unknown root exit status.'); + // Clamp the timeout to the expiration time. TODO: Once the + // event time is exact instead of inferred from expiration time + // we don't need this. + if (timeUntilExpirationMs < msUntilTimeout) { + msUntilTimeout = timeUntilExpirationMs; } } + + // Don't bother with a very short suspense time. + if (msUntilTimeout > 10) { + // The render is suspended, it hasn't timed out, and there's no + // lower priority work to do. Instead of committing the fallback + // immediately, wait for more data to arrive. + root.timeoutHandle = scheduleTimeout( + commitRoot.bind(null, root), + msUntilTimeout, + ); + break; + } } - // Before exiting, make sure there's a callback scheduled for the - // pending level. This is intentionally duplicated in the `catch` block, - // instead of using `finally`, because it needs to happen before we - // possibly return a continuation, and we can't return in the `finally` - // block without suppressing a potential error. - ensureRootIsScheduled(root); - if (root.callbackNode === originalCallbackNode) { - // The task node scheduled for this root is the same one that's - // currently executed. Need to return a continuation. - return performConcurrentWorkOnRoot.bind(null, root); + // The work expired. Commit immediately. + commitRoot(root); + break; + } + case RootCompleted: { + // The work completed. Ready to commit. + if ( + // do not delay if we're inside an act() scope + !( + __DEV__ && + flushSuspenseFallbacksInTests && + IsThisRendererActing.current + ) && + workInProgressRootLatestProcessedExpirationTime !== Sync && + workInProgressRootCanSuspendUsingConfig !== null + ) { + // If we have exceeded the minimum loading delay, which probably + // means we have shown a spinner already, we might have to suspend + // a bit longer to ensure that the spinner is shown for + // enough time. + const msUntilTimeout = computeMsUntilSuspenseLoadingDelay( + workInProgressRootLatestProcessedExpirationTime, + expirationTime, + workInProgressRootCanSuspendUsingConfig, + ); + if (msUntilTimeout > 10) { + markRootSuspendedAtTime(root, expirationTime); + root.timeoutHandle = scheduleTimeout( + commitRoot.bind(null, root), + msUntilTimeout, + ); + break; + } } - } catch (error) { - ensureRootIsScheduled(root); - throw error; + commitRoot(root); + break; + } + case RootLocked: { + // This root has a lock that prevents it from committing. Exit. If + // we begin work on the root again, without any intervening updates, + // it will finish without doing additional work. + markRootSuspendedAtTime(root, expirationTime); + break; + } + default: { + invariant(false, 'Unknown root exit status.'); } } - return null; } // This is the entry point for synchronous tasks that don't go @@ -951,56 +994,111 @@ function performSyncWorkOnRoot(root) { // Check if there's expired work on this root. Otherwise, render at Sync. const lastExpiredTime = root.lastExpiredTime; const expirationTime = lastExpiredTime !== NoWork ? lastExpiredTime : Sync; - try { - if (root.finishedExpirationTime === expirationTime) { - // There's already a pending commit at this expiration time. - // TODO: This is poorly factored. This case only exists for the - // batch.commit() API. - commitRoot(root); - } else { - renderRoot(root, expirationTime, true); - invariant( - workInProgressRootExitStatus !== RootIncomplete, - 'Cannot commit an incomplete root. This error is likely caused by a ' + - 'bug in React. Please file an issue.', - ); + if (root.finishedExpirationTime === expirationTime) { + // There's already a pending commit at this expiration time. + // TODO: This is poorly factored. This case only exists for the + // batch.commit() API. + commitRoot(root); + } else { + invariant( + (executionContext & (RenderContext | CommitContext)) === NoContext, + 'Should not already be working.', + ); - // We now have a consistent tree. The next step is either to commit it, - // or, if something suspended, wait to commit it after a timeout. - stopFinishedWorkLoopTimer(); + flushPassiveEffects(); - root.finishedWork = ((root.current.alternate: any): Fiber); - root.finishedExpirationTime = expirationTime; + // If the root or expiration time have changed, throw out the existing stack + // and prepare a fresh one. Otherwise we'll continue where we left off. + if ( + root !== workInProgressRoot || + expirationTime !== renderExpirationTime + ) { + prepareFreshStack(root, expirationTime); + startWorkOnPendingInteractions(root, expirationTime); + } - resolveLocksOnRoot(root, expirationTime); - if (workInProgressRootExitStatus === RootLocked) { - // This root has a lock that prevents it from committing. Exit. If we - // begin work on the root again, without any intervening updates, it - // will finish without doing additional work. - markRootSuspendedAtTime(root, expirationTime); - } else { - // Set this to null to indicate there's no in-progress render. - workInProgressRoot = null; + // If we have a work-in-progress fiber, it means there's still work to do + // in this root. + if (workInProgress !== null) { + const prevExecutionContext = executionContext; + executionContext |= RenderContext; + const prevDispatcher = pushDispatcher(root); + const prevInteractions = pushInteractions(root); + startWorkLoopTimer(workInProgress); - if (__DEV__) { - if ( - workInProgressRootExitStatus === RootSuspended || - workInProgressRootExitStatus === RootSuspendedWithDelay - ) { - flushSuspensePriorityWarningInDEV(); - } + do { + try { + workLoopSync(); + break; + } catch (thrownValue) { + handleError(root, thrownValue); } - commitRoot(root); + } while (true); + resetContextDependencies(); + executionContext = prevExecutionContext; + popDispatcher(prevDispatcher); + if (enableSchedulerTracing) { + popInteractions(((prevInteractions: any): Set)); + } + + if (workInProgressRootExitStatus === RootFatalErrored) { + const fatalError = workInProgressRootFatalError; + stopInterruptedWorkLoopTimer(); + prepareFreshStack(root, expirationTime); + markRootSuspendedAtTime(root, expirationTime); + ensureRootIsScheduled(root); + throw fatalError; } + + if (workInProgress !== null) { + // This is a sync render, so we should have finished the whole tree. + invariant( + false, + 'Cannot commit an incomplete root. This error is likely caused by a ' + + 'bug in React. Please file an issue.', + ); + } else { + // We now have a consistent tree. Because this is a sync render, we + // will commit it even if something suspended. The only exception is + // if the root is locked (using the unstable_createBatch API). + stopFinishedWorkLoopTimer(); + root.finishedWork = (root.current.alternate: any); + root.finishedExpirationTime = expirationTime; + resolveLocksOnRoot(root, expirationTime); + finishSyncRender(root, workInProgressRootExitStatus, expirationTime); + } + + // Before exiting, make sure there's a callback scheduled for the next + // pending level. + ensureRootIsScheduled(root); } - } finally { - // Before exiting, make sure there's a callback scheduled for the - // pending level. - ensureRootIsScheduled(root); } + return null; } +function finishSyncRender(root, exitStatus, expirationTime) { + if (exitStatus === RootLocked) { + // This root has a lock that prevents it from committing. Exit. If we + // begin work on the root again, without any intervening updates, it + // will finish without doing additional work. + markRootSuspendedAtTime(root, expirationTime); + } else { + // Set this to null to indicate there's no in-progress render. + workInProgressRoot = null; + + if (__DEV__) { + if ( + exitStatus === RootSuspended || + exitStatus === RootSuspendedWithDelay + ) { + flushSuspensePriorityWarningInDEV(); + } + } + commitRoot(root); + } +} + export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) { if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { invariant( @@ -1207,6 +1305,7 @@ function prepareFreshStack(root, expirationTime) { workInProgress = createWorkInProgress(root.current, null, expirationTime); renderExpirationTime = expirationTime; workInProgressRootExitStatus = RootIncomplete; + workInProgressRootFatalError = null; workInProgressRootLatestProcessedExpirationTime = Sync; workInProgressRootLatestSuspenseTimeout = Sync; workInProgressRootCanSuspendUsingConfig = null; @@ -1223,102 +1322,77 @@ function prepareFreshStack(root, expirationTime) { } } -// renderRoot should only be called from inside either -// `performConcurrentWorkOnRoot` or `performSyncWorkOnRoot`. -function renderRoot( - root: FiberRoot, - expirationTime: ExpirationTime, - isSync: boolean, -): void { - invariant( - (executionContext & (RenderContext | CommitContext)) === NoContext, - 'Should not already be working.', - ); +function handleError(root, thrownValue) { + do { + try { + // Reset module-level state that was set during the render phase. + resetContextDependencies(); + resetHooks(); - flushPassiveEffects(); + if (workInProgress === null || workInProgress.return === null) { + // Expected to be working on a non-root fiber. This is a fatal error + // because there's no ancestor that can handle it; the root is + // supposed to capture all errors that weren't caught by an error + // boundary. + workInProgressRootExitStatus = RootFatalErrored; + workInProgressRootFatalError = thrownValue; + return null; + } - // If the root or expiration time have changed, throw out the existing stack - // and prepare a fresh one. Otherwise we'll continue where we left off. - if (root !== workInProgressRoot || expirationTime !== renderExpirationTime) { - prepareFreshStack(root, expirationTime); - startWorkOnPendingInteractions(root, expirationTime); - } + if (enableProfilerTimer && workInProgress.mode & ProfileMode) { + // Record the time spent rendering before an error was thrown. This + // avoids inaccurate Profiler durations in the case of a + // suspended render. + stopProfilerTimerIfRunningAndRecordDelta(workInProgress, true); + } - // If we have a work-in-progress fiber, it means there's still work to do - // in this root. - if (workInProgress !== null) { - const prevExecutionContext = executionContext; - executionContext |= RenderContext; - let prevDispatcher = ReactCurrentDispatcher.current; - if (prevDispatcher === null) { - // The React isomorphic package does not include a default dispatcher. - // Instead the first renderer will lazily attach one, in order to give - // nicer error messages. - prevDispatcher = ContextOnlyDispatcher; - } - ReactCurrentDispatcher.current = ContextOnlyDispatcher; - let prevInteractions: Set | null = null; - if (enableSchedulerTracing) { - prevInteractions = __interactionsRef.current; - __interactionsRef.current = root.memoizedInteractions; + throwException( + root, + workInProgress.return, + workInProgress, + thrownValue, + renderExpirationTime, + ); + workInProgress = completeUnitOfWork(workInProgress); + } catch (yetAnotherThrownValue) { + // Something in the return path also threw. + thrownValue = yetAnotherThrownValue; + continue; } + // Return to the normal work loop. + return; + } while (true); +} - startWorkLoopTimer(workInProgress); - - do { - try { - // TODO: This is now the only place that `isSync` is used. Consider - // outlining the contents of `renderRoot`. - if (isSync) { - workLoopSync(); - } else { - workLoop(); - } - break; - } catch (thrownValue) { - // Reset module-level state that was set during the render phase. - resetContextDependencies(); - resetHooks(); - - const sourceFiber = workInProgress; - if (sourceFiber === null || sourceFiber.return === null) { - // Expected to be working on a non-root fiber. This is a fatal error - // because there's no ancestor that can handle it; the root is - // supposed to capture all errors that weren't caught by an error - // boundary. - prepareFreshStack(root, expirationTime); - executionContext = prevExecutionContext; - markRootSuspendedAtTime(root, expirationTime); - throw thrownValue; - } +function pushDispatcher(root) { + const prevDispatcher = ReactCurrentDispatcher.current; + ReactCurrentDispatcher.current = ContextOnlyDispatcher; + if (prevDispatcher === null) { + // The React isomorphic package does not include a default dispatcher. + // Instead the first renderer will lazily attach one, in order to give + // nicer error messages. + return ContextOnlyDispatcher; + } else { + return prevDispatcher; + } +} - if (enableProfilerTimer && sourceFiber.mode & ProfileMode) { - // Record the time spent rendering before an error was thrown. This - // avoids inaccurate Profiler durations in the case of a - // suspended render. - stopProfilerTimerIfRunningAndRecordDelta(sourceFiber, true); - } +function popDispatcher(prevDispatcher) { + ReactCurrentDispatcher.current = prevDispatcher; +} - const returnFiber = sourceFiber.return; - throwException( - root, - returnFiber, - sourceFiber, - thrownValue, - renderExpirationTime, - ); - // TODO: This is not wrapped in a try-catch, so if the complete phase - // throws, we won't capture it. - workInProgress = completeUnitOfWork(sourceFiber); - } - } while (true); +function pushInteractions(root) { + if (enableSchedulerTracing) { + const prevInteractions: Set | null = __interactionsRef.current; + __interactionsRef.current = root.memoizedInteractions; + return prevInteractions; + } + return null; +} - executionContext = prevExecutionContext; - resetContextDependencies(); - ReactCurrentDispatcher.current = prevDispatcher; - if (enableSchedulerTracing) { - __interactionsRef.current = ((prevInteractions: any): Set); - } +function popInteractions(prevInteractions) { + if (enableSchedulerTracing) { + __interactionsRef.current = prevInteractions; } } @@ -1432,7 +1506,7 @@ function workLoopSync() { } /** @noinline */ -function workLoop() { +function workLoopConcurrent() { // Perform work until Scheduler asks us to yield while (workInProgress !== null && !shouldYield()) { workInProgress = performUnitOfWork(workInProgress); @@ -1760,11 +1834,7 @@ function commitRootImpl(root, renderPriorityLevel) { if (firstEffect !== null) { const prevExecutionContext = executionContext; executionContext |= CommitContext; - let prevInteractions: Set | null = null; - if (enableSchedulerTracing) { - prevInteractions = __interactionsRef.current; - __interactionsRef.current = root.memoizedInteractions; - } + const prevInteractions = pushInteractions(root); // Reset this to null before calling lifecycles ReactCurrentOwner.current = null; @@ -1882,7 +1952,7 @@ function commitRootImpl(root, renderPriorityLevel) { requestPaint(); if (enableSchedulerTracing) { - __interactionsRef.current = ((prevInteractions: any): Set); + popInteractions(((prevInteractions: any): Set)); } executionContext = prevExecutionContext; } else { @@ -2151,18 +2221,13 @@ export function flushPassiveEffects() { } function flushPassiveEffectsImpl(root, expirationTime) { - let prevInteractions: Set | null = null; - if (enableSchedulerTracing) { - prevInteractions = __interactionsRef.current; - __interactionsRef.current = root.memoizedInteractions; - } - invariant( (executionContext & (RenderContext | CommitContext)) === NoContext, 'Cannot flush passive effects while already rendering.', ); const prevExecutionContext = executionContext; executionContext |= CommitContext; + const prevInteractions = pushInteractions(root); // Note: This currently assumes there are no passive effects on the root // fiber, because the root is not part of its own effect list. This could @@ -2193,7 +2258,7 @@ function flushPassiveEffectsImpl(root, expirationTime) { } if (enableSchedulerTracing) { - __interactionsRef.current = ((prevInteractions: any): Set); + popInteractions(((prevInteractions: any): Set)); finishPendingInteractions(root, expirationTime); } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 97f0f480fbeb..6ed257726d8c 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -1392,6 +1392,40 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(Scheduler).toFlushExpired(['Hi']); }); } + + it('handles errors in the return path of a component that suspends', async () => { + // Covers an edge case where an error is thrown inside the complete phase + // of a component that is in the return path of a component that suspends. + // The second error should also be handled (i.e. able to be captured by + // an error boundary. + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error, errorInfo) { + return {error}; + } + render() { + if (this.state.error) { + return `Caught an error: ${this.state.error.message}`; + } + return this.props.children; + } + } + + ReactNoop.renderLegacySyncRoot( + + + + + + + , + ); + + expect(Scheduler).toHaveYielded(['Suspend! [Async]']); + expect(ReactNoop).toMatchRenderedOutput( + 'Caught an error: Error in host config.', + ); + }); }); it('does not call lifecycles of a suspended component', async () => { diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 5cac8a18d89b..00edb16006fa 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -342,5 +342,6 @@ "341": "We just came from a parent so we must have had a parent. This is a bug in React.", "342": "A React component suspended while rendering, but no fallback UI was specified.\n\nAdd a component higher in the tree to provide a loading indicator or placeholder to display.", "343": "ReactDOMServer does not yet support scope components.", - "344": "Expected prepareToHydrateHostSuspenseInstance() to never be called. This error is likely caused by a bug in React. Please file an issue." + "344": "Expected prepareToHydrateHostSuspenseInstance() to never be called. This error is likely caused by a bug in React. Please file an issue.", + "345": "Root did not complete. This is a bug in React." }