Skip to content

Commit

Permalink
Don't replay consoles written inside onError/onPostpone.
Browse files Browse the repository at this point in the history
These aren't conceptually part of the request's render so we exit the
request context for those.
  • Loading branch information
sebmarkbage committed Feb 20, 2024
1 parent c054691 commit 0361411
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 4 deletions.
41 changes: 41 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1978,4 +1978,45 @@ describe('ReactFlight', () => {
</div>,
);
});

// @gate enableServerComponentLogs
it('replays logs, but not onError logs', async () => {
function foo() {
return 'hello';
}
function ServerComponent() {
console.log('hi', {prop: 123, fn: foo});
throw new Error('err');
}

let transport;
expect(() => {
// 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');
transport = ReactNoopFlightServer.render({root: <ServerComponent />});
}).toErrorDev('err');

const log = console.log;
try {
console.log = jest.fn();
// 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(console.log).toHaveBeenCalledTimes(1);
expect(console.log.mock.calls[0][0]).toBe('hi');
expect(console.log.mock.calls[0][1].prop).toBe(123);
const loggedFn = console.log.mock.calls[0][1].fn;
expect(typeof loggedFn).toBe('function');
expect(loggedFn).not.toBe(foo);
expect(loggedFn.toString()).toBe(foo.toString());
} finally {
console.log = log;
}
});
});
31 changes: 27 additions & 4 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1689,13 +1689,36 @@ function renderModelDestructive(
}

function logPostpone(request: Request, reason: string): void {
const onPostpone = request.onPostpone;
onPostpone(reason);
const prevRequest = currentRequest;
currentRequest = null;
try {
const onPostpone = request.onPostpone;
if (supportsRequestStorage) {
// Exit the request context while running callbacks.
requestStorage.run(undefined, onPostpone, reason);
} else {
onPostpone(reason);
}
} finally {
currentRequest = prevRequest;
}
}

function logRecoverableError(request: Request, error: mixed): string {
const onError = request.onError;
const errorDigest = onError(error);
const prevRequest = currentRequest;
currentRequest = null;
let errorDigest;
try {
const onError = request.onError;
if (supportsRequestStorage && false) {
// Exit the request context while running callbacks.
errorDigest = requestStorage.run(undefined, onError, error);
} else {
errorDigest = onError(error);
}
} finally {
currentRequest = prevRequest;
}
if (errorDigest != null && typeof errorDigest !== 'string') {
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
Expand Down

0 comments on commit 0361411

Please sign in to comment.