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] Normalize Stack Using Fake Evals #30401

Merged
merged 3 commits into from
Jul 22, 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
97 changes: 88 additions & 9 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -689,11 +689,21 @@ function createElement(
value: null,
});
if (enableOwnerStacks) {
let normalizedStackTrace: null | Error = null;
if (stack !== null) {
// We create a fake stack and then create an Error object inside of it.
// This means that the stack trace is now normalized into the native format
// of the browser and the stack frames will have been registered with
// source mapping information.
// This can unfortunately happen within a user space callstack which will
// remain on the stack.
normalizedStackTrace = createFakeJSXCallStackInDEV(response, stack);
}
Object.defineProperty(element, '_debugStack', {
configurable: false,
enumerable: false,
writable: true,
value: stack,
value: normalizedStackTrace,
});

let task: null | ConsoleTask = null;
Expand Down Expand Up @@ -724,6 +734,12 @@ function createElement(
writable: true,
value: task,
});

// This owner should ideally have already been initialized to avoid getting
// user stack frames on the stack.
if (owner !== null) {
initializeFakeStack(response, owner);
}
}
}

Expand Down Expand Up @@ -752,9 +768,9 @@ function createElement(
};
if (enableOwnerStacks) {
// $FlowFixMe[cannot-write]
erroredComponent.stack = element._debugStack;
erroredComponent.debugStack = element._debugStack;
// $FlowFixMe[cannot-write]
erroredComponent.task = element._debugTask;
erroredComponent.debugTask = element._debugTask;
}
erroredChunk._debugInfo = [erroredComponent];
}
Expand Down Expand Up @@ -915,9 +931,9 @@ function waitForReference<T>(
};
if (enableOwnerStacks) {
// $FlowFixMe[cannot-write]
erroredComponent.stack = element._debugStack;
erroredComponent.debugStack = element._debugStack;
// $FlowFixMe[cannot-write]
erroredComponent.task = element._debugTask;
erroredComponent.debugTask = element._debugTask;
}
const chunkDebugInfo: ReactDebugInfo =
chunk._debugInfo || (chunk._debugInfo = []);
Expand Down Expand Up @@ -2001,16 +2017,23 @@ function initializeFakeTask(
response: Response,
debugInfo: ReactComponentInfo | ReactAsyncInfo,
): null | ConsoleTask {
if (!supportsCreateTask || typeof debugInfo.stack !== 'string') {
if (!supportsCreateTask) {
return null;
}
const componentInfo: ReactComponentInfo = (debugInfo: any); // Refined
const stack: string = debugInfo.stack;
const cachedEntry = componentInfo.task;
const cachedEntry = componentInfo.debugTask;
if (cachedEntry !== undefined) {
return cachedEntry;
}

if (typeof debugInfo.stack !== 'string') {
// If this is an error, we should've really already initialized the task.
// If it's null, we can't initialize a task.
return null;
}

const stack = debugInfo.stack;

const ownerTask =
componentInfo.owner == null
? null
Expand All @@ -2034,10 +2057,63 @@ function initializeFakeTask(
componentTask = ownerTask.run(callStack);
}
// $FlowFixMe[cannot-write]: We consider this part of initialization.
componentInfo.task = componentTask;
componentInfo.debugTask = componentTask;
return componentTask;
}

const createFakeJSXCallStack = {
'react-stack-bottom-frame': function (
response: Response,
stack: string,
): Error {
const callStackForError = buildFakeCallStack(
response,
stack,
fakeJSXCallSite,
);
return callStackForError();
},
};

const createFakeJSXCallStackInDEV: (
response: Response,
stack: string,
) => Error = __DEV__
? // We use this technique to trick minifiers to preserve the function name.
(createFakeJSXCallStack['react-stack-bottom-frame'].bind(
createFakeJSXCallStack,
): any)
: (null: any);

/** @noinline */
function fakeJSXCallSite() {
// This extra call frame represents the JSX creation function. We always pop this frame
// off before presenting so it needs to be part of the stack.
return new Error('react-stack-top-frame');
}

function initializeFakeStack(
response: Response,
debugInfo: ReactComponentInfo | ReactAsyncInfo,
): void {
const cachedEntry = debugInfo.debugStack;
if (cachedEntry !== undefined) {
return;
}
if (typeof debugInfo.stack === 'string') {
// $FlowFixMe[cannot-write]
// $FlowFixMe[prop-missing]
debugInfo.debugStack = createFakeJSXCallStackInDEV(
response,
debugInfo.stack,
);
}
if (debugInfo.owner != null) {
// Initialize any owners not yet initialized.
initializeFakeStack(response, debugInfo.owner);
}
}

function resolveDebugInfo(
response: Response,
id: number,
Expand All @@ -2054,6 +2130,8 @@ function resolveDebugInfo(
// render phase so we're not inside a user space stack at this point. If we waited
// to initialize it when we need it, we might be inside user code.
initializeFakeTask(response, debugInfo);
initializeFakeStack(response, debugInfo);

const chunk = getChunk(response, id);
const chunkDebugInfo: ReactDebugInfo =
chunk._debugInfo || (chunk._debugInfo = []);
Expand Down Expand Up @@ -2096,6 +2174,7 @@ function resolveConsoleEntry(
);
if (owner != null) {
const task = initializeFakeTask(response, owner);
initializeFakeStack(response, owner);
if (task !== null) {
task.run(callStack);
return;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function normalizeCodeLocInfo(str) {

function normalizeComponentInfo(debugInfo) {
if (typeof debugInfo.stack === 'string') {
const {task, ...copy} = debugInfo;
const {debugTask, debugStack, ...copy} = debugInfo;
copy.stack = normalizeCodeLocInfo(debugInfo.stack);
if (debugInfo.owner) {
copy.owner = normalizeComponentInfo(debugInfo.owner);
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/__tests__/ReactUpdates-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1858,14 +1858,14 @@ describe('ReactUpdates', () => {

let error = null;
let ownerStack = null;
let nativeStack = null;
let debugStack = null;
const originalConsoleError = console.error;
console.error = e => {
error = e;
ownerStack = gate(flags => flags.enableOwnerStacks)
? React.captureOwnerStack()
: null;
nativeStack = new Error().stack;
debugStack = new Error().stack;
Scheduler.log('stop');
};
try {
Expand All @@ -1879,7 +1879,7 @@ describe('ReactUpdates', () => {

expect(error).toContain('Maximum update depth exceeded');
// The currently executing effect should be on the native stack
expect(nativeStack).toContain('at myEffect');
expect(debugStack).toContain('at myEffect');
if (gate(flags => flags.enableOwnerStacks)) {
expect(ownerStack).toContain('at App');
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -2015,7 +2015,7 @@ function createChildReconciler(
if (typeof debugInfo[i].stack === 'string') {
throwFiber._debugOwner = (debugInfo[i]: any);
if (enableOwnerStacks) {
throwFiber._debugTask = debugInfo[i].task;
throwFiber._debugTask = debugInfo[i].debugTask;
}
break;
}
Expand Down
14 changes: 5 additions & 9 deletions packages/react-reconciler/src/ReactFiberComponentStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,17 +165,13 @@ export function getOwnerStackByFiberInDev(workInProgress: Fiber): string {
info += '\n' + debugStack;
}
}
} else if (typeof owner.stack === 'string') {
} else if (owner.debugStack != null) {
// Server Component
// The Server Component stack can come from a different VM that formats it different.
// Likely V8. Since Chrome based browsers support createTask which is going to use
// another code path anyway. I.e. this is likely NOT a V8 based browser.
// This will cause some of the stack to have different formatting.
// TODO: Normalize server component stacks to the client formatting.
const ownerStack: string = owner.stack;
const ownerStack: Error = owner.debugStack;
owner = owner.owner;
if (owner && ownerStack !== '') {
info += '\n' + ownerStack;
if (owner && ownerStack) {
// TODO: Should we stash this somewhere for caching purposes?
info += '\n' + formatOwnerStack(ownerStack);
}
} else {
break;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberOwnerStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function filterDebugStack(error: Error): string {
// To keep things light we exclude the entire trace in this case.
return '';
}
const frames = stack.split('\n').slice(1);
const frames = stack.split('\n').slice(1); // Pop the JSX frame.
return frames.filter(isNotExternal).join('\n');
}

Expand Down
27 changes: 19 additions & 8 deletions packages/react-server/src/ReactFizzComponentStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,21 +159,32 @@ export function getOwnerStackByComponentStackNodeInDev(
componentStack;

while (owner) {
let debugStack: void | null | string | Error = owner.stack;
if (typeof debugStack !== 'string' && debugStack != null) {
// Stash the formatted stack so that we can avoid redoing the filtering.
// $FlowFixMe[cannot-write]: This has been refined to a ComponentStackNode.
owner.stack = debugStack = formatOwnerStack(debugStack);
let ownerStack: ?string = null;
if (owner.debugStack != null) {
// Server Component
// TODO: Should we stash this somewhere for caching purposes?
ownerStack = formatOwnerStack(owner.debugStack);
owner = owner.owner;
} else if (owner.stack != null) {
// Client Component
const node: ComponentStackNode = (owner: any);
if (typeof owner.stack !== 'string') {
ownerStack = node.stack = formatOwnerStack(owner.stack);
} else {
ownerStack = owner.stack;
}
owner = owner.owner;
} else {
owner = owner.owner;
}
owner = owner.owner;
// If we don't actually print the stack if there is no owner of this JSX element.
// In a real app it's typically not useful since the root app is always controlled
// by the framework. These also tend to have noisy stacks because they're not rooted
// in a React render but in some imperative bootstrapping code. It could be useful
// if the element was created in module scope. E.g. hoisted. We could add a a single
// stack frame for context for example but it doesn't say much if that's a wrapper.
if (owner && debugStack) {
info += '\n' + debugStack;
if (owner && ownerStack) {
info += '\n' + ownerStack;
}
}
return info;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-server/src/ReactFizzOwnerStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function filterDebugStack(error: Error): string {
// To keep things light we exclude the entire trace in this case.
return '';
}
const frames = stack.split('\n').slice(1);
const frames = stack.split('\n').slice(1); // Pop the JSX frame.
return frames.filter(isNotExternal).join('\n');
}

Expand Down
6 changes: 3 additions & 3 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -859,17 +859,17 @@ function pushServerComponentStack(
if (typeof componentInfo.name !== 'string') {
continue;
}
if (enableOwnerStacks && componentInfo.stack === undefined) {
if (enableOwnerStacks && componentInfo.debugStack === undefined) {
continue;
}
task.componentStack = {
parent: task.componentStack,
type: componentInfo,
owner: componentInfo.owner,
stack: componentInfo.stack,
stack: enableOwnerStacks ? componentInfo.debugStack : null,
};
if (enableOwnerStacks) {
task.debugTask = (componentInfo.task: any);
task.debugTask = (componentInfo.debugTask: any);
}
}
}
Expand Down
42 changes: 42 additions & 0 deletions packages/react-server/src/ReactFlightOwnerStack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
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 think it's likely that Fiber/Fizz/Flight OwnerStack.js merge into a shared helper for extracting a presentable string.

* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

// TODO: Make this configurable on the Request.
const externalRegExp = /\/node\_modules\/| \(node\:| node\:|\(\<anonymous\>\)/;

function isNotExternal(stackFrame: string): boolean {
return !externalRegExp.test(stackFrame);
}

function filterDebugStack(error: Error): string {
// 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
// the DevTools or framework's ignore lists to filter them out.
let stack = error.stack;
if (stack.startsWith('Error: react-stack-top-frame\n')) {
// V8's default formatting prefixes with the error message which we
// don't want/need.
stack = stack.slice(29);
}
let idx = stack.indexOf('react-stack-bottom-frame');
if (idx !== -1) {
idx = stack.lastIndexOf('\n', idx);
}
if (idx !== -1) {
// Cut off everything after the bottom frame since it'll be internals.
stack = stack.slice(0, idx);
}
const frames = stack.split('\n').slice(1); // Pop the JSX frame.
return frames.filter(isNotExternal).join('\n');
}

export function formatOwnerStack(ownerStackTrace: Error): string {
return filterDebugStack(ownerStackTrace);
}
Loading
Loading