Use the Nearest Parent of an Errored Promise as its Owner #29814
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
force-pushed
the
trackpromiseowner
branch
2 times, most recently
from
3281087
to
7d74cba
Compare
sebmarkbage
force-pushed
the
trackpromiseowner
branch
from
7d74cba
to
bf3f446
Compare
sebmarkbage
force-pushed
the
trackpromiseowner
branch
from
bf3f446
to
5b8d9e9
Compare
acdlite
reviewed
Jun 11, 2024
| for (let i = debugInfo.length - 1; i >= 0; i--) { | ||
| if (typeof debugInfo[i].stack === 'string') { | ||
| // Ideally we could get the Task from this object but we don't expose it | ||
| // from Flight since it's in a WeakMap so we use the |
There was a problem hiding this comment.
forgot to write the end of this comment
acdlite
approved these changes
Jun 11, 2024
If this is happening on the server where the FlightClient generates objects that will then be consumed again by the Flight server, we need to strip the task out before serializing it since it's not serializable but it can be restored from stack + owner.
Whether that nearest parent is a Server Component or the nearest Fiber.
sebmarkbage
force-pushed
the
trackpromiseowner
branch
from
52033d4
to
adc2f78
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Jun 11, 2024
Stacked on #29807. Conceptually the error's owner/task should ideally be captured when the Error constructor is called but neither `console.createTask` does this, nor do we override `Error` to capture our `owner`. So instead, we use the nearest parent as the owner/task of the error. This is usually the same thing when it's thrown from the same async component but not if you await a promise started from a different component/task. Before this stack the "owner" and "task" of a Lazy that errors was the nearest Fiber but if the thing erroring is a Server Component, we need to get that as the owner from the inner most part of debugInfo. To get the Task for that Server Component, we need to expose it on the ReactComponentInfo object. Unfortunately that makes the object not serializable so we need to special case this to exclude it from serialization. It gets restored again on the client. Before (Shell): <img width="813" alt="Screenshot 2024-06-06 at 5 16 20 PM" src="https://github.com/facebook/react/assets/63648/7da2d4c9-539b-494e-ba63-1abdc58ff13c"> After (App): <img width="811" alt="Screenshot 2024-06-08 at 12 29 23 AM" src="https://github.com/facebook/react/assets/63648/dbf40bd7-c24d-4200-81a6-5018bef55f6d"> DiffTrain build for commit 383b2a1.
github-actions bot
pushed a commit
that referenced
this pull request
Jun 11, 2024
Stacked on #29807. Conceptually the error's owner/task should ideally be captured when the Error constructor is called but neither `console.createTask` does this, nor do we override `Error` to capture our `owner`. So instead, we use the nearest parent as the owner/task of the error. This is usually the same thing when it's thrown from the same async component but not if you await a promise started from a different component/task. Before this stack the "owner" and "task" of a Lazy that errors was the nearest Fiber but if the thing erroring is a Server Component, we need to get that as the owner from the inner most part of debugInfo. To get the Task for that Server Component, we need to expose it on the ReactComponentInfo object. Unfortunately that makes the object not serializable so we need to special case this to exclude it from serialization. It gets restored again on the client. Before (Shell): <img width="813" alt="Screenshot 2024-06-06 at 5 16 20 PM" src="https://github.com/facebook/react/assets/63648/7da2d4c9-539b-494e-ba63-1abdc58ff13c"> After (App): <img width="811" alt="Screenshot 2024-06-08 at 12 29 23 AM" src="https://github.com/facebook/react/assets/63648/dbf40bd7-c24d-4200-81a6-5018bef55f6d"> DiffTrain build for [383b2a1](383b2a1)
This was referenced Jul 25, 2024
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.
Stacked on #29807.
Conceptually the error's owner/task should ideally be captured when the Error constructor is called but neither
console.createTaskdoes this, nor do we overrideErrorto capture ourowner. So instead, we use the nearest parent as the owner/task of the error. This is usually the same thing when it's thrown from the same async component but not if you await a promise started from a different component/task.Before this stack the "owner" and "task" of a Lazy that errors was the nearest Fiber but if the thing erroring is a Server Component, we need to get that as the owner from the inner most part of debugInfo.
To get the Task for that Server Component, we need to expose it on the ReactComponentInfo object. Unfortunately that makes the object not serializable so we need to special case this to exclude it from serialization. It gets restored again on the client.
Before (Shell):
After (App):
