Throw special error if rejected Promises are incorrectly instrumented (#36328)

When a rejected `Promise` is instrumented in userspace while setting the
rejection reason in the wrong field (e.g. `error` instead of `reason`),
React will throw undefined (because `usable.reason` doesn't exist). This
makes it incredibly hard to find the actual rejection reason.

React is now throwing a generic error if we couldn't find the rejection
reason. That will produce a callstack pointing into the problematic
Promise from where you can hopefully extract the real rejection reason
(alongside fixing the bad instrumentation).

We're doing this in prod since this is unlikely to surface in dev. We're
only doing runtime type-checking for the rejected case. That should be
hit rarely and therefore hopefully have negligible runtime impact.

packages/react-reconciler/src/ReactFiberThenable.js

@@ -192,12 +192,30 @@ export function trackUsedThenable<T>(
   // a listener that will update its status and result when it resolves.
   switch (thenable.status) {
     case 'fulfilled': {
+      // This could be a bad instrumentation that doesn't set .value.
+      // We're not type-checking since this is a hot path where you can
+      // track down easily when something becomes `undefined` unexpectedly.
       const fulfilledValue: T = thenable.value;
       return fulfilledValue;
     }
     case 'rejected': {
       const rejectedError = thenable.reason;
       checkIfUseWrappedInAsyncCatch(rejectedError);
+
+      // Rejected Promises are rarer so we're doing an extra type-check in
+      // case of a bad instrumentation that doesn't set .reason
+      // If we end up throwing `undefined` it becomes hard to track down
+      // where that throw originated because no callstack would exist.
+      // React would still have a Component stack but that could only be used
+      // as an approximation.
+      if (rejectedError === undefined && !('reason' in thenable)) {
+        throw new Error(
+          'A rejected Promise was passed to React without a `reason` property. ' +
+            'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' +
+            "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.",
+        );
+      }
+
       throw rejectedError;
     }
     default: {

packages/react-reconciler/src/__tests__/ReactUse-test.js

@@ -81,6 +81,30 @@ describe('ReactUse', () => {
     });
   }
 
+  function normalizeCodeLocInfo(str) {
+    return (
+      str &&
+      str.replace(
+        /^ +(?:at|in) ([\S]+)([^\n]*)(\n?)/gm,
+        function (m, name, location, eol) {
+          if (location.indexOf(__filename) === -1) {
+            // ignore frames from library code
+            return '';
+          }
+          return (
+            '    in ' +
+            name +
+            (/:\d+:\d+/.test(m)
+              ? ' ' +
+                location.replace(__dirname, '~').replace(/:\d+:\d+/, ':*:*')
+              : '') +
+            eol
+          );
+        },
+      )
+    );
+  }
+
   function Text({text}) {
     Scheduler.log(text);
     return text;
@@ -2189,4 +2213,108 @@ describe('ReactUse', () => {
       expect(root).toMatchRenderedOutput('Updated');
     },
   );
+
+  it('throws a descriptive error when a rejected promise without a reason property is passed to use()', async () => {
+    const badThenable = Promise.resolve();
+    // Simulate bad instrumentation: status is 'rejected' but the reason is not
+    // in the `reason` property as intended.
+    badThenable.status = 'rejected';
+    badThenable.error = new Error('Something went wrong');
+
+    function Child() {
+      return use(badThenable);
+    }
+
+    function App({
+      // Intentionally destrucutring a prop here so that our production error
+      // stack trick is triggered at the beginning of the function
+      prop,
+    }) {
+      return <Child />;
+    }
+
+    const uncaughtErrors = [];
+    const root = ReactNoop.createRoot({
+      onUncaughtError(error, errorInfo) {
+        uncaughtErrors.push({
+          callStack: normalizeCodeLocInfo(error.stack),
+          ownerStack: React.captureOwnerStack
+            ? normalizeCodeLocInfo(React.captureOwnerStack())
+            : null,
+          componentStack: normalizeCodeLocInfo(errorInfo.componentStack),
+        });
+      },
+    });
+
+    await act(() => {
+      root.render(<App />);
+    });
+
+    expect(uncaughtErrors).toEqual([
+      {
+        callStack:
+          'Error: A rejected Promise was passed to React without a `reason` property. ' +
+          'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' +
+          "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`." +
+          '\n    in Child  (~/ReactUse-test.js:*:*)' +
+          '\n    in Object.<anonymous>  (~/ReactUse-test.js:*:*)',
+        componentStack:
+          '\n    in Child  (~/ReactUse-test.js:*:*)' +
+          '\n    in App  (~/ReactUse-test.js:*:*)',
+        ownerStack: __DEV__ ? '\n    in App  (~/ReactUse-test.js:*:*)' : null,
+      },
+    ]);
+  });
+
+  it('throws a descriptive error when a rejected promise without a reason property is used as a child', async () => {
+    const badThenable = Promise.resolve();
+    // Simulate bad instrumentation: status is 'rejected' but the reason is not
+    // in the `reason` property as intended.
+    badThenable.status = 'rejected';
+    badThenable.error = new Error('Something went wrong');
+
+    function Child() {
+      return <div>{badThenable}</div>;
+    }
+
+    function App({
+      // Intentionally destrucutring a prop here so that our production error
+      // stack trick is triggered at the beginning of the function
+      prop,
+    }) {
+      return <Child />;
+    }
+
+    const uncaughtErrors = [];
+    const root = ReactNoop.createRoot({
+      onUncaughtError(error, errorInfo) {
+        uncaughtErrors.push({
+          callStack: normalizeCodeLocInfo(error.stack),
+          ownerStack: React.captureOwnerStack
+            ? normalizeCodeLocInfo(React.captureOwnerStack())
+            : null,
+          componentStack: normalizeCodeLocInfo(errorInfo.componentStack),
+        });
+      },
+    });
+
+    await act(() => {
+      root.render(<App />);
+    });
+
+    expect(uncaughtErrors).toEqual([
+      {
+        callStack:
+          'Error: A rejected Promise was passed to React without a `reason` property. ' +
+          'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' +
+          "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`." +
+          '\n    in Object.<anonymous>  (~/ReactUse-test.js:*:*)',
+        componentStack: '\n    in App  (~/ReactUse-test.js:*:*)',
+        ownerStack: __DEV__
+          ? // TODO: Should start in Child since that's the owner.
+            '\n    in App  (~/ReactUse-test.js:*:*)'
+          : null,
+      },
+    ]);
+  });
 });

packages/react-server/src/ReactFizzThenable.js

@@ -69,11 +69,29 @@ export function trackUsedThenable<T>(
   // a listener that will update its status and result when it resolves.
   switch (thenable.status) {
     case 'fulfilled': {
+      // This could be a bad instrumentation that doesn't set .value.
+      // We're not type-checking since this is a hot path where you can
+      // track down easily when something becomes `undefined` unexpectedly.
       const fulfilledValue: T = thenable.value;
       return fulfilledValue;
     }
     case 'rejected': {
       const rejectedError = thenable.reason;
+
+      // Rejected Promises are rarer so we're doing an extra type-check in
+      // case of a bad instrumentation that doesn't set .reason
+      // If we end up throwing `undefined` it becomes hard to track down
+      // where that throw originated because no callstack would exist.
+      // React would still have a Component stack but that could only be used
+      // as an approximation.
+      if (rejectedError === undefined && !('reason' in thenable)) {
+        throw new Error(
+          'A rejected Promise was passed to React without a `reason` property. ' +
+            'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' +
+            "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.",
+        );
+      }
+
       throw rejectedError;
     }
     default: {

packages/react-server/src/ReactFlightThenable.js

@@ -77,11 +77,29 @@ export function trackUsedThenable<T>(
   // a listener that will update its status and result when it resolves.
   switch (thenable.status) {
     case 'fulfilled': {
+      // This could be a bad instrumentation that doesn't set .value.
+      // We're not type-checking since this is a hot path where you can
+      // track down easily when something becomes `undefined` unexpectedly.
       const fulfilledValue: T = thenable.value;
       return fulfilledValue;
     }
     case 'rejected': {
       const rejectedError = thenable.reason;
+
+      // Rejected Promises are rarer so we're doing an extra type-check in
+      // case of a bad instrumentation that doesn't set .reason
+      // If we end up throwing `undefined` it becomes hard to track down
+      // where that throw originated because no callstack would exist.
+      // React would still have a Component stack but that could only be used
+      // as an approximation.
+      if (rejectedError === undefined && !('reason' in thenable)) {
+        throw new Error(
+          'A rejected Promise was passed to React without a `reason` property. ' +
+            'React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. ' +
+            "Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`.",
+        );
+      }
+
       throw rejectedError;
     }
     default: {

scripts/error-codes/codes.json

@@ -584,5 +584,6 @@
   "596": "Could not find the module \"%s\" in the React Client Manifest. This is probably a bug in the React Server Components bundler.",
   "597": "The module \"%s\" is marked as an async ESM module but was loaded as a CJS proxy. This is probably a bug in the React Server Components bundler.",
   "598": "Maximum update depth exceeded. This could be an infinite loop. This can happen when a component repeatedly calls setState during render phase or inside useLayoutEffect, causing infinite render loop. React limits the number of nested updates to prevent infinite loops.",
-  "599": "Expected an initialized chunk but got an initialized stream chunk instead. This payload may have been submitted by an older version of React."
-}
+  "599": "Expected an initialized chunk but got an initialized stream chunk instead. This payload may have been submitted by an older version of React.",
+  "600": "A rejected Promise was passed to React without a `reason` property. React threw a generic error from where the Promise was used to assist in identifying the problematic Promise. Make sure that instrumented Promises correctly set the `reason` property when setting `status` to `'rejected'`."
+}