Repository: facebook/react
Pull request: [Flight] Add some test coverage for some error cases

This adds some tests for some error cases loading modules.

I mistakenly thought that we didn't have any forward references anymore but there's a case where a module resolution synchronously errors. Typically this would only happen due to a bug in the bundler.

This is a weird edge but I'll probably model synchronous references to async module references as forward references anyway so I'll just remove this error for now.

I couldn't actually figure out how to properly test this using public APIs because the scenario only happens if we manage to read the model referencing the error before the error resolves. Which is race condition in the stream of the client. So this particular test didn't fail before. We could probably do something to yield the stream similar to Scheduler.yield but there's no user code involved here to do that from in predictably the right slot.

Changed files:
filename: packages/react-client/src/ReactFlightClient.js
patch: @@ -317,11 +317,6 @@ export function parseModelString(
       } else {
         const id = parseInt(value.substring(1), 16);
         const chunk = getChunk(response, id);
-        if (chunk._status === PENDING) {
-          throw new Error(
-            "We didn't expect to see a forward reference. This is a bug in the React Server.",
-          );
-        }
         return readChunk(chunk);
       }
     }

filename: packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
patch: @@ -19,20 +19,23 @@ global.setImmediate = cb => cb();
 
 let act;
 let clientExports;
+let clientModuleError;
 let webpackMap;
 let Stream;
 let React;
 let ReactDOMClient;
 let ReactServerDOMWriter;
 let ReactServerDOMReader;
 let Suspense;
+let ErrorBoundary;
 
 describe('ReactFlightDOM', () => {
   beforeEach(() => {
     jest.resetModules();
     act = require('jest-react').act;
     const WebpackMock = require('./utils/WebpackMock');
     clientExports = WebpackMock.clientExports;
+    clientModuleError = WebpackMock.clientModuleError;
     webpackMap = WebpackMock.webpackMap;
 
     Stream = require('stream');
@@ -41,6 +44,22 @@ describe('ReactFlightDOM', () => {
     ReactDOMClient = require('react-dom/client');
     ReactServerDOMWriter = require('react-server-dom-webpack/writer.node.server');
     ReactServerDOMReader = require('react-server-dom-webpack');
+
+    ErrorBoundary = class extends React.Component {
+      state = {hasError: false, error: null};
+      static getDerivedStateFromError(error) {
+        return {
+          hasError: true,
+          error,
+        };
+      }
+      render() {
+        if (this.state.hasError) {
+          return this.props.fallback(this.state.error);
+        }
+        return this.props.children;
+      }
+    };
   });
 
   function getTestStream() {
@@ -319,22 +338,6 @@ describe('ReactFlightDOM', () => {
 
     // Client Components
 
-    class ErrorBoundary extends React.Component {
-      state = {hasError: false, error: null};
-      static getDerivedStateFromError(error) {
-        return {
-          hasError: true,
-          error,
-        };
-      }
-      render() {
-        if (this.state.hasError) {
-          return this.props.fallback(this.state.error);
-        }
-        return this.props.children;
-      }
-    }
-
     function MyErrorBoundary({children}) {
       return (
         <ErrorBoundary fallback={e => <p>{e.message}</p>}>
@@ -605,22 +608,6 @@ describe('ReactFlightDOM', () => {
   it('should be able to complete after aborting and throw the reason client-side', async () => {
     const reportedErrors = [];
 
-    class ErrorBoundary extends React.Component {
-      state = {hasError: false, error: null};
-      static getDerivedStateFromError(error) {
-        return {
-          hasError: true,
-          error,
-        };
-      }
-      render() {
-        if (this.state.hasError) {
-          return this.props.fallback(this.state.error);
-        }
-        return this.props.children;
-      }
-    }
-
     const {writable, readable} = getTestStream();
     const {pipe, abort} = ReactServerDOMWriter.renderToPipeableStream(
       <div>
@@ -661,4 +648,159 @@ describe('ReactFlightDOM', () => {
 
     expect(reportedErrors).toEqual(['for reasons']);
   });
+
+  it('should be able to recover from a direct reference erroring client-side', async () => {
+    const reportedErrors = [];
+
+    const ClientComponent = clientExports(function({prop}) {
+      return 'This should never render';
+    });
+
+    const ClientReference = clientModuleError(new Error('module init error'));
+
+    const {writable, readable} = getTestStream();
+    const {pipe} = ReactServerDOMWriter.renderToPipeableStream(
+      <div>
+        <ClientComponent prop={ClientReference} />
+      </div>,
+      webpackMap,
+      {
+        onError(x) {
+          reportedErrors.push(x);
+        },
+      },
+    );
+    pipe(writable);
+    const response = ReactServerDOMReader.createFromReadableStream(readable);
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+
+    function App({res}) {
+      return res.readRoot();
+    }
+
+    await act(async () => {
+      root.render(
+        <ErrorBoundary fallback={e => <p>{e.message}</p>}>
+          <Suspense fallback={<p>(loading)</p>}>
+            <App res={response} />
+          </Suspense>
+        </ErrorBoundary>,
+      );
+    });
+    expect(container.innerHTML).toBe('<p>module init error</p>');
+
+    expect(reportedErrors).toEqual([]);
+  });
+
+  it('should be able to recover from a direct reference erroring client-side async', async () => {
+    const reportedErrors = [];
+
+    const ClientComponent = clientExports(function({prop}) {
+      return 'This should never render';
+    });
+
+    let rejectPromise;
+    const ClientReference = await clientExports(
+      new Promise((resolve, reject) => {
+        rejectPromise = reject;
+      }),
+    );
+
+    const {writable, readable} = getTestStream();
+    const {pipe} = ReactServerDOMWriter.renderToPipeableStream(
+      <div>
+        <ClientComponent prop={ClientReference} />
+      </div>,
+      webpackMap,
+      {
+        onError(x) {
+          reportedErrors.push(x);
+        },
+      },
+    );
+    pipe(writable);
+    const response = ReactServerDOMReader.createFromReadableStream(readable);
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+
+    function App({res}) {
+      return res.readRoot();
+    }
+
+    await act(async () => {
+      root.render(
+        <ErrorBoundary fallback={e => <p>{e.message}</p>}>
+          <Suspense fallback={<p>(loading)</p>}>
+            <App res={response} />
+          </Suspense>
+        </ErrorBoundary>,
+      );
+    });
+
+    expect(container.innerHTML).toBe('<p>(loading)</p>');
+
+    await act(async () => {
+      rejectPromise(new Error('async module init error'));
+    });
+
+    expect(container.innerHTML).toBe('<p>async module init error</p>');
+
+    expect(reportedErrors).toEqual([]);
+  });
+
+  it('should be able to recover from a direct reference erroring server-side', async () => {
+    const reportedErrors = [];
+
+    const ClientComponent = clientExports(function({prop}) {
+      return 'This should never render';
+    });
+
+    // We simulate a bug in the Webpack bundler which causes an error on the server.
+    for (const id in webpackMap) {
+      Object.defineProperty(webpackMap, id, {
+        get: () => {
+          throw new Error('bug in the bundler');
+        },
+      });
+    }
+
+    const {writable, readable} = getTestStream();
+    const {pipe} = ReactServerDOMWriter.renderToPipeableStream(
+      <div>
+        <ClientComponent />
+      </div>,
+      webpackMap,
+      {
+        onError(x) {
+          reportedErrors.push(x);
+        },
+      },
+    );
+    pipe(writable);
+
+    const response = ReactServerDOMReader.createFromReadableStream(readable);
+
+    const container = document.createElement('div');
+    const root = ReactDOMClient.createRoot(container);
+
+    function App({res}) {
+      return res.readRoot();
+    }
+
+    await act(async () => {
+      root.render(
+        <ErrorBoundary fallback={e => <p>{e.message}</p>}>
+          <Suspense fallback={<p>(loading)</p>}>
+            <App res={response} />
+          </Suspense>
+        </ErrorBoundary>,
+      );
+    });
+    expect(container.innerHTML).toBe('<p>bug in the bundler</p>');
+
+    expect(reportedErrors).toEqual([]);
+  });
 });

filename: packages/react-server-dom-webpack/src/__tests__/utils/WebpackMock.js
patch: @@ -12,8 +12,12 @@ const Module = require('module');
 
 let webpackModuleIdx = 0;
 const webpackModules = {};
+const webpackErroredModules = {};
 const webpackMap = {};
 global.__webpack_require__ = function(id) {
+  if (webpackErroredModules[id]) {
+    throw webpackErroredModules[id];
+  }
   return webpackModules[id];
 };
 
@@ -36,6 +40,27 @@ Module._extensions['.client.js'] = previousLoader;
 exports.webpackMap = webpackMap;
 exports.webpackModules = webpackModules;
 
+exports.clientModuleError = function clientModuleError(moduleError) {
+  const idx = '' + webpackModuleIdx++;
+  webpackErroredModules[idx] = moduleError;
+  const path = url.pathToFileURL(idx).href;
+  webpackMap[path] = {
+    '': {
+      id: idx,
+      chunks: [],
+      name: '',
+    },
+    '*': {
+      id: idx,
+      chunks: [],
+      name: '*',
+    },
+  };
+  const mod = {exports: {}};
+  nodeLoader(mod, idx);
+  return mod.exports;
+};
+
 exports.clientExports = function clientExports(moduleExports) {
   const idx = '' + webpackModuleIdx++;
   webpackModules[idx] = moduleExports;
@@ -53,17 +78,20 @@ exports.clientExports = function clientExports(moduleExports) {
     },
   };
   if (typeof moduleExports.then === 'function') {
-    moduleExports.then(asyncModuleExports => {
-      for (const name in asyncModuleExports) {
-        webpackMap[path] = {
-          [name]: {
-            id: idx,
-            chunks: [],
-            name: name,
-          },
-        };
-      }
-    });
+    moduleExports.then(
+      asyncModuleExports => {
+        for (const name in asyncModuleExports) {
+          webpackMap[path] = {
+            [name]: {
+              id: idx,
+              chunks: [],
+              name: name,
+            },
+          };
+        }
+      },
+      () => {},
+    );
   }
   for (const name in moduleExports) {
     webpackMap[path] = {