-
Notifications
You must be signed in to change notification settings - Fork 49.2k
[Flight] Parse Stack Trace from Structured CallSite if available #33135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,23 +9,104 @@ | |
|
||
import type {ReactStackTrace} from 'shared/ReactTypes'; | ||
|
||
import DefaultPrepareStackTrace from 'shared/DefaultPrepareStackTrace'; | ||
let framesToSkip: number = 0; | ||
let collectedStackTrace: null | ReactStackTrace = null; | ||
|
||
function getStack(error: Error): string { | ||
// We override Error.prepareStackTrace with our own version that normalizes | ||
// the stack to V8 formatting even if the server uses other formatting. | ||
// It also ensures that source maps are NOT applied to this since that can | ||
// be slow we're better off doing that lazily from the client instead of | ||
// eagerly on the server. If the stack has already been read, then we might | ||
// not get a normalized stack and it might still have been source mapped. | ||
const previousPrepare = Error.prepareStackTrace; | ||
Error.prepareStackTrace = DefaultPrepareStackTrace; | ||
try { | ||
// eslint-disable-next-line react-internal/safe-string-coercion | ||
return String(error.stack); | ||
} finally { | ||
Error.prepareStackTrace = previousPrepare; | ||
const identifierRegExp = /^[a-zA-Z_$][0-9a-zA-Z_$]*$/; | ||
|
||
function getMethodCallName(callSite: CallSite): string { | ||
const typeName = callSite.getTypeName(); | ||
const methodName = callSite.getMethodName(); | ||
const functionName = callSite.getFunctionName(); | ||
let result = ''; | ||
if (functionName) { | ||
if ( | ||
typeName && | ||
identifierRegExp.test(functionName) && | ||
functionName !== typeName | ||
) { | ||
result += typeName + '.'; | ||
} | ||
result += functionName; | ||
if ( | ||
methodName && | ||
functionName !== methodName && | ||
!functionName.endsWith('.' + methodName) && | ||
!functionName.endsWith(' ' + methodName) | ||
) { | ||
result += ' [as ' + methodName + ']'; | ||
} | ||
} else { | ||
if (typeName) { | ||
result += typeName + '.'; | ||
} | ||
if (methodName) { | ||
result += methodName; | ||
} else { | ||
result += '<anonymous>'; | ||
} | ||
} | ||
return result; | ||
} | ||
|
||
function collectStackTrace( | ||
error: Error, | ||
structuredStackTrace: CallSite[], | ||
): string { | ||
const result: ReactStackTrace = []; | ||
// Collect structured stack traces from the callsites. | ||
// We mirror how V8 serializes stack frames and how we later parse them. | ||
for (let i = framesToSkip; i < structuredStackTrace.length; i++) { | ||
const callSite = structuredStackTrace[i]; | ||
let name = callSite.getFunctionName() || '<anonymous>'; | ||
if (name === 'react-stack-bottom-frame') { | ||
// Skip everything after the bottom frame since it'll be internals. | ||
break; | ||
} else if (callSite.isNative()) { | ||
result.push([name, '', 0, 0]); | ||
} else { | ||
// We encode complex function calls as if they're part of the function | ||
// name since we cannot simulate the complex ones and they look the same | ||
// as function names in UIs on the client as well as stacks. | ||
if (callSite.isConstructor()) { | ||
name = 'new ' + name; | ||
} else if (!callSite.isToplevel()) { | ||
name = getMethodCallName(callSite); | ||
} | ||
if (name === '<anonymous>') { | ||
name = ''; | ||
} | ||
let filename = callSite.getScriptNameOrSourceURL() || '<anonymous>'; | ||
if (filename === '<anonymous>') { | ||
filename = ''; | ||
} | ||
if (callSite.isEval() && !filename) { | ||
const origin = callSite.getEvalOrigin(); | ||
if (origin) { | ||
filename = origin.toString() + ', <anonymous>'; | ||
} | ||
} | ||
const line = callSite.getLineNumber() || 0; | ||
const col = callSite.getColumnNumber() || 0; | ||
result.push([name, filename, line, col]); | ||
} | ||
} | ||
// At the same time we generate a string stack trace just in case someone | ||
// else reads it. Ideally, we'd call the previous prepareStackTrace to | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might have to call the previous one regardless in case the previous one is a custom one that also contains side-effects. Or do that in Next.js depending on the timing. The tricky bit is figuring how if that's the native one we want to avoid because it would be wasteful to call that. |
||
// ensure it's in the expected format but it's common for that to be | ||
// source mapped and since we do a lot of eager parsing of errors, it | ||
// would be slow in those environments. We could maybe just rely on those | ||
// environments having to disable source mapping globally to speed things up. | ||
// For now, we just generate a default V8 formatted stack trace without | ||
// source mapping as a fallback. | ||
const name = error.name || 'Error'; | ||
const message = error.message || ''; | ||
let stack = name + ': ' + message; | ||
for (let i = 0; i < structuredStackTrace.length; i++) { | ||
stack += '\n at ' + structuredStackTrace[i].toString(); | ||
} | ||
collectedStackTrace = result; | ||
return stack; | ||
} | ||
|
||
// This matches either of these V8 formats. | ||
|
@@ -39,7 +120,32 @@ export function parseStackTrace( | |
error: Error, | ||
skipFrames: number, | ||
): ReactStackTrace { | ||
let stack = getStack(error); | ||
// We override Error.prepareStackTrace with our own version that collects | ||
// the structured data. We need more information than the raw stack gives us | ||
// and we need to ensure that we don't get the source mapped version. | ||
collectedStackTrace = null; | ||
framesToSkip = skipFrames; | ||
const previousPrepare = Error.prepareStackTrace; | ||
Error.prepareStackTrace = collectStackTrace; | ||
let stack; | ||
try { | ||
// eslint-disable-next-line react-internal/safe-string-coercion | ||
stack = String(error.stack); | ||
} finally { | ||
Error.prepareStackTrace = previousPrepare; | ||
} | ||
|
||
if (collectedStackTrace !== null) { | ||
const result = collectedStackTrace; | ||
collectedStackTrace = null; | ||
return result; | ||
} | ||
|
||
// If the stack has already been read, or this is not actually a V8 compatible | ||
// engine then we might not get a normalized stack and it might still have been | ||
// source mapped. Regardless we try our best to parse it. This works best if the | ||
// environment just uses default V8 formatting and no source mapping. | ||
|
||
if (stack.startsWith('Error: react-stack-top-frame\n')) { | ||
// V8's default formatting prefixes with the error message which we | ||
// don't want/need. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to test that? Asking because people already complain that our lint rules don't allow unicode characters in Component names so this may come up again here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to mirror what V8 does as much as possible so we get the same from parsing and not.
https://github.com/v8/v8/blob/main/src/objects/call-site-info.cc#L767
However, I didn't dig into what isIdentifier does since you get lost in codegen. I just saw others do this. Usually what I do is this:
https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHydrationDiffs.js#L212-L213