Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,27 @@ function defaultFilterStackFrame(
);
}

// DEV-only cache of parsed and filtered stack frames.
const stackTraceCache: WeakMap<Error, ReactStackTrace> = __DEV__
? new WeakMap()
: (null: any);

function filterStackTrace(
request: Request,
error: Error,
skipFrames: number,
): ReactStackTrace {
const existing = stackTraceCache.get(error);
if (existing !== undefined) {
// Return a clone because the Flight protocol isn't yet resilient to deduping
// objects in the debug info. TODO: Support deduping stacks.
const clone = existing.slice(0);
for (let i = 0; i < clone.length; i++) {
// $FlowFixMe[invalid-tuple-arity]
clone[i] = clone[i].slice(0);
}
return clone;
}
// Since stacks can be quite large and we pass a lot of them, we filter them out eagerly
// to save bandwidth even in DEV. We'll also replay these stacks on the client so by
// stripping them early we avoid that overhead. Otherwise we'd normally just rely on
Expand All @@ -183,6 +199,7 @@ function filterStackTrace(
i--;
}
}
stackTraceCache.set(error, stack);
return stack;
}

Expand Down
138 changes: 122 additions & 16 deletions packages/react-server/src/ReactFlightStackConfigV8.js
Original file line number Diff line number Diff line change
Expand Up @@ -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_$]*$/;
Copy link
Collaborator

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.

Copy link
Collaborator Author

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


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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.
Expand All @@ -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.
Expand Down
Loading