Repository: facebook/react
Pull request: [Flight] Handle errors during JSON stringify of console values

When we serialize debug info we should never error even though we don't currently support everything being serialized. Since it's non-essential dev information.

We already handle errors in the replacer but not when errors happen in the JSON algorithm itself - such as cyclic errors.

We should ideally support cyclic objects but regardless we should gracefully handle the errors.

Changed files:
filename: packages/react-client/src/__tests__/ReactFlight-test.js
patch: @@ -3146,6 +3146,65 @@ describe('ReactFlight', () => {
     expect(ownerStacks).toEqual(['\n    in App (at **)']);
   });
 
+  // @gate enableServerComponentLogs && __DEV__
+  it('replays logs with cyclic objects', async () => {
+    const cyclic = {cycle: null};
+    cyclic.cycle = cyclic;
+
+    function ServerComponent() {
+      console.log('hi', {cyclic});
+      return null;
+    }
+
+    function App() {
+      return ReactServer.createElement(ServerComponent);
+    }
+
+    // These tests are specifically testing console.log.
+    // Assign to `mockConsoleLog` so we can still inspect it when `console.log`
+    // is overridden by the test modules. The original function will be restored
+    // after this test finishes by `jest.restoreAllMocks()`.
+    const mockConsoleLog = spyOnDevAndProd(console, 'log').mockImplementation(
+      () => {},
+    );
+
+    // Reset the modules so that we get a new overridden console on top of the
+    // one installed by expect. This ensures that we still emit console.error
+    // calls.
+    jest.resetModules();
+    jest.mock('react', () => require('react/react.react-server'));
+    ReactServer = require('react');
+    ReactNoopFlightServer = require('react-noop-renderer/flight-server');
+    const transport = ReactNoopFlightServer.render({
+      root: ReactServer.createElement(App),
+    });
+
+    expect(mockConsoleLog).toHaveBeenCalledTimes(1);
+    expect(mockConsoleLog.mock.calls[0][0]).toBe('hi');
+    expect(mockConsoleLog.mock.calls[0][1].cyclic).toBe(cyclic);
+    mockConsoleLog.mockClear();
+    mockConsoleLog.mockImplementation(() => {});
+
+    // The error should not actually get logged because we're not awaiting the root
+    // so it's not thrown but the server log also shouldn't be replayed.
+    await ReactNoopFlightClient.read(transport);
+
+    expect(mockConsoleLog).toHaveBeenCalledTimes(1);
+    // TODO: Support cyclic objects in console encoding.
+    // expect(mockConsoleLog.mock.calls[0][0]).toBe('hi');
+    // const cyclic2 = mockConsoleLog.mock.calls[0][1].cyclic;
+    // expect(cyclic2).not.toBe(cyclic); // Was serialized and therefore cloned
+    // expect(cyclic2.cycle).toBe(cyclic2);
+    expect(mockConsoleLog.mock.calls[0][0]).toBe(
+      'Unknown Value: React could not send it from the server.',
+    );
+    expect(mockConsoleLog.mock.calls[0][1].message).toBe(
+      'Converting circular structure to JSON\n' +
+        "    --> starting at object with constructor 'Object'\n" +
+        "    --- property 'cycle' closes the circle",
+    );
+  });
+
   it('uses the server component debug info as the element owner in DEV', async () => {
     function Container({children}) {
       return children;

filename: packages/react-server/src/ReactFlightServer.js
patch: @@ -3750,8 +3750,16 @@ function outlineConsoleValue(
     }
   }
 
-  // $FlowFixMe[incompatible-type] stringify can return null
-  const json: string = stringify(model, replacer);
+  let json: string;
+  try {
+    // $FlowFixMe[incompatible-cast] stringify can return null
+    json = (stringify(model, replacer): string);
+  } catch (x) {
+    // $FlowFixMe[incompatible-cast] stringify can return null
+    json = (stringify(
+      'Unknown Value: React could not send it from the server.\n' + x.message,
+    ): string);
+  }
 
   request.pendingChunks++;
   const id = request.nextChunkId++;
@@ -3810,8 +3818,23 @@ function emitConsoleChunk(
   const payload = [methodName, stackTrace, owner, env];
   // $FlowFixMe[method-unbinding]
   payload.push.apply(payload, args);
-  // $FlowFixMe[incompatible-type] stringify can return null
-  const json: string = stringify(payload, replacer);
+  let json: string;
+  try {
+    // $FlowFixMe[incompatible-type] stringify can return null
+    json = stringify(payload, replacer);
+  } catch (x) {
+    json = stringify(
+      [
+        methodName,
+        stackTrace,
+        owner,
+        env,
+        'Unknown Value: React could not send it from the server.',
+        x,
+      ],
+      replacer,
+    );
+  }
   const row = serializeRowHeader('W', id) + json + '\n';
   const processedChunk = stringToChunk(row);
   request.completedRegularChunks.push(processedChunk);