[Flight] Parse Stack on the Server and Transfer Structured Stack (#30410

) Stacked on #30401. Previously we were transferring the original V8 stack trace string to the client and then parsing it there. However, really the server is the one that knows what format it is and it should be able to vary by server environment. We also don't use the raw string anymore (at least not in enableOwnerStacks). We always create the native Error stacks. The string also made it unclear which environment it is and it was tempting to just use it as is. Instead I parse it on the server and make it a structured stack in the transfer format. It also makes it clear that it needs to be formatted in the current environment before presented.
facebook · Jul 22, 2024 · 0676385 · 0676385
1 parent b15c198
commit 0676385
Show file tree

Hide file tree

Showing 22 changed files with 267 additions and 123 deletions.
diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js
@@ -12,6 +12,7 @@ import type {
   ReactDebugInfo,
   ReactComponentInfo,
   ReactAsyncInfo,
+  ReactStackTrace,
 } from 'shared/ReactTypes';
 import type {LazyComponent} from 'react/src/ReactLazy';
 
@@ -624,7 +625,7 @@ function createElement(
   key: mixed,
   props: mixed,
   owner: null | ReactComponentInfo, // DEV-only
-  stack: null | string, // DEV-only
+  stack: null | ReactStackTrace, // DEV-only
   validated: number, // DEV-only
 ):
   | React$Element<any>
@@ -1738,6 +1739,27 @@ function stopStream(
   controller.close(row === '' ? '"$undefined"' : row);
 }
 
+function formatV8Stack(
+  errorName: string,
+  errorMessage: string,
+  stack: null | ReactStackTrace,
+): string {
+  let v8StyleStack = errorName + ': ' + errorMessage;
+  if (stack) {
+    for (let i = 0; i < stack.length; i++) {
+      const frame = stack[i];
+      const [name, filename, line, col] = frame;
+      if (!name) {
+        v8StyleStack += '\n    at ' + filename + ':' + line + ':' + col;
+      } else {
+        v8StyleStack +=
+          '\n    at ' + name + ' (' + filename + ':' + line + ':' + col + ')';
+      }
+    }
+  }
+  return v8StyleStack;
+}
+
 type ErrorWithDigest = Error & {digest?: string};
 function resolveErrorProd(
   response: Response,
@@ -1773,7 +1795,7 @@ function resolveErrorDev(
   id: number,
   digest: string,
   message: string,
-  stack: string,
+  stack: ReactStackTrace,
   env: string,
 ): void {
   if (!__DEV__) {
@@ -1793,7 +1815,8 @@ function resolveErrorDev(
       message ||
         'An error occurred in the Server Components render but no message was provided',
     );
-    error.stack = stack;
+    // For backwards compat we use the V8 formatting when the flag is off.
+    error.stack = formatV8Stack(error.name, error.message, stack);
   } else {
     const callStack = buildFakeCallStack(
       response,
@@ -1853,7 +1876,7 @@ function resolvePostponeDev(
   response: Response,
   id: number,
   reason: string,
-  stack: string,
+  stack: ReactStackTrace,
 ): void {
   if (!__DEV__) {
     // These errors should never make it into a build so we don't need to encode them in codes.json
@@ -1862,11 +1885,34 @@ function resolvePostponeDev(
       'resolvePostponeDev should never be called in production mode. Use resolvePostponeProd instead. This is a bug in React.',
     );
   }
-  // eslint-disable-next-line react-internal/prod-error-codes
-  const error = new Error(reason || '');
-  const postponeInstance: Postpone = (error: any);
-  postponeInstance.$$typeof = REACT_POSTPONE_TYPE;
-  postponeInstance.stack = stack;
+  let postponeInstance: Postpone;
+  if (!enableOwnerStacks) {
+    // Executing Error within a native stack isn't really limited to owner stacks
+    // but we gate it behind the same flag for now while iterating.
+    // eslint-disable-next-line react-internal/prod-error-codes
+    postponeInstance = (Error(reason || ''): any);
+    postponeInstance.$$typeof = REACT_POSTPONE_TYPE;
+    // For backwards compat we use the V8 formatting when the flag is off.
+    postponeInstance.stack = formatV8Stack(
+      postponeInstance.name,
+      postponeInstance.message,
+      stack,
+    );
+  } else {
+    const callStack = buildFakeCallStack(
+      response,
+      stack,
+      // $FlowFixMe[incompatible-use]
+      Error.bind(null, reason || ''),
+    );
+    const rootTask = response._debugRootTask;
+    if (rootTask != null) {
+      postponeInstance = rootTask.run(callStack);
+    } else {
+      postponeInstance = callStack();
+    }
+    postponeInstance.$$typeof = REACT_POSTPONE_TYPE;
+  }
   const chunks = response._chunks;
   const chunk = chunks.get(id);
   if (!chunk) {
@@ -1973,40 +2019,25 @@ function createFakeFunction<T>(
   return fn;
 }
 
-// This matches either of these V8 formats.
-//     at name (filename:0:0)
-//     at filename:0:0
-//     at async filename:0:0
-const frameRegExp =
-  /^ {3} at (?:(.+) \(([^\)]+):(\d+):(\d+)\)|(?:async )?([^\)]+):(\d+):(\d+))$/;
-
 function buildFakeCallStack<T>(
   response: Response,
-  stack: string,
+  stack: ReactStackTrace,
   innerCall: () => T,
 ): () => T {
-  const frames = stack.split('\n');
   let callStack = innerCall;
-  for (let i = 0; i < frames.length; i++) {
-    const frame = frames[i];
-    let fn = fakeFunctionCache.get(frame);
+  for (let i = 0; i < stack.length; i++) {
+    const frame = stack[i];
+    const frameKey = frame.join('-');
+    let fn = fakeFunctionCache.get(frameKey);
     if (fn === undefined) {
-      const parsed = frameRegExp.exec(frame);
-      if (!parsed) {
-        // We assume the server returns a V8 compatible stack trace.
-        continue;
-      }
-      const name = parsed[1] || '';
-      const filename = parsed[2] || parsed[5] || '';
-      const line = +(parsed[3] || parsed[6]);
-      const col = +(parsed[4] || parsed[7]);
+      const [name, filename, line, col] = frame;
       const sourceMap = response._debugFindSourceMapURL
         ? response._debugFindSourceMapURL(filename)
         : null;
       fn = createFakeFunction(name, filename, sourceMap, line, col);
       // TODO: This cache should technically live on the response since the _debugFindSourceMapURL
       // function is an input and can vary by response.
-      fakeFunctionCache.set(frame, fn);
+      fakeFunctionCache.set(frameKey, fn);
     }
     callStack = fn.bind(null, callStack);
   }
@@ -2026,7 +2057,7 @@ function initializeFakeTask(
     return cachedEntry;
   }
 
-  if (typeof debugInfo.stack !== 'string') {
+  if (debugInfo.stack == null) {
     // If this is an error, we should've really already initialized the task.
     // If it's null, we can't initialize a task.
     return null;
@@ -2064,7 +2095,7 @@ function initializeFakeTask(
 const createFakeJSXCallStack = {
   'react-stack-bottom-frame': function (
     response: Response,
-    stack: string,
+    stack: ReactStackTrace,
   ): Error {
     const callStackForError = buildFakeCallStack(
       response,
@@ -2077,7 +2108,7 @@ const createFakeJSXCallStack = {
 
 const createFakeJSXCallStackInDEV: (
   response: Response,
-  stack: string,
+  stack: ReactStackTrace,
 ) => Error = __DEV__
   ? // We use this technique to trick minifiers to preserve the function name.
     (createFakeJSXCallStack['react-stack-bottom-frame'].bind(
@@ -2100,7 +2131,7 @@ function initializeFakeStack(
   if (cachedEntry !== undefined) {
     return;
   }
-  if (typeof debugInfo.stack === 'string') {
+  if (debugInfo.stack != null) {
     // $FlowFixMe[cannot-write]
     // $FlowFixMe[prop-missing]
     debugInfo.debugStack = createFakeJSXCallStackInDEV(
@@ -2154,8 +2185,13 @@ function resolveConsoleEntry(
     return;
   }
 
-  const payload: [string, string, null | ReactComponentInfo, string, mixed] =
-    parseModel(response, value);
+  const payload: [
+    string,
+    ReactStackTrace,
+    null | ReactComponentInfo,
+    string,
+    mixed,
+  ] = parseModel(response, value);
   const methodName = payload[0];
   const stackTrace = payload[1];
   const owner = payload[2];

diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -27,10 +27,24 @@ function normalizeCodeLocInfo(str) {
   );
 }
 
+function formatV8Stack(stack) {
+  let v8StyleStack = '';
+  if (stack) {
+    for (let i = 0; i < stack.length; i++) {
+      const [name] = stack[i];
+      if (v8StyleStack !== '') {
+        v8StyleStack += '\n';
+      }
+      v8StyleStack += '    in ' + name + ' (at **)';
+    }
+  }
+  return v8StyleStack;
+}
+
 function normalizeComponentInfo(debugInfo) {
-  if (typeof debugInfo.stack === 'string') {
+  if (Array.isArray(debugInfo.stack)) {
     const {debugTask, debugStack, ...copy} = debugInfo;
-    copy.stack = normalizeCodeLocInfo(debugInfo.stack);
+    copy.stack = formatV8Stack(debugInfo.stack);
     if (debugInfo.owner) {
       copy.owner = normalizeComponentInfo(debugInfo.owner);
     }

diff --git a/packages/react-reconciler/src/ReactFiberOwnerStack.js b/packages/react-reconciler/src/ReactFiberOwnerStack.js
@@ -8,7 +8,7 @@
  */
 
 // TODO: Make this configurable on the root.
-const externalRegExp = /\/node\_modules\/|\(\<anonymous\>\)/;
+const externalRegExp = /\/node\_modules\/|\(\<anonymous\>/;
 
 function isNotExternal(stackFrame: string): boolean {
   return !externalRegExp.test(stackFrame);

diff --git a/packages/react-server/src/ReactFizzComponentStack.js b/packages/react-server/src/ReactFizzComponentStack.js
@@ -165,17 +165,17 @@ export function getOwnerStackByComponentStackNodeInDev(
         // TODO: Should we stash this somewhere for caching purposes?
         ownerStack = formatOwnerStack(owner.debugStack);
         owner = owner.owner;
-      } else if (owner.stack != null) {
+      } else {
         // Client Component
         const node: ComponentStackNode = (owner: any);
-        if (typeof owner.stack !== 'string') {
-          ownerStack = node.stack = formatOwnerStack(owner.stack);
-        } else {
-          ownerStack = owner.stack;
+        if (node.stack != null) {
+          if (typeof node.stack !== 'string') {
+            ownerStack = node.stack = formatOwnerStack(node.stack);
+          } else {
+            ownerStack = node.stack;
+          }
         }
         owner = owner.owner;
-      } else {
-        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

diff --git a/packages/react-server/src/ReactFizzOwnerStack.js b/packages/react-server/src/ReactFizzOwnerStack.js
@@ -8,7 +8,7 @@
  */
 
 // TODO: Make this configurable on the root.
-const externalRegExp = /\/node\_modules\/|\(\<anonymous\>\)/;
+const externalRegExp = /\/node\_modules\/|\(\<anonymous\>/;
 
 function isNotExternal(stackFrame: string): boolean {
   return !externalRegExp.test(stackFrame);

diff --git a/packages/react-server/src/ReactFlightOwnerStack.js b/packages/react-server/src/ReactFlightOwnerStack.js
@@ -8,7 +8,7 @@
  */
 
 // TODO: Make this configurable on the Request.
-const externalRegExp = /\/node\_modules\/| \(node\:| node\:|\(\<anonymous\>\)/;
+const externalRegExp = /\/node\_modules\/| \(node\:| node\:|\(\<anonymous\>/;
 
 function isNotExternal(stackFrame: string): boolean {
   return !externalRegExp.test(stackFrame);