SIGN IN SIGN UP
facebook / react UNCLAIMED

The library for web and native user interfaces.

0 0 858 JavaScript
/**
* 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
*/
import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev';
import ReactSharedInternals from 'shared/ReactSharedInternals';
2024-11-05 15:05:04 -05:00
import DefaultPrepareStackTrace from 'shared/DefaultPrepareStackTrace';
import {formatOwnerStack} from './ReactOwnerStackFrames';
let prefix;
let suffix;
export function describeBuiltInComponentFrame(name: string): string {
if (prefix === undefined) {
// Extract the VM specific prefix used by each line.
try {
throw Error();
} catch (x) {
const match = x.stack.trim().match(/\n( *(at )?)/);
prefix = (match && match[1]) || '';
suffix =
x.stack.indexOf('\n at') > -1
? // V8
' (<anonymous>)'
: // JSC/Spidermonkey
x.stack.indexOf('@') > -1
? '@unknown:0:0'
: // Other
'';
}
}
// We use the prefix to ensure our stacks line up with native stack frames.
return '\n' + prefix + name + suffix;
}
export function describeDebugInfoFrame(
name: string,
env: ?string,
location: ?Error,
): string {
if (location != null) {
// If we have a location, it's the child's owner stack. Treat the bottom most frame as
// the location of this function.
const childStack = formatOwnerStack(location);
const idx = childStack.lastIndexOf('\n');
const lastLine = idx === -1 ? childStack : childStack.slice(idx + 1);
if (lastLine.indexOf(name) !== -1) {
// For async stacks it's possible we don't have the owner on it. As a precaution only
// use this frame if it has the name of the function in it.
return '\n' + lastLine;
}
}
return describeBuiltInComponentFrame(name + (env ? ' [' + env + ']' : ''));
2024-02-23 12:04:55 -05:00
}
let reentry = false;
let componentFrameCache;
if (__DEV__) {
const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
componentFrameCache = new PossiblyWeakMap<Function, string>();
}
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
/**
* Leverages native browser/VM stack frames to get proper details (e.g.
* filename, line + col number) for a single component in a component stack. We
* do this by:
* (1) throwing and catching an error in the function - this will be our
* control error.
* (2) calling the component which will eventually throw an error that we'll
* catch - this will be our sample error.
* (3) diffing the control and sample error stacks to find the stack frame
* which represents our component.
*/
export function describeNativeComponentFrame(
fn: Function,
construct: boolean,
): string {
// If something asked for a stack inside a fake render, it should get ignored.
if (!fn || reentry) {
return '';
}
if (__DEV__) {
const frame = componentFrameCache.get(fn);
if (frame !== undefined) {
return frame;
}
}
reentry = true;
const previousPrepareStackTrace = Error.prepareStackTrace;
2024-11-05 15:05:04 -05:00
Error.prepareStackTrace = DefaultPrepareStackTrace;
let previousDispatcher = null;
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
if (__DEV__) {
previousDispatcher = ReactSharedInternals.H;
// Set the dispatcher in DEV because this might be call in the render function
// for warnings.
ReactSharedInternals.H = null;
disableLogs();
}
try {
/**
* Finding a common stack frame between sample and control errors can be
* tricky given the different types and levels of stack trace truncation from
* different JS VMs. So instead we'll attempt to control what that common
* frame should be through this object method:
* Having both the sample and control errors be in the function under the
* `DescribeNativeComponentFrameRoot` property, + setting the `name` and
* `displayName` properties of the function ensures that a stack
* frame exists that has the method name `DescribeNativeComponentFrameRoot` in
* it for both control and sample stacks.
*/
const RunInRootFrame = {
DetermineComponentFrameRoot(): [?string, ?string] {
let control;
try {
// This should throw.
if (construct) {
// Something should be setting the props in the constructor.
const Fake = function () {
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
throw Error();
};
// $FlowFixMe[prop-missing]
Object.defineProperty(Fake.prototype, 'props', {
set: function () {
// We use a throwing setter instead of frozen or non-writable props
// because that won't throw in a non-strict mode function.
throw Error();
},
});
if (typeof Reflect === 'object' && Reflect.construct) {
// We construct a different control for this case to include any extra
// frames added by the construct call.
try {
Reflect.construct(Fake, []);
} catch (x) {
control = x;
}
Reflect.construct(fn, [], Fake);
} else {
try {
Fake.call();
} catch (x) {
control = x;
}
// $FlowFixMe[prop-missing] found when upgrading Flow
fn.call(Fake.prototype);
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
}
} else {
try {
throw Error();
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
} catch (x) {
control = x;
}
// TODO(luna): This will currently only throw if the function component
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too
const maybePromise = fn();
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
// If the function component returns a promise, it's likely an async
// component, which we don't yet support. Attach a noop catch handler to
// silence the error.
// TODO: Implement component stacks for async client components?
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
if (sample && control && typeof sample.stack === 'string') {
return [sample.stack, control.stack];
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
}
}
return [null, null];
},
};
// $FlowFixMe[prop-missing]
RunInRootFrame.DetermineComponentFrameRoot.displayName =
'DetermineComponentFrameRoot';
const namePropDescriptor = Object.getOwnPropertyDescriptor(
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
RunInRootFrame.DetermineComponentFrameRoot,
'name',
);
// Before ES6, the `name` property was not configurable.
if (namePropDescriptor && namePropDescriptor.configurable) {
// V8 utilizes a function's `name` property when generating a stack trace.
Object.defineProperty(
RunInRootFrame.DetermineComponentFrameRoot,
// Configurable properties can be updated even if its writable descriptor
// is set to `false`.
// $FlowFixMe[cannot-write]
'name',
{value: 'DetermineComponentFrameRoot'},
);
}
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
const [sampleStack, controlStack] =
RunInRootFrame.DetermineComponentFrameRoot();
if (sampleStack && controlStack) {
// This extracts the first frame from the sample that isn't also in the control.
// Skipping one frame that we assume is the frame that calls the two.
Update stack diffing algorithm in describeNativeComponentFrame (#27132) ## Summary There's a bug with the existing stack comparison algorithm in `describeNativeComponentFrame` — specifically how it attempts to find a common root frame between the control and sample stacks. This PR attempts to fix that bug by injecting a frame that can have a guaranteed string in it for us to search for in both stacks to find a common root. ## Brief Background/How it works now Right now `describeNativeComponentFrame` does the following to leverage native browser/VM stack frames to get details (e.g. script path, row and col #s) for a single component: 1. Throwing and catching a control error in the function 2. Calling the component which should eventually throw an error (most of the time), that we'll catch as our sample error. 3. Diffing the stacks in the control and sample errors to find the line which should represent our component call. ## What's broken To account for potential stack trace truncation, the stack diffing algorithm first attempts to find a common "root" frame by inspecting the earliest frame of the sample stack and searching for an identical frame in the control stack starting from the bottom. However, there are a couple of scenarios which I've hit that cause the above approach to not work correctly. First, it's possible that for render passes of extremely large component trees to have a lot of repeating internal react function calls, which can result in an incorrect common or "root" frame found. Here's a small example from a stack trace using React Fizz for SSR. Our control frame can look like this: ``` Error: at Fake (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Actual common root frame with the sample stack at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Incorrectly chosen common root frame at renderNodeDestructive (...) ``` And our sample stack can look like this: ``` Error: at set (...) at PureComponent (...) at call (native) at apply (native) at ErrorBoundary (...) at construct (native) at describeNativeComponentFrame (...) at describeClassComponentFrame (...) at getStackByComponentStackNode (...) at getCurrentStackInDEV (...) at renderNodeDestructive (...) at renderElement (...) at renderNodeDestructiveImpl (...) // <-- Root frame that's common in the control stack ``` Here you can see that the earliest trace in the sample stack, the `renderNodeDestructiveImpl` call, can exactly match with multiple `renderNodeDestructiveImpl` calls in the control stack (including file path and line + col #s). Currently the algorithm will chose the earliest/last frame with the `renderNodeDestructiveImpl` call (which is the second last frame in our control stack), which is incorrect. The actual matching frame in the control stack is the latest or first frame (when traversing from the top) with the `renderNodeDestructiveImpl` call. This leads to the rest of the stack diffing associating an incorrect frame (`at getStackByComponentStackNode (...)`) for the component. Another issue with this approach is that it assumes all VMs will truncate stack traces at the *bottom*, [which isn't the case for the Hermes VM](https://github.com/facebook/hermes/blob/df07cf713a84a4434c83c08cede38ba438dc6aca/lib/VM/JSError.cpp#L688-L699) which **truncates stack traces in the middle**, placing a ``` at renderNodeDestructiveImpl (...) ... skipping {n} frames at renderNodeDestructive (...) ``` line in the middle of the stack trace for all stacks that contain more than 100 traces. This causes stack traces for React Native apps using the Hermes VM to potentially break for large component trees. Although for this specific case with Hermes, it's possible to account for this by either manually grepping and removing the `... skipping` line and everything below it (see draft PR: #26999), or by implementing the non-standard `prepareStackTrace` API which Hermes also supports to manually generate a stack trace that truncates from the bottom ([example implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-hermes-fix)). ## The Fix I found different ways to go about fixing this. The first was to search for a common stack frame starting from the top/latest frame. It's a relatively small change ([see implementation](https://github.com/facebook/react/compare/main...KarimP:react:component-stack-fix-2)), although it is less performant by being n^2 (albeit with `n` realistically being <= 5 here). It's also a bit more buggy for class components given that different VMs insert a different amount of additional lines for new/construct calls... Another fix would be to actually implement a [longest common substring](https://en.wikipedia.org/wiki/Longest_common_substring) algorithm, which can also be roughly n^2 time (assuming the longest common substring between control and sample will be most of the sample frame). The fix I ended up going with was have the lines that throw the control error and the lines that call/instantiate the component be inside a distinct method under an object property (`"DescribeNativeComponentFrameRoot"`). All major VMs (Safari's JavaScriptCore, Firefox's SpiderMonkey, V8, Hermes, and Bun) should display the object property name their stack trace. I've also set the `name` and `displayName` properties for method as well to account for minification, any advanced optimizations (e.g. key crushing), and VM inconsistencies (both Bun and Safari seem to exclusively use the value under `displayName` and not `name` in traces for methods defined under an object's own property...). We can then find this "common" frame by simply finding the line that has our special method name (`"DescribeNativeComponentFrameRoot"`), and the rest of the code to determine the actual component line works as expected. If by any chance we don't find a frame with our special method name in either control or sample stack traces, we then revert back to the existing approach mentioned above by searching for the last line of the sample frame in the control frame. ## How did you test this change? 1. There are bunch of existing tests that ensure a properly formatted component trace is logged for certain scenarios, so I ensured the existing full test suite passed 2. I threw an error in a component that's deep in the component hierarchy of a large React app (facebook) to ensure there's stack trace truncation, and ensured the correct component stack trace was logged for Chrome, Safari, and Firefox, and with and without minification. 3. Ran a large React app (facebook) on the Hermes VM, threw an error in a component that's deep in the component hierarchy, and ensured that component frames are generated despite stack traces being truncated in the middle.
2023-11-08 11:45:31 -05:00
const sampleLines = sampleStack.split('\n');
const controlLines = controlStack.split('\n');
let s = 0;
let c = 0;
while (
s < sampleLines.length &&
!sampleLines[s].includes('DetermineComponentFrameRoot')
) {
s++;
}
while (
c < controlLines.length &&
!controlLines[c].includes('DetermineComponentFrameRoot')
) {
c++;
}
// We couldn't find our intentionally injected common root frame, attempt
// to find another common root frame by search from the bottom of the
// control stack...
if (s === sampleLines.length || c === controlLines.length) {
s = sampleLines.length - 1;
c = controlLines.length - 1;
while (s >= 1 && c >= 0 && sampleLines[s] !== controlLines[c]) {
// We expect at least one stack frame to be shared.
// Typically this will be the root most one. However, stack frames may be
// cut off due to maximum stack limits. In this case, one maybe cut off
// earlier than the other. We assume that the sample is longer or the same
// and there for cut off earlier. So we should find the root most frame in
// the sample somewhere in the control.
c--;
}
}
for (; s >= 1 && c >= 0; s--, c--) {
// Next we find the first one that isn't the same which should be the
// frame that called our sample function and the control.
if (sampleLines[s] !== controlLines[c]) {
// In V8, the first line is describing the message but other VMs don't.
// If we're about to return the first line, and the control is also on the same
// line, that's a pretty good indicator that our sample threw at same line as
// the control. I.e. before we entered the sample frame. So we ignore this result.
// This can happen if you passed a class to function component, or non-function.
if (s !== 1 || c !== 1) {
do {
s--;
c--;
// We may still have similar intermediate frames from the construct call.
// The next one that isn't the same should be our match though.
if (c < 0 || sampleLines[s] !== controlLines[c]) {
// V8 adds a "new" prefix for native classes. Let's remove it to make it prettier.
let frame = '\n' + sampleLines[s].replace(' at new ', ' at ');
// If our component frame is labeled "<anonymous>"
// but we have a user-provided "displayName"
// splice it in to make the stack more readable.
if (fn.displayName && frame.includes('<anonymous>')) {
frame = frame.replace('<anonymous>', fn.displayName);
}
if (__DEV__) {
if (typeof fn === 'function') {
componentFrameCache.set(fn, frame);
}
}
// Return the line we found.
return frame;
}
} while (s >= 1 && c >= 0);
}
break;
}
}
}
} finally {
reentry = false;
if (__DEV__) {
ReactSharedInternals.H = previousDispatcher;
reenableLogs();
}
Error.prepareStackTrace = previousPrepareStackTrace;
}
// Fallback to just using the name if we couldn't make it throw.
const name = fn ? fn.displayName || fn.name : '';
const syntheticFrame = name ? describeBuiltInComponentFrame(name) : '';
if (__DEV__) {
if (typeof fn === 'function') {
componentFrameCache.set(fn, syntheticFrame);
}
}
return syntheticFrame;
}
export function describeClassComponentFrame(ctor: Function): string {
return describeNativeComponentFrame(ctor, true);
}
export function describeFunctionComponentFrame(fn: Function): string {
return describeNativeComponentFrame(fn, false);
}