[Flight] Eval Fake Server Component Functions to Recreate Native Stacks #29632
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
This lets the native stack traces include Server Component's stack frames. This applies to client element's stacks, console replay stacks, and server component's stacks.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
This is because otherwise it ends up being "eval" instead of missing due to this being in an eval:ed script.
sebmarkbage
commented
May 29, 2024
| type !== null && | ||
| type.$$typeof === REACT_LAZY_TYPE | ||
| ) { | ||
| if (type._init === readChunk) { |
There was a problem hiding this comment.
I'm thinking of maybe using "use client" as the task name in this case because you're about to enter the the client and likely the client code will include the name of the client component but we may not have the code loaded yet when the element is created.
There should be only one of these if you follow the owner stack. For host components and built-ins we use the string name since nothing else will have those as the owner it'll always terminate with those and they don't have another name.
hoxyq
approved these changes
May 29, 2024
Comment on lines
+1712
to
+1714
| const filename = parsed[2] || parsed[5] || ''; | ||
| const line = +(parsed[3] || parsed[6]); | ||
| const col = +(parsed[4] || parsed[7]); |
There was a problem hiding this comment.
Are you sure that all of this will be present in the stack frame?
There was a problem hiding this comment.
If it's not, it won't match the regexp an we'll skip the frame. This is not perfect for edge cases like built-ins. We also have to do something for Hermes for HaaS and any other VM that we choose to support. We can iterate.
Bun uses JSC but it seems to use V8-style stack trace formatting too.
There is also an option that use the prepareStackTrace technique on the server to get a structured stack and then send structured data in the RSC protocol instead of strings. This would also let use do arbitrary formatting on the client. I was hoping to avoid that complexity though.
github-actions bot
pushed a commit
that referenced
this pull request
May 30, 2024
…ks (#29632) We have three kinds of stacks that we send in the RSC protocol: - The stack trace where a replayed `console.log` was called on the server. - The JSX callsite that created a Server Component which then later called another component. - The JSX callsite that created a Host or Client Component. These stack frames disappear in native stacks on the client since they're executed on the server. This evals a fake file which only has one call in it on the same line/column as the server. Then we call through these fake modules to "replay" the callstack. We then replay the `console.log` within this stack, or call `console.createTask` in this stack to recreate the stack. The main concern with this approach is the performance. It adds significant cost to create all these eval:ed functions but it should eventually balance out. This doesn't yet apply source maps to these. With source maps it'll be able to show the server source code when clicking the links. I don't love how these appear. - Because we haven't yet initialized the client module we don't have the name of the client component we're about to render yet which leads to the `<...>` task name. - The `(async)` suffix Chrome adds is still a problem. - The VMxxxx prefix is used to disambiguate which is noisy. Might be helped by source maps. - The continuation of the async stacks end up rooted somewhere in the bootstrapping of the app. This might be ok when the bootstrapping ends up ignore listed but it's kind of a problem that you can't clear the async stack. <img width="927" alt="Screenshot 2024-05-28 at 11 58 56 PM" src="https://github.com/facebook/react/assets/63648/1c9d32ce-e671-47c8-9d18-9fab3bffabd0"> <img width="431" alt="Screenshot 2024-05-28 at 11 58 07 PM" src="https://github.com/facebook/react/assets/63648/52f57518-bbed-400e-952d-6650835ac6b6"> <img width="327" alt="Screenshot 2024-05-28 at 11 58 31 PM" src="https://github.com/facebook/react/assets/63648/d311a639-79a1-457f-9a46-4f3298d07e65"> <img width="817" alt="Screenshot 2024-05-28 at 11 59 12 PM" src="https://github.com/facebook/react/assets/63648/3aefd356-acf4-4daa-bdbf-b8c8345f6d4b"> DiffTrain build for commit 9d4fba0.
github-actions bot
pushed a commit
that referenced
this pull request
May 30, 2024
…ks (#29632) We have three kinds of stacks that we send in the RSC protocol: - The stack trace where a replayed `console.log` was called on the server. - The JSX callsite that created a Server Component which then later called another component. - The JSX callsite that created a Host or Client Component. These stack frames disappear in native stacks on the client since they're executed on the server. This evals a fake file which only has one call in it on the same line/column as the server. Then we call through these fake modules to "replay" the callstack. We then replay the `console.log` within this stack, or call `console.createTask` in this stack to recreate the stack. The main concern with this approach is the performance. It adds significant cost to create all these eval:ed functions but it should eventually balance out. This doesn't yet apply source maps to these. With source maps it'll be able to show the server source code when clicking the links. I don't love how these appear. - Because we haven't yet initialized the client module we don't have the name of the client component we're about to render yet which leads to the `<...>` task name. - The `(async)` suffix Chrome adds is still a problem. - The VMxxxx prefix is used to disambiguate which is noisy. Might be helped by source maps. - The continuation of the async stacks end up rooted somewhere in the bootstrapping of the app. This might be ok when the bootstrapping ends up ignore listed but it's kind of a problem that you can't clear the async stack. <img width="927" alt="Screenshot 2024-05-28 at 11 58 56 PM" src="https://github.com/facebook/react/assets/63648/1c9d32ce-e671-47c8-9d18-9fab3bffabd0"> <img width="431" alt="Screenshot 2024-05-28 at 11 58 07 PM" src="https://github.com/facebook/react/assets/63648/52f57518-bbed-400e-952d-6650835ac6b6"> <img width="327" alt="Screenshot 2024-05-28 at 11 58 31 PM" src="https://github.com/facebook/react/assets/63648/d311a639-79a1-457f-9a46-4f3298d07e65"> <img width="817" alt="Screenshot 2024-05-28 at 11 59 12 PM" src="https://github.com/facebook/react/assets/63648/3aefd356-acf4-4daa-bdbf-b8c8345f6d4b"> DiffTrain build for [9d4fba0](9d4fba0)
sebmarkbage
added a commit
that referenced
this pull request
May 30, 2024
Follow up to #29632. It's possible for `eval` to throw such as if we're in a CSP environment. This is non-essential debug information. We can still proceed to create a fake stack entry. It'll still have the right name. It just won't have the right line/col number nor source url/source map. It might also be ignored listed since it's inside Flight.
github-actions bot
pushed a commit
that referenced
this pull request
May 30, 2024
Follow up to #29632. It's possible for `eval` to throw such as if we're in a CSP environment. This is non-essential debug information. We can still proceed to create a fake stack entry. It'll still have the right name. It just won't have the right line/col number nor source url/source map. It might also be ignored listed since it's inside Flight. DiffTrain build for commit 9710853.
github-actions bot
pushed a commit
that referenced
this pull request
May 30, 2024
Follow up to #29632. It's possible for `eval` to throw such as if we're in a CSP environment. This is non-essential debug information. We can still proceed to create a fake stack entry. It'll still have the right name. It just won't have the right line/col number nor source url/source map. It might also be ignored listed since it's inside Flight. DiffTrain build for [9710853](9710853)
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.
We have three kinds of stacks that we send in the RSC protocol:
console.logwas called on the server.These stack frames disappear in native stacks on the client since they're executed on the server. This evals a fake file which only has one call in it on the same line/column as the server. Then we call through these fake modules to "replay" the callstack. We then replay the
console.logwithin this stack, or callconsole.createTaskin this stack to recreate the stack.The main concern with this approach is the performance. It adds significant cost to create all these eval:ed functions but it should eventually balance out.
This doesn't yet apply source maps to these. With source maps it'll be able to show the server source code when clicking the links.
I don't love how these appear.
<...>task name.(async)suffix Chrome adds is still a problem.