Skip to content
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

[Flight] Override prepareStackTrace when reading stacks #29740

Merged
merged 1 commit into from
Jun 5, 2024
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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ module.exports = {
$ReadOnlyArray: 'readonly',
$ArrayBufferView: 'readonly',
$Shape: 'readonly',
CallSite: 'readonly',
ConsoleTask: 'readonly', // TOOD: Figure out what the official name of this will be.
ReturnType: 'readonly',
AnimationFrameID: 'readonly',
Expand Down
26 changes: 14 additions & 12 deletions fixtures/flight/server/region.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,11 @@ if (process.env.NODE_ENV === 'development') {
res.set('Content-type', 'application/json');
let requestedFilePath = req.query.name;

let isCompiledOutput = false;
if (requestedFilePath.startsWith('file://')) {
// We assume that if it was prefixed with file:// it's referring to the compiled output
// and if it's a direct file path we assume it's source mapped back to original format.
isCompiledOutput = true;
requestedFilePath = requestedFilePath.slice(7);
}

Expand All @@ -204,11 +208,11 @@ if (process.env.NODE_ENV === 'development') {
let map;
// There are two ways to return a source map depending on what we observe in error.stack.
// A real app will have a similar choice to make for which strategy to pick.
if (!sourceMap || Error.prepareStackTrace === undefined) {
// When --enable-source-maps is enabled, the error.stack that we use to track
// stacks will have had the source map already applied so it's pointing to the
// original source. We return a blank source map that just maps everything to
// the original source in this case.
if (!sourceMap || !isCompiledOutput) {
// If a file doesn't have a source map, such as this file, then we generate a blank
// source map that just contains the original content and segments pointing to the
// original lines.
// Similarly
const sourceContent = await readFile(requestedFilePath, 'utf8');
const lines = sourceContent.split('\n').length;
map = {
Expand All @@ -222,13 +226,11 @@ if (process.env.NODE_ENV === 'development') {
sourceRoot: '',
};
} else {
// If something has overridden prepareStackTrace it is likely not getting the
// natively applied source mapping to error.stack and so the line will point to
// the compiled output similar to how a browser works.
// E.g. ironically this can happen with the source-map-support library that is
// auto-invoked by @babel/register if external source maps are generated.
// In this case we just use the source map that the native source mapping would
// have used.
// We always set prepareStackTrace before reading the stack so that we get the stack
// without source maps applied. Therefore we have to use the original source map.
// If something read .stack before we did, we might observe the line/column after
// source mapping back to the original file. We use the isCompiledOutput check above
// in that case.
map = sourceMap.payload;
}
res.write(JSON.stringify(map));
Expand Down
45 changes: 37 additions & 8 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,41 @@ function isNotExternal(stackFrame: string): boolean {
return !externalRegExp.test(stackFrame);
}

function prepareStackTrace(
error: Error,
structuredStackTrace: CallSite[],
): string {
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();
}
return stack;
}

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.
// So the client still needs to be resilient to this.
const previousPrepare = Error.prepareStackTrace;
Error.prepareStackTrace = prepareStackTrace;
try {
// eslint-disable-next-line react-internal/safe-string-coercion
return String(error.stack);
} finally {
Error.prepareStackTrace = previousPrepare;
}
}

function initCallComponentFrame(): string {
// Extract the stack frame of the callComponentInDEV function.
const error = callComponentInDEV(Error, 'react-stack-top-frame', {});
const stack = error.stack;
const stack = getStack(error);
const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0;
const endIdx = stack.indexOf('\n', startIdx);
if (endIdx === -1) {
Expand All @@ -155,7 +186,7 @@ function initCallIteratorFrame(): string {
(callIteratorInDEV: any)({next: null});
return '';
} catch (error) {
const stack = error.stack;
const stack = getStack(error);
const startIdx = stack.startsWith('TypeError: ')
? stack.indexOf('\n') + 1
: 0;
Expand All @@ -174,7 +205,7 @@ function initCallLazyInitFrame(): string {
_init: Error,
_payload: 'react-stack-top-frame',
});
const stack = error.stack;
const stack = getStack(error);
const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0;
const endIdx = stack.indexOf('\n', startIdx);
if (endIdx === -1) {
Expand All @@ -188,7 +219,7 @@ function filterDebugStack(error: Error): string {
// 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
// the DevTools or framework's ignore lists to filter them out.
let stack = error.stack;
let stack = getStack(error);
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 Expand Up @@ -2575,8 +2606,7 @@ function emitPostponeChunk(
try {
// eslint-disable-next-line react-internal/safe-string-coercion
reason = String(postponeInstance.message);
// eslint-disable-next-line react-internal/safe-string-coercion
stack = String(postponeInstance.stack);
stack = getStack(postponeInstance);
} catch (x) {}
row = serializeRowHeader('P', id) + stringify({reason, stack}) + '\n';
} else {
Expand All @@ -2601,8 +2631,7 @@ function emitErrorChunk(
if (error instanceof Error) {
// eslint-disable-next-line react-internal/safe-string-coercion
message = String(error.message);
// eslint-disable-next-line react-internal/safe-string-coercion
stack = String(error.stack);
stack = getStack(error);
} else if (typeof error === 'object' && error !== null) {
message = describeObjectForErrorMessage(error);
} else {
Expand Down