-
Notifications
You must be signed in to change notification settings - Fork 48k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add onRecoverableError option to hydrateRoot, createRoot #23207
Changes from all commits
cb1c764
0f23548
667e431
1ee9f60
2ab18ff
73ff091
61ef8f4
24abe60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -214,6 +214,7 @@ function render( | |
false, | ||
null, | ||
'', | ||
null, | ||
); | ||
roots.set(containerTag, root); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -210,6 +210,7 @@ function render( | |
false, | ||
null, | ||
'', | ||
null, | ||
); | ||
roots.set(containerTag, root); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; | |
import type {StackCursor} from './ReactFiberStack.old'; | ||
import type {Flags} from './ReactFiberFlags'; | ||
import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; | ||
import type {EventPriority} from './ReactEventPriorities.old'; | ||
|
||
import { | ||
warnAboutDeprecatedLifecycles, | ||
|
@@ -76,6 +77,7 @@ import { | |
supportsMicrotasks, | ||
errorHydratingContainer, | ||
scheduleMicrotask, | ||
logRecoverableError, | ||
} from './ReactFiberHostConfig'; | ||
|
||
import { | ||
|
@@ -296,6 +298,11 @@ let workInProgressRootInterleavedUpdatedLanes: Lanes = NoLanes; | |
let workInProgressRootRenderPhaseUpdatedLanes: Lanes = NoLanes; | ||
// Lanes that were pinged (in an interleaved event) during this render. | ||
let workInProgressRootPingedLanes: Lanes = NoLanes; | ||
// Errors that are thrown during the render phase. | ||
let workInProgressRootConcurrentErrors: Array<mixed> | null = null; | ||
// These are errors that we recovered from without surfacing them to the UI. | ||
// We will log them once the tree commits. | ||
let workInProgressRootRecoverableErrors: Array<mixed> | null = null; | ||
|
||
// The most recent time we committed a fallback. This lets us ensure a train | ||
// model where we don't commit new loading states in too quick succession. | ||
|
@@ -894,13 +901,36 @@ function recoverFromConcurrentError(root, errorRetryLanes) { | |
} | ||
} | ||
|
||
const errorsFromFirstAttempt = workInProgressRootConcurrentErrors; | ||
const exitStatus = renderRootSync(root, errorRetryLanes); | ||
if (exitStatus !== RootErrored) { | ||
// Successfully finished rendering on retry | ||
if (errorsFromFirstAttempt !== null) { | ||
// The errors from the failed first attempt have been recovered. Add | ||
// them to the collection of recoverable errors. We'll log them in the | ||
// commit phase. | ||
queueRecoverableErrors(errorsFromFirstAttempt); | ||
} | ||
} else { | ||
// The UI failed to recover. | ||
} | ||
|
||
executionContext = prevExecutionContext; | ||
|
||
return exitStatus; | ||
} | ||
|
||
export function queueRecoverableErrors(errors: Array<mixed>) { | ||
if (workInProgressRootConcurrentErrors === null) { | ||
workInProgressRootRecoverableErrors = errors; | ||
} else { | ||
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply( | ||
null, | ||
errors, | ||
); | ||
} | ||
} | ||
|
||
function finishConcurrentRender(root, exitStatus, lanes) { | ||
switch (exitStatus) { | ||
case RootIncomplete: | ||
|
@@ -913,7 +943,7 @@ function finishConcurrentRender(root, exitStatus, lanes) { | |
case RootErrored: { | ||
// We should have already attempted to retry this tree. If we reached | ||
// this point, it errored again. Commit it. | ||
commitRoot(root); | ||
commitRoot(root, workInProgressRootRecoverableErrors); | ||
break; | ||
} | ||
case RootSuspended: { | ||
|
@@ -953,14 +983,14 @@ function finishConcurrentRender(root, exitStatus, lanes) { | |
// lower priority work to do. Instead of committing the fallback | ||
// immediately, wait for more data to arrive. | ||
root.timeoutHandle = scheduleTimeout( | ||
commitRoot.bind(null, root), | ||
commitRoot.bind(null, root, workInProgressRootRecoverableErrors), | ||
msUntilTimeout, | ||
); | ||
break; | ||
} | ||
} | ||
// The work expired. Commit immediately. | ||
commitRoot(root); | ||
commitRoot(root, workInProgressRootRecoverableErrors); | ||
break; | ||
} | ||
case RootSuspendedWithDelay: { | ||
|
@@ -991,20 +1021,20 @@ function finishConcurrentRender(root, exitStatus, lanes) { | |
// Instead of committing the fallback immediately, wait for more data | ||
// to arrive. | ||
root.timeoutHandle = scheduleTimeout( | ||
commitRoot.bind(null, root), | ||
commitRoot.bind(null, root, workInProgressRootRecoverableErrors), | ||
msUntilTimeout, | ||
); | ||
break; | ||
} | ||
} | ||
|
||
// Commit the placeholder. | ||
commitRoot(root); | ||
commitRoot(root, workInProgressRootRecoverableErrors); | ||
break; | ||
} | ||
case RootCompleted: { | ||
// The work completed. Ready to commit. | ||
commitRoot(root); | ||
commitRoot(root, workInProgressRootRecoverableErrors); | ||
break; | ||
} | ||
default: { | ||
|
@@ -1124,7 +1154,7 @@ function performSyncWorkOnRoot(root) { | |
const finishedWork: Fiber = (root.current.alternate: any); | ||
root.finishedWork = finishedWork; | ||
root.finishedLanes = lanes; | ||
commitRoot(root); | ||
commitRoot(root, workInProgressRootRecoverableErrors); | ||
|
||
// Before exiting, make sure there's a callback scheduled for the next | ||
// pending level. | ||
|
@@ -1320,6 +1350,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { | |
workInProgressRootInterleavedUpdatedLanes = NoLanes; | ||
workInProgressRootRenderPhaseUpdatedLanes = NoLanes; | ||
workInProgressRootPingedLanes = NoLanes; | ||
workInProgressRootConcurrentErrors = null; | ||
workInProgressRootRecoverableErrors = null; | ||
|
||
enqueueInterleavedUpdates(); | ||
|
||
|
@@ -1474,10 +1506,15 @@ export function renderDidSuspendDelayIfPossible(): void { | |
} | ||
} | ||
|
||
export function renderDidError() { | ||
export function renderDidError(error: mixed) { | ||
if (workInProgressRootExitStatus !== RootSuspendedWithDelay) { | ||
workInProgressRootExitStatus = RootErrored; | ||
} | ||
if (workInProgressRootConcurrentErrors === null) { | ||
workInProgressRootConcurrentErrors = [error]; | ||
} else { | ||
workInProgressRootConcurrentErrors.push(error); | ||
} | ||
} | ||
|
||
// Called during render to determine if anything has suspended. | ||
|
@@ -1781,15 +1818,15 @@ function completeUnitOfWork(unitOfWork: Fiber): void { | |
} | ||
} | ||
|
||
function commitRoot(root) { | ||
function commitRoot(root: FiberRoot, recoverableErrors: null | Array<mixed>) { | ||
// TODO: This no longer makes any sense. We already wrap the mutation and | ||
// layout phases. Should be able to remove. | ||
const previousUpdateLanePriority = getCurrentUpdatePriority(); | ||
const prevTransition = ReactCurrentBatchConfig.transition; | ||
try { | ||
ReactCurrentBatchConfig.transition = 0; | ||
setCurrentUpdatePriority(DiscreteEventPriority); | ||
commitRootImpl(root, previousUpdateLanePriority); | ||
commitRootImpl(root, recoverableErrors, previousUpdateLanePriority); | ||
} finally { | ||
ReactCurrentBatchConfig.transition = prevTransition; | ||
setCurrentUpdatePriority(previousUpdateLanePriority); | ||
|
@@ -1798,7 +1835,11 @@ function commitRoot(root) { | |
return null; | ||
} | ||
|
||
function commitRootImpl(root, renderPriorityLevel) { | ||
function commitRootImpl( | ||
root: FiberRoot, | ||
recoverableErrors: null | Array<mixed>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is kind of annoying but there's no obvious place to store these. We don't usually pass things directly from the render phase to the commit phase because we stash it on a fiber. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get this part. All calls to commitRoot call this with the global So can't you just avoid passing it and instead access There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an annoying edge case when both 1) you're in a multi-root app 2) the commit phase is delayed with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, and with suspense images probably. That is annoying. |
||
renderPriorityLevel: EventPriority, | ||
) { | ||
do { | ||
// `flushPassiveEffects` will call `flushSyncUpdateQueue` at the end, which | ||
// means `flushPassiveEffects` will sometimes result in additional | ||
|
@@ -2069,6 +2110,22 @@ function commitRootImpl(root, renderPriorityLevel) { | |
// additional work on this root is scheduled. | ||
ensureRootIsScheduled(root, now()); | ||
|
||
if (recoverableErrors !== null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The timing of these are interesting given that we were talking about logging order. If it's uncaught it'll be logged after this, but if it's caught the logging will happen in an error boundary who is supposed to log it in componentDidCatch which is a layout effect. So it seems to me that this needs to be logged before the layout effect phase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good point, though I don't know if we can be sure of the order unless we start tracking the recoverable errors per subtree. If there were recoverable errors and regular errors ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea but if they're separate subtrees they're more likely independent and we could've rendered them in any order. The cases I'm mostly thinking about is when you have something like a hydration that succeeded and then errored because it wasn't quite fixed. However, I can't think of a case where that wouldn't be two separate consecutive commits. That would be more of an issue with the throw vs next task throw thing, but not this case. |
||
// There were errors during this render, but recovered from them without | ||
// needing to surface it to the UI. We log them here. | ||
for (let i = 0; i < recoverableErrors.length; i++) { | ||
const recoverableError = recoverableErrors[i]; | ||
const onRecoverableError = root.onRecoverableError; | ||
if (onRecoverableError !== null) { | ||
onRecoverableError(recoverableError); | ||
} else { | ||
// No user-provided onRecoverableError. Use the default behavior | ||
// provided by the renderer's host config. | ||
logRecoverableError(recoverableError); | ||
} | ||
} | ||
} | ||
|
||
if (hasUncaughtError) { | ||
hasUncaughtError = false; | ||
const error = firstUncaughtError; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.error or something maybe to start with?