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 d05a7b9
Show file tree
Hide file tree
Showing 21 changed files with 91 additions and 30 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 && __DEV__
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) {
// 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
2 changes: 1 addition & 1 deletion packages/react-server/src/ReactServerStreamConfigFB.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export opaque type BinaryChunk = string;
export function flushBuffered(destination: Destination) {}

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export function beginWriting(destination: Destination) {}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-server/src/forks/ReactFizzConfig.custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export type {TransitionStatus};
export const isPrimaryRenderer = false;

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export const resetResumableState = $$$config.resetResumableState;
export const completeResumableState = $$$config.completeResumableState;
Expand Down
5 changes: 2 additions & 3 deletions packages/react-server/src/forks/ReactFizzConfig.dom-edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ export * from 'react-dom-bindings/src/server/ReactFizzConfigDOM';

// For now, we get this from the global scope, but this will likely move to a module.
export const supportsRequestStorage = typeof AsyncLocalStorage === 'function';
export const requestStorage: AsyncLocalStorage<Request> = supportsRequestStorage
? new AsyncLocalStorage()
: (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> =
supportsRequestStorage ? new AsyncLocalStorage() : (null: any);
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ import type {Request} from 'react-server/src/ReactFizzServer';
export * from 'react-dom-bindings/src/server/ReactFizzConfigDOMLegacy';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ import type {Request} from 'react-server/src/ReactFizzServer';
export * from 'react-dom-bindings/src/server/ReactFizzConfigDOM';

export const supportsRequestStorage = true;
export const requestStorage: AsyncLocalStorage<Request> =
export const requestStorage: AsyncLocalStorage<Request | void> =
new AsyncLocalStorage();
2 changes: 1 addition & 1 deletion packages/react-server/src/forks/ReactFizzConfig.dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ import type {Request} from 'react-server/src/ReactFizzServer';
export * from 'react-dom-bindings/src/server/ReactFizzConfigDOM';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const isPrimaryRenderer = false;
export const prepareHostDispatcher = () => {};

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export function createHints(): any {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export * from 'react-server-dom-esm/src/ReactFlightServerConfigESMBundler';
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = true;
export const requestStorage: AsyncLocalStorage<Request> =
export const requestStorage: AsyncLocalStorage<Request | void> =
new AsyncLocalStorage();

export * from '../ReactFlightServerConfigDebugNoop';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export * from 'react-server-dom-turbopack/src/ReactFlightServerConfigTurbopackBu
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export * from '../ReactFlightServerConfigDebugNoop';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export * from 'react-server-dom-webpack/src/ReactFlightServerConfigWebpackBundle
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export * from '../ReactFlightServerConfigDebugNoop';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export * from '../ReactFlightServerConfigBundlerCustom';
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export * from '../ReactFlightServerConfigDebugNoop';
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

// For now, we get this from the global scope, but this will likely move to a module.
export const supportsRequestStorage = typeof AsyncLocalStorage === 'function';
export const requestStorage: AsyncLocalStorage<Request> = supportsRequestStorage
? new AsyncLocalStorage()
: (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> =
supportsRequestStorage ? new AsyncLocalStorage() : (null: any);

// We use the Node version but get access to async_hooks from a global.
import type {HookCallbacks, AsyncHook} from 'async_hooks';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

// For now, we get this from the global scope, but this will likely move to a module.
export const supportsRequestStorage = typeof AsyncLocalStorage === 'function';
export const requestStorage: AsyncLocalStorage<Request> = supportsRequestStorage
? new AsyncLocalStorage()
: (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> =
supportsRequestStorage ? new AsyncLocalStorage() : (null: any);

// We use the Node version but get access to async_hooks from a global.
import type {HookCallbacks, AsyncHook} from 'async_hooks';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export * from 'react-server-dom-fb/src/ReactFlightServerConfigFBBundler';
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export * from '../ReactFlightServerConfigDebugNoop';
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export * from '../ReactFlightServerConfigBundlerCustom';
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = false;
export const requestStorage: AsyncLocalStorage<Request> = (null: any);
export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);

export * from '../ReactFlightServerConfigDebugNoop';
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export * from 'react-server-dom-esm/src/ReactFlightServerConfigESMBundler';
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = true;
export const requestStorage: AsyncLocalStorage<Request> =
export const requestStorage: AsyncLocalStorage<Request | void> =
new AsyncLocalStorage();

export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export * from 'react-server-dom-turbopack/src/ReactFlightServerConfigTurbopackBu
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = true;
export const requestStorage: AsyncLocalStorage<Request> =
export const requestStorage: AsyncLocalStorage<Request | void> =
new AsyncLocalStorage();

export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export * from 'react-server-dom-webpack/src/ReactFlightServerConfigWebpackBundle
export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM';

export const supportsRequestStorage = true;
export const requestStorage: AsyncLocalStorage<Request> =
export const requestStorage: AsyncLocalStorage<Request | void> =
new AsyncLocalStorage();

export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks';
Expand Down
4 changes: 2 additions & 2 deletions scripts/flow/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ declare module 'async_hooks' {
declare class AsyncLocalStorage<T> {
disable(): void;
getStore(): T | void;
run(store: T, callback: (...args: any[]) => void, ...args: any[]): void;
run<R>(store: T, callback: (...args: any[]) => R, ...args: any[]): R;
enterWith(store: T): void;
}
declare interface AsyncResource {}
Expand Down Expand Up @@ -316,7 +316,7 @@ declare module 'async_hooks' {
declare class AsyncLocalStorage<T> {
disable(): void;
getStore(): T | void;
run(store: T, callback: (...args: any[]) => void, ...args: any[]): void;
run<R>(store: T, callback: (...args: any[]) => R, ...args: any[]): R;
enterWith(store: T): void;
}

Expand Down

0 comments on commit d05a7b9

Please sign in to comment.