Skip to content

Commit

Permalink
[Flight] Handle errors during JSON stringify of console values (#31391)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage authored Oct 31, 2024
1 parent ea3ac58 commit b7e2157
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 4 deletions.
59 changes: 59 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 27 additions & 4 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b7e2157

Please sign in to comment.