Use the Nearest Parent of an Errored Promise as its Owner (#29814)

sebmarkbage · web-flow · commit 383b2a184562 · 2024-06-11T18:10:24.000-04:00
Stacked on #29807. Conceptually the error's owner/task should ideally be captured when the Error constructor is called but neither `console.createTask` does this, nor do we override `Error` to capture our `owner`. So instead, we use the nearest parent as the owner/task of the error. This is usually the same thing when it's thrown from the same async component but not if you await a promise started from a different component/task. Before this stack the "owner" and "task" of a Lazy that errors was the nearest Fiber but if the thing erroring is a Server Component, we need to get that as the owner from the inner most part of debugInfo. To get the Task for that Server Component, we need to expose it on the ReactComponentInfo object. Unfortunately that makes the object not serializable so we need to special case this to exclude it from serialization. It gets restored again on the client. Before (Shell): <img width="813" alt="Screenshot 2024-06-06 at 5 16 20 PM" src="https://github.com/facebook/react/assets/63648/7da2d4c9-539b-494e-ba63-1abdc58ff13c"> After (App): <img width="811" alt="Screenshot 2024-06-08 at 12 29 23 AM" src="https://github.com/facebook/react/assets/63648/dbf40bd7-c24d-4200-81a6-5018bef55f6d">
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -245,7 +245,7 @@ module.exports = {
       },
     ],
     'no-shadow': ERROR,
-    'no-unused-vars': [ERROR, {args: 'none'}],
+    'no-unused-vars': [ERROR, {args: 'none', ignoreRestSiblings: true}],
     'no-use-before-define': OFF,
     'no-useless-concat': OFF,
     quotes: [ERROR, 'single', {avoidEscape: true, allowTemplateLiterals: true}],
diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js
@@ -162,6 +162,7 @@ type InitializedStreamChunk<
   value: T,
   reason: FlightStreamController,
   _response: Response,
+  _debugInfo?: null | ReactDebugInfo,
   then(resolve: (ReadableStream) => mixed, reject?: (mixed) => mixed): void,
 };
 type ErroredChunk<T> = {
@@ -1710,11 +1711,6 @@ function resolveHint<Code: HintCode>(
 const supportsCreateTask =
   __DEV__ && enableOwnerStacks && !!(console: any).createTask;
 
-const taskCache: null | WeakMap<
-  ReactComponentInfo | ReactAsyncInfo,
-  ConsoleTask,
-> = supportsCreateTask ? new WeakMap() : null;
-
 type FakeFunction<T> = (() => T) => T;
 const fakeFunctionCache: Map<string, FakeFunction<any>> = __DEV__
   ? new Map()
@@ -1834,12 +1830,12 @@ function initializeFakeTask(
   response: Response,
   debugInfo: ReactComponentInfo | ReactAsyncInfo,
 ): null | ConsoleTask {
-  if (taskCache === null || typeof debugInfo.stack !== 'string') {
+  if (!supportsCreateTask || typeof debugInfo.stack !== 'string') {
     return null;
   }
   const componentInfo: ReactComponentInfo = (debugInfo: any); // Refined
   const stack: string = debugInfo.stack;
-  const cachedEntry = taskCache.get((componentInfo: any));
+  const cachedEntry = componentInfo.task;
   if (cachedEntry !== undefined) {
     return cachedEntry;
   }
@@ -1856,16 +1852,20 @@ function initializeFakeTask(
   );
   const callStack = buildFakeCallStack(response, stack, createTaskFn);
 
+  let componentTask;
   if (ownerTask === null) {
     const rootTask = response._debugRootTask;
     if (rootTask != null) {
-      return rootTask.run(callStack);
+      componentTask = rootTask.run(callStack);
     } else {
-      return callStack();
+      componentTask = callStack();
     }
   } else {
-    return ownerTask.run(callStack);
+    componentTask = ownerTask.run(callStack);
   }
+  // $FlowFixMe[cannot-write]: We consider this part of initialization.
+  componentInfo.task = componentTask;
+  return componentTask;
 }
 
 function resolveDebugInfo(
diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -27,14 +27,27 @@ function normalizeCodeLocInfo(str) {
   );
 }
 
+function normalizeComponentInfo(debugInfo) {
+  if (typeof debugInfo.stack === 'string') {
+    const {task, ...copy} = debugInfo;
+    copy.stack = normalizeCodeLocInfo(debugInfo.stack);
+    if (debugInfo.owner) {
+      copy.owner = normalizeComponentInfo(debugInfo.owner);
+    }
+    return copy;
+  } else {
+    return debugInfo;
+  }
+}
+
 function getDebugInfo(obj) {
   const debugInfo = obj._debugInfo;
   if (debugInfo) {
+    const copy = [];
     for (let i = 0; i < debugInfo.length; i++) {
-      if (typeof debugInfo[i].stack === 'string') {
-        debugInfo[i].stack = normalizeCodeLocInfo(debugInfo[i].stack);
-      }
+      copy.push(normalizeComponentInfo(debugInfo[i]));
     }
+    return copy;
   }
   return debugInfo;
 }
diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js
@@ -48,6 +48,7 @@ import {
   enableRefAsProp,
   enableAsyncIterableChildren,
   disableLegacyMode,
+  enableOwnerStacks,
 } from 'shared/ReactFeatureFlags';
 
 import {
@@ -1959,7 +1960,28 @@ function createChildReconciler(
       const throwFiber = createFiberFromThrow(x, returnFiber.mode, lanes);
       throwFiber.return = returnFiber;
       if (__DEV__) {
-        throwFiber._debugInfo = currentDebugInfo;
+        const debugInfo = (throwFiber._debugInfo = currentDebugInfo);
+        // Conceptually the error's owner/task should ideally be captured when the
+        // Error constructor is called but neither console.createTask does this,
+        // nor do we override them to capture our `owner`. So instead, we use the
+        // nearest parent as the owner/task of the error. This is usually the same
+        // thing when it's thrown from the same async component but not if you await
+        // a promise started from a different component/task.
+        throwFiber._debugOwner = returnFiber._debugOwner;
+        if (enableOwnerStacks) {
+          throwFiber._debugTask = returnFiber._debugTask;
+        }
+        if (debugInfo != null) {
+          for (let i = debugInfo.length - 1; i >= 0; i--) {
+            if (typeof debugInfo[i].stack === 'string') {
+              throwFiber._debugOwner = (debugInfo[i]: any);
+              if (enableOwnerStacks) {
+                throwFiber._debugTask = debugInfo[i].task;
+              }
+              break;
+            }
+          }
+        }
       }
       return throwFiber;
     } finally {
diff --git a/packages/react-reconciler/src/getComponentNameFromFiber.js b/packages/react-reconciler/src/getComponentNameFromFiber.js
@@ -44,6 +44,7 @@ import {
   LegacyHiddenComponent,
   CacheComponent,
   TracingMarkerComponent,
+  Throw,
 } from 'react-reconciler/src/ReactWorkTags';
 import getComponentNameFromType from 'shared/getComponentNameFromType';
 import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols';
@@ -160,6 +161,26 @@ export default function getComponentNameFromFiber(fiber: Fiber): string | null {
       if (enableLegacyHidden) {
         return 'LegacyHidden';
       }
+      break;
+    case Throw: {
+      if (__DEV__) {
+        // For an error in child position we use the of the inner most parent component.
+        // Whether a Server Component or the parent Fiber.
+        const debugInfo = fiber._debugInfo;
+        if (debugInfo != null) {
+          for (let i = debugInfo.length - 1; i >= 0; i--) {
+            if (typeof debugInfo[i].name === 'string') {
+              return debugInfo[i].name;
+            }
+          }
+        }
+        if (fiber.return === null) {
+          return null;
+        }
+        return getComponentNameFromFiber(fiber.return);
+      }
+      return null;
+    }
   }
 
   return null;
diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
@@ -352,6 +352,7 @@ export type ReactClientValue =
   // subtype, so the receiver can only accept once of these.
   | React$Element<string>
   | React$Element<ClientReference<any> & any>
+  | ReactComponentInfo
   | string
   | boolean
   | number
@@ -2462,6 +2463,32 @@ function renderModelDestructive(
       );
     }
     if (__DEV__) {
+      if (
+        // TODO: We don't currently have a brand check on ReactComponentInfo. Reconsider.
+        typeof value.task === 'object' &&
+        value.task !== null &&
+        // $FlowFixMe[method-unbinding]
+        typeof value.task.run === 'function' &&
+        typeof value.name === 'string' &&
+        typeof value.env === 'string' &&
+        value.owner !== undefined &&
+        (enableOwnerStacks
+          ? typeof (value: any).stack === 'string'
+          : typeof (value: any).stack === 'undefined')
+      ) {
+        // This looks like a ReactComponentInfo. We can't serialize the ConsoleTask object so we
+        // need to omit it before serializing.
+        const componentDebugInfo = {
+          name: value.name,
+          env: value.env,
+          owner: value.owner,
+        };
+        if (enableOwnerStacks) {
+          (componentDebugInfo: any).stack = (value: any).stack;
+        }
+        return (componentDebugInfo: any);
+      }
+
       if (objectName(value) !== 'Object') {
         console.error(
           'Only plain objects can be passed to Client Components from Server Components. ' +
@@ -3231,6 +3258,12 @@ function forwardDebugInfo(
 ) {
   for (let i = 0; i < debugInfo.length; i++) {
     request.pendingChunks++;
+    if (typeof debugInfo[i].name === 'string') {
+      // We outline this model eagerly so that we can refer to by reference as an owner.
+      // If we had a smarter way to dedupe we might not have to do this if there ends up
+      // being no references to this as an owner.
+      outlineModel(request, debugInfo[i]);
+    }
     emitDebugChunk(request, id, debugInfo[i]);
   }
 }
diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js
@@ -183,6 +183,7 @@ export type ReactComponentInfo = {
   +env?: string,
   +owner?: null | ReactComponentInfo,
   +stack?: null | string,
+  +task?: null | ConsoleTask,
 };
 
 export type ReactAsyncInfo = {
