Skip to content

Commit

Permalink
[suspense][error handling] Inline renderRoot and fix error handling b…
Browse files Browse the repository at this point in the history
…ug (#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
  • Loading branch information
acdlite committed Sep 23, 2019
1 parent 0a52770 commit 013b7ad
Show file tree
Hide file tree
Showing 3 changed files with 504 additions and 404 deletions.

0 comments on commit 013b7ad

Please sign in to comment.