[Flight] Normalize Stack Using Fake Evals #30401
Merged
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
commented
Jul 19, 2024
| @@ -0,0 +1,42 @@ | |||
| /** | |||
There was a problem hiding this comment.
I think it's likely that Fiber/Fizz/Flight OwnerStack.js merge into a shared helper for extracting a presentable string.
sebmarkbage
commented
Jul 19, 2024
| } | ||
| return stack.slice(startIdx, endIdx); | ||
| } | ||
|
|
||
| function filterDebugStack(error: Error): string { |
There was a problem hiding this comment.
This is different from the one in FlightOwnerStack which is for formatting the string for captureOwnerStack() for presentation.
This one is for formatting the string into the serializable stack field for serialization. This is turning into a structured form instead of a string in the follow up #30410.
This could be coming from the current server, however, this Error could also be one that includes fake stack frames it originated on another server. In this case we'd need to do something to reverse engineer it to get it back into its serialized form. E.g. stripping the rsc://React/ prefixes.
sebmarkbage
commented
Jul 20, 2024
sebmarkbage
force-pushed
the
flightnormalizestack
branch
from
a83d050
to
1794201
Compare
|
There's three "urls/files" essentially:
While it would be enough for us to emit the original source file URL + original line number + column number in the stack to get it pointing to the right place, we don't actually have this information in React. It is also not enough to point to the url loaded by the server, because the browser doesn't have access to this. E.g. if it's a bundled chunk that is never loaded by the browser. The browser only sees the fake eval and the source map pointing to the original source file. Even if it's not bundled and just transpiled we'd have the file but we'd not have the original line number. We also need to generate more than one fake eval per file so we give it the |
eps1lon
approved these changes
Jul 22, 2024
We already do this for recreating serialized errors. This does it for stacks both in the stacks of parent components, JSX callsites and owners.
This represents the stack trace as a native console.createTask object in the current execution environment. Not as part of the serialized protocol.
This represents the stack in the current execution environment. As opposed to the .stack property which represents the stack in the transport protocol.
sebmarkbage
force-pushed
the
flightnormalizestack
branch
from
1794201
to
523ef76
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Jul 22, 2024
Stacked on #30400 and #30369 Previously we were using fake evals to recreate a stack for console replaying and thrown errors. However, for owner stacks we just used the raw string that came from the server. This means that the format of the owner stack could include different formats. Like Spidermonkey format for the client components and V8 for the server components. This means that this stack can't be parsed natively by the browser like when printing them as error like in #30289. Additionally, since there's no source file registered with that name and no source mapping url, it can't be source mapped. Before: <img width="1329" alt="before-firefox" src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4"> Instead, we need to create a fake stack like we do for the other things. That way when it's printed as an Error it gets source mapped. It also means that the format is consistently in the native format of the current browser. After: <img width="753" alt="after-firefox" src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3"> So this is nice because you can just take the result from `captureOwnerStack()` and append it to an `Error` stack and print it natively. E.g. this is what React DevTools will do. If you want to parse and present it yourself though it's a bit awkward though. The `captureOwnerStack()` API now includes a bunch of `rsc://React/` URLs. These don't really have any direct connection to the source map. Only the browser knows this connection from the eval. You basically have to strip the prefix and then manually pass the remainder to your own `findSourceMapURL`. Another awkward part is that since Safari doesn't support eval sourceURL exposed into `error.stack` - it means that `captureOwnerStack()` get an empty location for server components since the fake eval doesn't work there. That's not a big deal since these stacks are already broken even for client modules for many because the `eval-source-map` strategy in Webpack doesn't work in Safari for this same reason. A lot of this refactoring is just clarifying that there's three kind of ReactComponentInfo fields: - `stack` - The raw stack as described on the original server. - `debugStack` - The Error object containing the stack as represented in the current client as fake evals. - `debugTask` - The same thing as `debugStack` but described in terms of a native `console.createTask`. DiffTrain build for [b15c198](b15c198)
sebmarkbage
added a commit
that referenced
this pull request
Jul 22, 2024
) Stacked on #30401. Previously we were transferring the original V8 stack trace string to the client and then parsing it there. However, really the server is the one that knows what format it is and it should be able to vary by server environment. We also don't use the raw string anymore (at least not in enableOwnerStacks). We always create the native Error stacks. The string also made it unclear which environment it is and it was tempting to just use it as is. Instead I parse it on the server and make it a structured stack in the transfer format. It also makes it clear that it needs to be formatted in the current environment before presented.
felixshiftellecon
added a commit
to felixshiftellecon/react
that referenced
this pull request
Jul 24, 2024
…ebook#30410) Stacked on facebook#30401. Previously we were transferring the original V8 stack trace string to the client and then parsing it there. However, really the server is the one that knows what format it is and it should be able to vary by server environment. We also don't use the raw string anymore (at least not in enableOwnerStacks). We always create the native Error stacks. The string also made it unclear which environment it is and it was tempting to just use it as is. Instead I parse it on the server and make it a structured stack in the transfer format. It also makes it clear that it needs to be formatted in the current environment before presented.
felixshiftellecon
added a commit
to felixshiftellecon/react
that referenced
this pull request
Jul 24, 2024
Stacked on facebook#30400 and facebook#30369 Previously we were using fake evals to recreate a stack for console replaying and thrown errors. However, for owner stacks we just used the raw string that came from the server. This means that the format of the owner stack could include different formats. Like Spidermonkey format for the client components and V8 for the server components. This means that this stack can't be parsed natively by the browser like when printing them as error like in facebook#30289. Additionally, since there's no source file registered with that name and no source mapping url, it can't be source mapped. Before: <img width="1329" alt="before-firefox" src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4"> Instead, we need to create a fake stack like we do for the other things. That way when it's printed as an Error it gets source mapped. It also means that the format is consistently in the native format of the current browser. After: <img width="753" alt="after-firefox" src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3"> So this is nice because you can just take the result from `captureOwnerStack()` and append it to an `Error` stack and print it natively. E.g. this is what React DevTools will do. If you want to parse and present it yourself though it's a bit awkward though. The `captureOwnerStack()` API now includes a bunch of `rsc://React/` URLs. These don't really have any direct connection to the source map. Only the browser knows this connection from the eval. You basically have to strip the prefix and then manually pass the remainder to your own `findSourceMapURL`. Another awkward part is that since Safari doesn't support eval sourceURL exposed into `error.stack` - it means that `captureOwnerStack()` get an empty location for server components since the fake eval doesn't work there. That's not a big deal since these stacks are already broken even for client modules for many because the `eval-source-map` strategy in Webpack doesn't work in Safari for this same reason. A lot of this refactoring is just clarifying that there's three kind of ReactComponentInfo fields: - `stack` - The raw stack as described on the original server. - `debugStack` - The Error object containing the stack as represented in the current client as fake evals. - `debugTask` - The same thing as `debugStack` but described in terms of a native `console.createTask`.
felixshiftellecon
added a commit
to felixshiftellecon/react
that referenced
this pull request
Jul 24, 2024
…ebook#30410) Stacked on facebook#30401. Previously we were transferring the original V8 stack trace string to the client and then parsing it there. However, really the server is the one that knows what format it is and it should be able to vary by server environment. We also don't use the raw string anymore (at least not in enableOwnerStacks). We always create the native Error stacks. The string also made it unclear which environment it is and it was tempting to just use it as is. Instead I parse it on the server and make it a structured stack in the transfer format. It also makes it clear that it needs to be formatted in the current environment before presented.
felixshiftellecon
added a commit
to felixshiftellecon/react
that referenced
this pull request
Jul 24, 2024
Stacked on facebook#30400 and facebook#30369 Previously we were using fake evals to recreate a stack for console replaying and thrown errors. However, for owner stacks we just used the raw string that came from the server. This means that the format of the owner stack could include different formats. Like Spidermonkey format for the client components and V8 for the server components. This means that this stack can't be parsed natively by the browser like when printing them as error like in facebook#30289. Additionally, since there's no source file registered with that name and no source mapping url, it can't be source mapped. Before: <img width="1329" alt="before-firefox" src="https://github.com/user-attachments/assets/cbe03f9c-96ac-48fb-b58f-f3a224a774f4"> Instead, we need to create a fake stack like we do for the other things. That way when it's printed as an Error it gets source mapped. It also means that the format is consistently in the native format of the current browser. After: <img width="753" alt="after-firefox" src="https://github.com/user-attachments/assets/b436f1f5-ca37-4203-b29f-df9828c9fad3"> So this is nice because you can just take the result from `captureOwnerStack()` and append it to an `Error` stack and print it natively. E.g. this is what React DevTools will do. If you want to parse and present it yourself though it's a bit awkward though. The `captureOwnerStack()` API now includes a bunch of `rsc://React/` URLs. These don't really have any direct connection to the source map. Only the browser knows this connection from the eval. You basically have to strip the prefix and then manually pass the remainder to your own `findSourceMapURL`. Another awkward part is that since Safari doesn't support eval sourceURL exposed into `error.stack` - it means that `captureOwnerStack()` get an empty location for server components since the fake eval doesn't work there. That's not a big deal since these stacks are already broken even for client modules for many because the `eval-source-map` strategy in Webpack doesn't work in Safari for this same reason. A lot of this refactoring is just clarifying that there's three kind of ReactComponentInfo fields: - `stack` - The raw stack as described on the original server. - `debugStack` - The Error object containing the stack as represented in the current client as fake evals. - `debugTask` - The same thing as `debugStack` but described in terms of a native `console.createTask`.
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 #30400 and #30369
Previously we were using fake evals to recreate a stack for console replaying and thrown errors. However, for owner stacks we just used the raw string that came from the server.
This means that the format of the owner stack could include different formats. Like Spidermonkey format for the client components and V8 for the server components. This means that this stack can't be parsed natively by the browser like when printing them as error like in #30289. Additionally, since there's no source file registered with that name and no source mapping url, it can't be source mapped.
Before:
Instead, we need to create a fake stack like we do for the other things. That way when it's printed as an Error it gets source mapped. It also means that the format is consistently in the native format of the current browser.
After:
So this is nice because you can just take the result from
captureOwnerStack()and append it to anErrorstack and print it natively. E.g. this is what React DevTools will do.If you want to parse and present it yourself though it's a bit awkward though. The
captureOwnerStack()API now includes a bunch ofrsc://React/URLs. These don't really have any direct connection to the source map. Only the browser knows this connection from the eval. You basically have to strip the prefix and then manually pass the remainder to your ownfindSourceMapURL.Another awkward part is that since Safari doesn't support eval sourceURL exposed into
error.stack- it means thatcaptureOwnerStack()get an empty location for server components since the fake eval doesn't work there. That's not a big deal since these stacks are already broken even for client modules for many because theeval-source-mapstrategy in Webpack doesn't work in Safari for this same reason.A lot of this refactoring is just clarifying that there's three kind of ReactComponentInfo fields:
stack- The raw stack as described on the original server.debugStack- The Error object containing the stack as represented in the current client as fake evals.debugTask- The same thing asdebugStackbut described in terms of a nativeconsole.createTask.