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 = 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) { - 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) { 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, +) { + 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 = 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) { - 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) { 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, +) { + 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(); + }); + }); + // 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())).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'); + } }); }); });