From 39b49c83005c216ffc35b70a67cf56252f400dfd Mon Sep 17 00:00:00 2001 From: Andrew Clark <git@andrewclark.io> Date: Wed, 9 Feb 2022 21:15:07 -0500 Subject: [PATCH] Suspend on uncaught error, if inside transition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Usually, if an error isn't caught by an error boundary, we treat it like a panic: the whole app will unmount and we'll throw a top-level error. However, if we're in an async transition, what we can do instead is suspend the transition — i.e. remain on the current screen, like we do during a refresh when we're waiting for new data to load in the background. The reason we only do this for transitions is because synchronous renders are expected to commit synchronously to maintain consistency with external state. (We arguably should suspend-on-uncaught-error for non-sync concurrent renders like continuous inputs, too, but that merits further discussion.) The suspended error is logged with onRecoverableError. --- .../src/createReactNoop.js | 13 +- .../src/ReactFiberThrow.new.js | 26 ++-- .../src/ReactFiberThrow.old.js | 26 ++-- .../src/ReactFiberWorkLoop.new.js | 119 ++++++++++++------ .../src/ReactFiberWorkLoop.old.js | 119 ++++++++++++------ .../ReactConcurrentErrorRecovery-test.js | 33 +++++ ...tIncrementalErrorHandling-test.internal.js | 46 +++++-- .../useMutableSource-test.internal.js | 16 ++- 8 files changed, 288 insertions(+), 110 deletions(-) diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index c314af59cb59a..26fa456f2815e 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -938,7 +938,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return NoopRenderer.flushSync(fn); } - function onRecoverableError(error) { + function onRecoverableErrorDefault(error) { // TODO: Turn this on once tests are fixed // eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args // console.error(error); @@ -972,7 +972,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { null, false, '', - onRecoverableError, + onRecoverableErrorDefault, ); roots.set(rootID, root); } @@ -980,7 +980,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { }, // TODO: Replace ReactNoop.render with createRoot + root.render - createRoot() { + createRoot(options) { const container = { rootID: '' + idCounter++, pendingChildren: [], @@ -994,8 +994,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { null, false, '', - onRecoverableError, + options && options.onRecoverableError + ? options.onRecoverableError + : onRecoverableErrorDefault, ); + return { _Scheduler: Scheduler, render(children: ReactNodeList) { @@ -1024,7 +1027,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { null, false, '', - onRecoverableError, + onRecoverableErrorDefault, ); return { _Scheduler: Scheduler, diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 6a0c70f8e6d00..66b6420710bb8 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -62,6 +62,8 @@ import { } from './ReactFiberSuspenseContext.new'; import { renderDidError, + renderDidErrorUncaught, + queueConcurrentError, onUncaughtError, markLegacyErrorBoundaryAsFailed, isAlreadyFailedLegacyErrorBoundary, @@ -516,22 +518,32 @@ function throwException( queueHydrationError(value); return; } - } else { - // Otherwise, fall through to the error path. } + + // Otherwise, fall through to the error path. + + // Push the error to a queue. If we end up recovering without surfacing + // the error to the user, we'll updgrade this to a recoverable error and + // log it with onRecoverableError. + // + // This is intentionally a separate call from renderDidError because in + // some cases we use the error handling path as an implementation detail + // to unwind the stack, but we don't want to log it as a real error. An + // example is suspending outside of a Suspense boundary (see previous + // branch above). + queueConcurrentError(value); } // We didn't find a boundary that could handle this type of exception. Start // over and traverse parent path again, this time treating the exception // as an error. - renderDidError(value); - - value = createCapturedValue(value, sourceFiber); + const error = value; + const errorInfo = createCapturedValue(error, sourceFiber); let workInProgress = returnFiber; do { switch (workInProgress.tag) { case HostRoot: { - const errorInfo = value; + renderDidErrorUncaught(); workInProgress.flags |= ShouldCapture; const lane = pickArbitraryLane(rootRenderLanes); workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); @@ -541,7 +553,7 @@ function throwException( } case ClassComponent: // Capture and retry - const errorInfo = value; + renderDidError(); const ctor = workInProgress.type; const instance = workInProgress.stateNode; if ( diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 21ab03f4ac925..4b3b70707d0cc 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -62,6 +62,8 @@ import { } from './ReactFiberSuspenseContext.old'; import { renderDidError, + renderDidErrorUncaught, + queueConcurrentError, onUncaughtError, markLegacyErrorBoundaryAsFailed, isAlreadyFailedLegacyErrorBoundary, @@ -516,22 +518,32 @@ function throwException( queueHydrationError(value); return; } - } else { - // Otherwise, fall through to the error path. } + + // Otherwise, fall through to the error path. + + // Push the error to a queue. If we end up recovering without surfacing + // the error to the user, we'll updgrade this to a recoverable error and + // log it with onRecoverableError. + // + // This is intentionally a separate call from renderDidError because in + // some cases we use the error handling path as an implementation detail + // to unwind the stack, but we don't want to log it as a real error. An + // example is suspending outside of a Suspense boundary (see previous + // branch above). + queueConcurrentError(value); } // We didn't find a boundary that could handle this type of exception. Start // over and traverse parent path again, this time treating the exception // as an error. - renderDidError(value); - - value = createCapturedValue(value, sourceFiber); + const error = value; + const errorInfo = createCapturedValue(error, sourceFiber); let workInProgress = returnFiber; do { switch (workInProgress.tag) { case HostRoot: { - const errorInfo = value; + renderDidErrorUncaught(); workInProgress.flags |= ShouldCapture; const lane = pickArbitraryLane(rootRenderLanes); workInProgress.lanes = mergeLanes(workInProgress.lanes, lane); @@ -541,7 +553,7 @@ function throwException( } case ClassComponent: // Capture and retry - const errorInfo = value; + renderDidError(); const ctor = workInProgress.type; const instance = workInProgress.stateNode; if ( diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 33131e20bb632..be3af3df5b7bd 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -251,13 +251,14 @@ const RenderContext = /* */ 0b0010; const CommitContext = /* */ 0b0100; export const RetryAfterError = /* */ 0b1000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootIncomplete = 0; -const RootFatalErrored = 1; -const RootErrored = 2; -const RootSuspended = 3; -const RootSuspendedWithDelay = 4; -const RootCompleted = 5; +const RootErroredInternal = 1; +const RootErroredUncaught = 2; +const RootErrored = 3; +const RootSuspended = 4; +const RootSuspendedWithDelay = 5; +const RootCompleted = 6; // Describes where we are in the React execution stack let executionContext: ExecutionContext = NoContext; @@ -281,8 +282,9 @@ const subtreeRenderLanesCursor: StackCursor<Lanes> = createCursor(NoLanes); // Whether to root completed, errored, suspended, etc. let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; -// A fatal error, if one is thrown -let workInProgressRootFatalError: mixed = null; +// An internal error that can't be handled using the normal error handling path. +// This happens when there's a bug within React itself. +let workInProgressInternalError: mixed = null; // "Included" lanes refer to lanes that were worked on during this render. It's // slightly different than `renderLanes` because `renderLanes` can change as you // enter and exit an Offscreen tree. This value is the combination of all render @@ -818,7 +820,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { ? renderRootConcurrent(root, lanes) : renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { - if (exitStatus === RootErrored) { + if (exitStatus === RootErrored || exitStatus === RootErroredUncaught) { // If something threw an error, try rendering one more time. We'll // render synchronously to block concurrent data mutations, and we'll // includes all pending updates are included. If it still fails after @@ -829,12 +831,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) { exitStatus = recoverFromConcurrentError(root, errorRetryLanes); } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; + if (exitStatus === RootErroredInternal) { + const internalError = workInProgressInternalError; prepareFreshStack(root, NoLanes); markRootSuspended(root, lanes); ensureRootIsScheduled(root, now()); - throw fatalError; + throw internalError; } // Check if this render may have yielded to a concurrent event, and if so, @@ -853,7 +855,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { exitStatus = renderRootSync(root, lanes); // We need to check again if something threw - if (exitStatus === RootErrored) { + if (exitStatus === RootErrored || exitStatus === RootErroredUncaught) { const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); if (errorRetryLanes !== NoLanes) { lanes = errorRetryLanes; @@ -862,12 +864,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // concurrent events. } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; + if (exitStatus === RootErroredInternal) { + const internalError = workInProgressInternalError; prepareFreshStack(root, NoLanes); markRootSuspended(root, lanes); ensureRootIsScheduled(root, now()); - throw fatalError; + throw internalError; } } @@ -902,7 +904,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) { const errorsFromFirstAttempt = workInProgressRootConcurrentErrors; const exitStatus = renderRootSync(root, errorRetryLanes); - if (exitStatus !== RootErrored) { + if (exitStatus !== RootErrored && exitStatus !== RootErroredUncaught) { // Successfully finished rendering on retry if (errorsFromFirstAttempt !== null) { // The errors from the failed first attempt have been recovered. Add @@ -920,11 +922,11 @@ function recoverFromConcurrentError(root, errorRetryLanes) { } export function queueRecoverableErrors(errors: Array<mixed>) { - if (workInProgressRootConcurrentErrors === null) { + if (workInProgressRootRecoverableErrors === null) { workInProgressRootRecoverableErrors = errors; } else { - workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply( - workInProgressRootConcurrentErrors, + workInProgressRootRecoverableErrors.push.apply( + workInProgressRootRecoverableErrors, errors, ); } @@ -933,12 +935,35 @@ export function queueRecoverableErrors(errors: Array<mixed>) { function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: - case RootFatalErrored: { + case RootErroredInternal: { throw new Error('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 RootErroredUncaught: { + // An error was thrown but was not caught by an error boundary. This will + // cause the whole root to unmount. However, if this render was the + // result of a transition (e.g. startTransition) we can suspend instead. + if (includesOnlyTransitions(lanes)) { + // This is a transition, so we'll suspend instead of surfacing + // the error. + markRootSuspended(root, lanes); + + // Log the errors that were thrown during this render. Normally we log + // recoverable errors in the commit phase, but we do it here in this + // case because we intentionally skipped the commit phase. + if (workInProgressRootConcurrentErrors !== null) { + logRecoverableErrors(root, workInProgressRootConcurrentErrors); + } + return; + } + + commitRoot(root, workInProgressRootRecoverableErrors); + + // TODO: Currently, when there's an uncaught error, we add it to the root + // fiber's effect queue and re-throw it at the end of the commit phase. + // It might make more sense to rethrow the error here instead. The timing + // is the same, but we wouldn't have to queue the error. + break; + } case RootErrored: { // We should have already attempted to retry this tree. If we reached // this point, it errored again. Commit it. @@ -1128,7 +1153,10 @@ function performSyncWorkOnRoot(root) { } let exitStatus = renderRootSync(root, lanes); - if (root.tag !== LegacyRoot && exitStatus === RootErrored) { + if ( + root.tag !== LegacyRoot && + (exitStatus === RootErrored || exitStatus === RootErroredUncaught) + ) { // If something threw an error, try rendering one more time. We'll render // synchronously to block concurrent data mutations, and we'll includes // all pending updates are included. If it still fails after the second @@ -1140,8 +1168,8 @@ function performSyncWorkOnRoot(root) { } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; + if (exitStatus === RootErroredInternal) { + const fatalError = workInProgressInternalError; prepareFreshStack(root, NoLanes); markRootSuspended(root, lanes); ensureRootIsScheduled(root, now()); @@ -1344,7 +1372,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgress = createWorkInProgress(root.current, null); workInProgressRootRenderLanes = subtreeRenderLanes = workInProgressRootIncludedLanes = lanes; workInProgressRootExitStatus = RootIncomplete; - workInProgressRootFatalError = null; + workInProgressInternalError = null; workInProgressRootSkippedLanes = NoLanes; workInProgressRootInterleavedUpdatedLanes = NoLanes; workInProgressRootRenderPhaseUpdatedLanes = NoLanes; @@ -1376,8 +1404,8 @@ function handleError(root, thrownValue): void { // 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; + workInProgressRootExitStatus = RootErroredInternal; + workInProgressInternalError = thrownValue; // Set `workInProgress` to null. This represents advancing to the next // sibling, or the parent if there are no siblings. But since the root // has no siblings nor a parent, we set it to null. Usually this is @@ -1482,7 +1510,8 @@ export function renderDidSuspendDelayIfPossible(): void { if ( workInProgressRootExitStatus === RootIncomplete || workInProgressRootExitStatus === RootSuspended || - workInProgressRootExitStatus === RootErrored + workInProgressRootExitStatus === RootErrored || + workInProgressRootExitStatus === RootErroredUncaught ) { workInProgressRootExitStatus = RootSuspendedWithDelay; } @@ -1505,10 +1534,19 @@ export function renderDidSuspendDelayIfPossible(): void { } } -export function renderDidError(error: mixed) { +export function renderDidError() { if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { workInProgressRootExitStatus = RootErrored; } +} + +export function renderDidErrorUncaught() { + if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { + workInProgressRootExitStatus = RootErroredUncaught; + } +} + +export function queueConcurrentError(error: mixed) { if (workInProgressRootConcurrentErrors === null) { workInProgressRootConcurrentErrors = [error]; } else { @@ -2112,11 +2150,7 @@ function commitRootImpl( if (recoverableErrors !== null) { // There were errors during this render, but recovered from them without // needing to surface it to the UI. We log them here. - const onRecoverableError = root.onRecoverableError; - for (let i = 0; i < recoverableErrors.length; i++) { - const recoverableError = recoverableErrors[i]; - onRecoverableError(recoverableError); - } + logRecoverableErrors(root, recoverableErrors); } if (hasUncaughtError) { @@ -2176,6 +2210,17 @@ function commitRootImpl( return null; } +function logRecoverableErrors( + root: FiberRoot, + recoverableErrors: Array<mixed>, +) { + const onRecoverableError = root.onRecoverableError; + for (let i = 0; i < recoverableErrors.length; i++) { + const recoverableError = recoverableErrors[i]; + onRecoverableError(recoverableError); + } +} + function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) { if (enableCache) { const pooledCacheLanes = (root.pooledCacheLanes &= remainingLanes); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 17ee82384d5e6..49fad4ea86eb9 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -251,13 +251,14 @@ const RenderContext = /* */ 0b0010; const CommitContext = /* */ 0b0100; export const RetryAfterError = /* */ 0b1000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootIncomplete = 0; -const RootFatalErrored = 1; -const RootErrored = 2; -const RootSuspended = 3; -const RootSuspendedWithDelay = 4; -const RootCompleted = 5; +const RootErroredInternal = 1; +const RootErroredUncaught = 2; +const RootErrored = 3; +const RootSuspended = 4; +const RootSuspendedWithDelay = 5; +const RootCompleted = 6; // Describes where we are in the React execution stack let executionContext: ExecutionContext = NoContext; @@ -281,8 +282,9 @@ const subtreeRenderLanesCursor: StackCursor<Lanes> = createCursor(NoLanes); // Whether to root completed, errored, suspended, etc. let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; -// A fatal error, if one is thrown -let workInProgressRootFatalError: mixed = null; +// An internal error that can't be handled using the normal error handling path. +// This happens when there's a bug within React itself. +let workInProgressInternalError: mixed = null; // "Included" lanes refer to lanes that were worked on during this render. It's // slightly different than `renderLanes` because `renderLanes` can change as you // enter and exit an Offscreen tree. This value is the combination of all render @@ -818,7 +820,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { ? renderRootConcurrent(root, lanes) : renderRootSync(root, lanes); if (exitStatus !== RootIncomplete) { - if (exitStatus === RootErrored) { + if (exitStatus === RootErrored || exitStatus === RootErroredUncaught) { // If something threw an error, try rendering one more time. We'll // render synchronously to block concurrent data mutations, and we'll // includes all pending updates are included. If it still fails after @@ -829,12 +831,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) { exitStatus = recoverFromConcurrentError(root, errorRetryLanes); } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; + if (exitStatus === RootErroredInternal) { + const internalError = workInProgressInternalError; prepareFreshStack(root, NoLanes); markRootSuspended(root, lanes); ensureRootIsScheduled(root, now()); - throw fatalError; + throw internalError; } // Check if this render may have yielded to a concurrent event, and if so, @@ -853,7 +855,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { exitStatus = renderRootSync(root, lanes); // We need to check again if something threw - if (exitStatus === RootErrored) { + if (exitStatus === RootErrored || exitStatus === RootErroredUncaught) { const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); if (errorRetryLanes !== NoLanes) { lanes = errorRetryLanes; @@ -862,12 +864,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // concurrent events. } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; + if (exitStatus === RootErroredInternal) { + const internalError = workInProgressInternalError; prepareFreshStack(root, NoLanes); markRootSuspended(root, lanes); ensureRootIsScheduled(root, now()); - throw fatalError; + throw internalError; } } @@ -902,7 +904,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) { const errorsFromFirstAttempt = workInProgressRootConcurrentErrors; const exitStatus = renderRootSync(root, errorRetryLanes); - if (exitStatus !== RootErrored) { + if (exitStatus !== RootErrored && exitStatus !== RootErroredUncaught) { // Successfully finished rendering on retry if (errorsFromFirstAttempt !== null) { // The errors from the failed first attempt have been recovered. Add @@ -920,11 +922,11 @@ function recoverFromConcurrentError(root, errorRetryLanes) { } export function queueRecoverableErrors(errors: Array<mixed>) { - if (workInProgressRootConcurrentErrors === null) { + if (workInProgressRootRecoverableErrors === null) { workInProgressRootRecoverableErrors = errors; } else { - workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply( - workInProgressRootConcurrentErrors, + workInProgressRootRecoverableErrors.push.apply( + workInProgressRootRecoverableErrors, errors, ); } @@ -933,12 +935,35 @@ export function queueRecoverableErrors(errors: Array<mixed>) { function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { case RootIncomplete: - case RootFatalErrored: { + case RootErroredInternal: { throw new Error('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 RootErroredUncaught: { + // An error was thrown but was not caught by an error boundary. This will + // cause the whole root to unmount. However, if this render was the + // result of a transition (e.g. startTransition) we can suspend instead. + if (includesOnlyTransitions(lanes)) { + // This is a transition, so we'll suspend instead of surfacing + // the error. + markRootSuspended(root, lanes); + + // Log the errors that were thrown during this render. Normally we log + // recoverable errors in the commit phase, but we do it here in this + // case because we intentionally skipped the commit phase. + if (workInProgressRootConcurrentErrors !== null) { + logRecoverableErrors(root, workInProgressRootConcurrentErrors); + } + return; + } + + commitRoot(root, workInProgressRootRecoverableErrors); + + // TODO: Currently, when there's an uncaught error, we add it to the root + // fiber's effect queue and re-throw it at the end of the commit phase. + // It might make more sense to rethrow the error here instead. The timing + // is the same, but we wouldn't have to queue the error. + break; + } case RootErrored: { // We should have already attempted to retry this tree. If we reached // this point, it errored again. Commit it. @@ -1128,7 +1153,10 @@ function performSyncWorkOnRoot(root) { } let exitStatus = renderRootSync(root, lanes); - if (root.tag !== LegacyRoot && exitStatus === RootErrored) { + if ( + root.tag !== LegacyRoot && + (exitStatus === RootErrored || exitStatus === RootErroredUncaught) + ) { // If something threw an error, try rendering one more time. We'll render // synchronously to block concurrent data mutations, and we'll includes // all pending updates are included. If it still fails after the second @@ -1140,8 +1168,8 @@ function performSyncWorkOnRoot(root) { } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; + if (exitStatus === RootErroredInternal) { + const fatalError = workInProgressInternalError; prepareFreshStack(root, NoLanes); markRootSuspended(root, lanes); ensureRootIsScheduled(root, now()); @@ -1344,7 +1372,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgress = createWorkInProgress(root.current, null); workInProgressRootRenderLanes = subtreeRenderLanes = workInProgressRootIncludedLanes = lanes; workInProgressRootExitStatus = RootIncomplete; - workInProgressRootFatalError = null; + workInProgressInternalError = null; workInProgressRootSkippedLanes = NoLanes; workInProgressRootInterleavedUpdatedLanes = NoLanes; workInProgressRootRenderPhaseUpdatedLanes = NoLanes; @@ -1376,8 +1404,8 @@ function handleError(root, thrownValue): void { // 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; + workInProgressRootExitStatus = RootErroredInternal; + workInProgressInternalError = thrownValue; // Set `workInProgress` to null. This represents advancing to the next // sibling, or the parent if there are no siblings. But since the root // has no siblings nor a parent, we set it to null. Usually this is @@ -1482,7 +1510,8 @@ export function renderDidSuspendDelayIfPossible(): void { if ( workInProgressRootExitStatus === RootIncomplete || workInProgressRootExitStatus === RootSuspended || - workInProgressRootExitStatus === RootErrored + workInProgressRootExitStatus === RootErrored || + workInProgressRootExitStatus === RootErroredUncaught ) { workInProgressRootExitStatus = RootSuspendedWithDelay; } @@ -1505,10 +1534,19 @@ export function renderDidSuspendDelayIfPossible(): void { } } -export function renderDidError(error: mixed) { +export function renderDidError() { if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { workInProgressRootExitStatus = RootErrored; } +} + +export function renderDidErrorUncaught() { + if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { + workInProgressRootExitStatus = RootErroredUncaught; + } +} + +export function queueConcurrentError(error: mixed) { if (workInProgressRootConcurrentErrors === null) { workInProgressRootConcurrentErrors = [error]; } else { @@ -2112,11 +2150,7 @@ function commitRootImpl( if (recoverableErrors !== null) { // There were errors during this render, but recovered from them without // needing to surface it to the UI. We log them here. - const onRecoverableError = root.onRecoverableError; - for (let i = 0; i < recoverableErrors.length; i++) { - const recoverableError = recoverableErrors[i]; - onRecoverableError(recoverableError); - } + logRecoverableErrors(root, recoverableErrors); } if (hasUncaughtError) { @@ -2176,6 +2210,17 @@ function commitRootImpl( return null; } +function logRecoverableErrors( + root: FiberRoot, + recoverableErrors: Array<mixed>, +) { + const onRecoverableError = root.onRecoverableError; + for (let i = 0; i < recoverableErrors.length; i++) { + const recoverableError = recoverableErrors[i]; + onRecoverableError(recoverableError); + } +} + function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) { if (enableCache) { const pooledCacheLanes = (root.pooledCacheLanes &= remainingLanes); diff --git a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js index b968826845923..2b65c3fcc826b 100644 --- a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js +++ b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js @@ -398,4 +398,37 @@ describe('ReactConcurrentErrorRecovery', () => { // Now we can show the error boundary that's wrapped around B. expect(root).toMatchRenderedOutput('Oops!B2'); }); + + test("uncaught errors at the root should suspend if they're part of a transition", async () => { + const root = ReactNoop.createRoot({ + onRecoverableError(error) { + Scheduler.unstable_yieldValue('Log recoverable error: ' + error); + }, + }); + + function Throws() { + throw new Error('Oops!'); + } + + await act(async () => { + root.render('(empty)'); + }); + + // Trigger an error during render. Because it's wrapped with + // startTransition, the render will suspend instead of unmounting the app. + await act(async () => { + startTransition(() => { + root.render(<Throws />); + }); + }); + // The error is logged with onRecoverableError + expect(Scheduler).toHaveYielded(['Log recoverable error: Error: Oops!']); + // Previous screen is still visible. + expect(root).toMatchRenderedOutput('(empty)'); + + // Confirm that if there's no startTransition, the error surfaces + expect(() => act(() => root.render(<Throws />))).toThrow('Oops!'); + // onRecoverableError is not called this time + expect(Scheduler).toHaveYielded([]); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 55ce08b45450b..00c72d674c27c 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -416,14 +416,29 @@ describe('ReactIncrementalErrorHandling', () => { // Finish the rest of the async work expect(Scheduler).toFlushAndYieldThrough(['Sibling']); - // Old scheduler renders, commits, and throws synchronously - expect(() => Scheduler.unstable_flushNumberOfYields(1)).toThrow('oops'); - expect(Scheduler).toHaveYielded([ - 'Parent', - 'BadRender', - 'Sibling', - 'commit', - ]); + if (gate(flags => flags.enableSyncDefaultUpdates)) { + // When the update is wrapped in startTransition, we suspend instead of + // rethrowing the error. That part isn't really relevant to this test; + // the only reason we use startTransition above is to ensure the update + // is concurrent. If/when enableSyncDefaultUpdates lands, remove + // this branch. + Scheduler.unstable_flushNumberOfYields(1); + expect(Scheduler).toHaveYielded([ + 'Parent', + 'BadRender', + 'Sibling', + // Render suspends without committing + // 'commit', + ]); + } else { + expect(() => Scheduler.unstable_flushNumberOfYields(1)).toThrow('oops'); + expect(Scheduler).toHaveYielded([ + 'Parent', + 'BadRender', + 'Sibling', + 'commit', + ]); + } expect(ReactNoop.getChildren()).toEqual([]); }); @@ -464,10 +479,19 @@ describe('ReactIncrementalErrorHandling', () => { // Expire the render midway through Scheduler.unstable_advanceTime(10000); - expect(() => { + if (gate(flags => flags.enableSyncDefaultUpdates)) { + // When the update is wrapped in startTransition, we suspend instead of + // rethrowing the error. That part isn't really relevant to this test; + // the only reason we use startTransition above is to ensure the update + // is concurrent. If/when enableSyncDefaultUpdates lands, remove + // this branch. Scheduler.unstable_flushExpired(); - ReactNoop.flushSync(); - }).toThrow('Oops'); + } else { + expect(() => { + Scheduler.unstable_flushExpired(); + ReactNoop.flushSync(); + }).toThrow('Oops'); + } expect(Scheduler).toHaveYielded([ // The render expired, but we shouldn't throw out the partial work. diff --git a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js index 11cabd6f3175a..d5f52087ac314 100644 --- a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js +++ b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js @@ -1918,9 +1918,11 @@ describe('useMutableSource', () => { // TODO (useMutableSource) Act will automatically flush remaining work from render 1, // but at this point something in the hooks dispatcher has been broken by jest.resetModules() // Figure out what this is and remove this catch. - expect(() => - PrevScheduler.unstable_flushAllWithoutAsserting(), - ).toThrow('Invalid hook call'); + if (gate(flags => !flags.enableSyncDefaultUpdates)) { + expect(() => + PrevScheduler.unstable_flushAllWithoutAsserting(), + ).toThrow('Invalid hook call'); + } }); }); @@ -2002,9 +2004,11 @@ describe('useMutableSource', () => { // TODO (useMutableSource) Act will automatically flush remaining work from render 1, // but at this point something in the hooks dispatcher has been broken by jest.resetModules() // Figure out what this is and remove this catch. - expect(() => - PrevScheduler.unstable_flushAllWithoutAsserting(), - ).toThrow('Invalid hook call'); + if (gate(flags => !flags.enableSyncDefaultUpdates)) { + expect(() => + PrevScheduler.unstable_flushAllWithoutAsserting(), + ).toThrow('Invalid hook call'); + } }); }); });