Save native stack as a separate Error

This represents the stack in the current execution environment. As opposed
to the .stack property which represents the stack in the transport protocol.

packages/react-client/src/ReactFlightClient.js

@@ -697,13 +697,7 @@ function createElement(
         // source mapping information.
         // This can unfortunately happen within a user space callstack which will
         // remain on the stack.
-        const callStackForError = buildFakeCallStack(
-          response,
-          stack,
-          // $FlowFixMe[incompatible-use]
-          Error.bind(null, 'react-stack-top-frame'),
-        );
-        normalizedStackTrace = callStackForError();
+        normalizedStackTrace = createFakeJSXCallStackInDEV(response, stack);
       }
       Object.defineProperty(element, '_debugStack', {
         configurable: false,
@@ -774,7 +768,7 @@ function createElement(
         };
         if (enableOwnerStacks) {
           // $FlowFixMe[cannot-write]
-          erroredComponent.stack = element._debugStack;
+          erroredComponent.debugStack = element._debugStack;
           // $FlowFixMe[cannot-write]
           erroredComponent.debugTask = element._debugTask;
         }
@@ -937,7 +931,7 @@ function waitForReference<T>(
         };
         if (enableOwnerStacks) {
           // $FlowFixMe[cannot-write]
-          erroredComponent.stack = element._debugStack;
+          erroredComponent.debugStack = element._debugStack;
           // $FlowFixMe[cannot-write]
           erroredComponent.debugTask = element._debugTask;
         }
@@ -2067,40 +2061,52 @@ function initializeFakeTask(
   return componentTask;
 }
 
-const createFakeCallStack = {
-  'react-stack-bottom-frame': function (fakeCallStack: () => Error): Error {
-    return fakeCallStack();
+const createFakeJSXCallStack = {
+  'react-stack-bottom-frame': function (
+    response: Response,
+    stack: string,
+  ): Error {
+    const callStackForError = buildFakeCallStack(
+      response,
+      stack,
+      fakeJSXCallSite,
+    );
+    return callStackForError();
   },
 };
 
-const createFakeCallStackInDEV: (fakeCallStack: () => Error) => Error = __DEV__
+const createFakeJSXCallStackInDEV: (
+  response: Response,
+  stack: string,
+) => Error = __DEV__
   ? // We use this technique to trick minifiers to preserve the function name.
-    (createFakeCallStack['react-stack-bottom-frame'].bind(
-      createFakeCallStack,
+    (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 {
-  if ((debugInfo: any)._stackInitialized) {
+  const cachedEntry = debugInfo.debugStack;
+  if (cachedEntry !== undefined) {
     return;
   }
   if (typeof debugInfo.stack === 'string') {
-    const callStackForError = buildFakeCallStack(
+    // $FlowFixMe[cannot-write]
+    // $FlowFixMe[prop-missing]
+    debugInfo.debugStack = createFakeJSXCallStackInDEV(
       response,
       debugInfo.stack,
-      // $FlowFixMe[incompatible-use]
-      Error.bind(null, 'react-stack-top-frame'),
     );
-    // TODO: Ideally we should leave this as an Error instead of eagerly force it.
-    // TODO: This object identity is important because it might be used as an owner
-    // so we cannot clone it. But by changing the format we also cannot reuse the stack
-    // elsewhere to create fake tasks. We have already done that above so should be ok.
-    // $FlowFixMe[cannot-write]
-    debugInfo.stack = createFakeCallStackInDEV(callStackForError).stack;
-    (debugInfo: any)._stackInitialized = true;
   }
   if (debugInfo.owner != null) {
     // Initialize any owners not yet initialized.

packages/react-client/src/__tests__/ReactFlight-test.js

@@ -29,7 +29,7 @@ function normalizeCodeLocInfo(str) {
 
 function normalizeComponentInfo(debugInfo) {
   if (typeof debugInfo.stack === 'string') {
-    const {debugTask, ...copy} = debugInfo;
+    const {debugTask, debugStack, ...copy} = debugInfo;
     copy.stack = normalizeCodeLocInfo(debugInfo.stack);
     if (debugInfo.owner) {
       copy.owner = normalizeComponentInfo(debugInfo.owner);

packages/react-dom/src/__tests__/ReactUpdates-test.js

@@ -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 {
@@ -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 {

packages/react-reconciler/src/ReactFiberComponentStack.js

@@ -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;

packages/react-server/src/ReactFizzComponentStack.js

@@ -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;

packages/react-server/src/ReactFizzServer.js

@@ -859,14 +859,14 @@ 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.debugTask: any);

packages/react-server/src/ReactFlightOwnerStack.js

@@ -0,0 +1,42 @@
+/**
+ * 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);
+}

packages/react-server/src/ReactFlightServer.js

@@ -351,7 +351,7 @@ type Task = {
   thenableState: ThenableState | null,
   environmentName: string, // DEV-only. Used to track if the environment for this task changed.
   debugOwner: null | ReactComponentInfo, // DEV-only
-  debugStack: null | string, // DEV-only
+  debugStack: null | Error, // DEV-only
   debugTask: null | ConsoleTask, // DEV-only
 };
 
@@ -972,7 +972,10 @@ function callWithDebugContextInDEV<A, T>(
   };
   if (enableOwnerStacks) {
     // $FlowFixMe[cannot-write]
-    componentDebugInfo.stack = task.debugStack;
+    componentDebugInfo.stack =
+      task.debugStack === null ? null : filterDebugStack(task.debugStack);
+    // $FlowFixMe[cannot-write]
+    componentDebugInfo.debugStack = task.debugStack;
     // $FlowFixMe[cannot-write]
     componentDebugInfo.debugTask = task.debugTask;
   }
@@ -1031,7 +1034,10 @@ function renderFunctionComponent<Props>(
       }: ReactComponentInfo);
       if (enableOwnerStacks) {
         // $FlowFixMe[cannot-write]
-        componentDebugInfo.stack = task.debugStack;
+        componentDebugInfo.stack =
+          task.debugStack === null ? null : filterDebugStack(task.debugStack);
+        // $FlowFixMe[cannot-write]
+        componentDebugInfo.debugStack = task.debugStack;
         // $FlowFixMe[cannot-write]
         componentDebugInfo.debugTask = task.debugTask;
       }
@@ -1423,7 +1429,7 @@ function renderClientElement(
           key,
           props,
           task.debugOwner,
-          task.debugStack,
+          task.debugStack === null ? null : filterDebugStack(task.debugStack),
           validated,
         ]
       : [REACT_ELEMENT_TYPE, type, key, props, task.debugOwner]
@@ -1602,7 +1608,7 @@ function createTask(
   implicitSlot: boolean,
   abortSet: Set<Task>,
   debugOwner: null | ReactComponentInfo, // DEV-only
-  debugStack: null | string, // DEV-only
+  debugStack: null | Error, // DEV-only
   debugTask: null | ConsoleTask, // DEV-only
 ): Task {
   request.pendingChunks++;
@@ -2209,10 +2215,7 @@ function renderModelDestructive(
         if (__DEV__) {
           task.debugOwner = element._owner;
           if (enableOwnerStacks) {
-            task.debugStack =
-              !element._debugStack || typeof element._debugStack === 'string'
-                ? element._debugStack
-                : filterDebugStack(element._debugStack);
+            task.debugStack = element._debugStack;
             task.debugTask = element._debugTask;
           }
           // TODO: Pop this. Since we currently don't have a point where we can pop the stack
@@ -2511,10 +2514,11 @@ function renderModelDestructive(
     if (__DEV__) {
       if (
         // TODO: We don't currently have a brand check on ReactComponentInfo. Reconsider.
-        typeof value.debugTask === 'object' &&
-        value.debugTask !== null &&
-        // $FlowFixMe[method-unbinding]
-        typeof value.debugTask.run === 'function' &&
+        ((typeof value.debugTask === 'object' &&
+          value.debugTask !== null &&
+          // $FlowFixMe[method-unbinding]
+          typeof value.debugTask.run === 'function') ||
+          value.debugStack instanceof Error) &&
         typeof value.name === 'string' &&
         typeof value.env === 'string' &&
         value.owner !== undefined &&
@@ -2524,7 +2528,10 @@ function renderModelDestructive(
       ) {
         // This looks like a ReactComponentInfo. We can't serialize the ConsoleTask object so we
         // need to omit it before serializing.
-        const componentDebugInfo: Omit<ReactComponentInfo, 'debugTask'> = {
+        const componentDebugInfo: Omit<
+          ReactComponentInfo,
+          'debugTask' | 'debugStack',
+        > = {
           name: value.name,
           env: value.env,
           owner: (value: any).owner,

packages/react-server/src/flight/ReactFlightComponentStack.js

@@ -13,6 +13,8 @@ import {describeBuiltInComponentFrame} from 'shared/ReactComponentStackFrame';
 
 import {enableOwnerStacks} from 'shared/ReactFeatureFlags';
 
+import {formatOwnerStack} from '../ReactFlightOwnerStack';
+
 export function getOwnerStackByComponentInfoInDev(
   componentInfo: ReactComponentInfo,
 ): string {
@@ -34,12 +36,13 @@ export function getOwnerStackByComponentInfoInDev(
     let owner: void | null | ReactComponentInfo = componentInfo;
 
     while (owner) {
-      if (typeof owner.stack === 'string') {
+      const ownerStack: ?Error = owner.debugStack;
+      if (ownerStack != null) {
         // Server Component
-        const ownerStack: string = owner.stack;
         owner = owner.owner;
-        if (owner && ownerStack !== '') {
-          info += '\n' + ownerStack;
+        if (owner) {
+          // TODO: Should we stash this somewhere for caching purposes?
+          info += '\n' + formatOwnerStack(ownerStack);
         }
       } else {
         break;

packages/shared/ReactTypes.js

@@ -184,6 +184,7 @@ export type ReactComponentInfo = {
   +owner?: null | ReactComponentInfo,
   +stack?: null | string,
   // Stashed Data for the Specific Execution Environment. Not part of the transport protocol
+  +debugStack?: null | Error,
   +debugTask?: null | ConsoleTask,
 };