Suspend on uncaught error, if inside transition #23266
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
|
Comparing: efd8f64...39b49c8 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
acdlite
force-pushed
the
uncaught-errors-suspend
branch
from
e64ac70
to
d5ae0f8
Compare
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.
acdlite
force-pushed
the
uncaught-errors-suspend
branch
from
d5ae0f8
to
39b49c8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I started on this PR because I thought it would be helpful for implementing my next PR to allow suspending in the shell: #23267. However, the implementation details in that PR ended up going a different route, and now I'm unsure if we should keep the behavior in this PR. I'll leave this one open for discussion purposes, though.
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.)
When this happens, we will log the error with
onRecoverableError. (This is a difference from the next PR, #23267, which implements similar behavior for suspending outside of a Suspense boundary. In that case, we don't log an error, because it's a supported pattern.)