From a2ef4628b6dd0ca0260f9b30e67778ac9326968f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 20 Feb 2024 12:50:48 -0500 Subject: [PATCH] Don't replay consoles written inside onError/onPostpone. These aren't conceptually part of the request's render so we exit the request context for those. --- .../src/__tests__/ReactFlight-test.js | 45 +++++++++++++++++++ .../react-server/src/ReactFlightServer.js | 31 +++++++++++-- .../src/ReactServerStreamConfigFB.js | 2 +- .../src/forks/ReactFizzConfig.custom.js | 2 +- .../src/forks/ReactFizzConfig.dom-edge.js | 5 +-- .../src/forks/ReactFizzConfig.dom-legacy.js | 2 +- .../src/forks/ReactFizzConfig.dom-node.js | 2 +- .../src/forks/ReactFizzConfig.dom.js | 2 +- .../forks/ReactFlightServerConfig.custom.js | 2 +- ...ReactFlightServerConfig.dom-browser-esm.js | 2 +- ...lightServerConfig.dom-browser-turbopack.js | 2 +- .../ReactFlightServerConfig.dom-browser.js | 2 +- .../forks/ReactFlightServerConfig.dom-bun.js | 2 +- ...ctFlightServerConfig.dom-edge-turbopack.js | 5 +-- .../forks/ReactFlightServerConfig.dom-edge.js | 5 +-- ...tFlightServerConfig.dom-fb-experimental.js | 2 +- .../ReactFlightServerConfig.dom-legacy.js | 2 +- .../ReactFlightServerConfig.dom-node-esm.js | 2 +- ...ctFlightServerConfig.dom-node-turbopack.js | 2 +- .../forks/ReactFlightServerConfig.dom-node.js | 2 +- scripts/flow/environment.js | 4 +- 21 files changed, 95 insertions(+), 30 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 06dee1d54ed22..d66084506aa3e 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -1978,4 +1978,49 @@ describe('ReactFlight', () => { , ); }); + + // @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: }); + }).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); + + if (__DEV__) { + 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()); + } else { + expect(console.log).toHaveBeenCalledTimes(0); + } + } finally { + console.log = log; + } + }); }); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 547165228c1fc..fef18a1b3b0b5 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -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( diff --git a/packages/react-server/src/ReactServerStreamConfigFB.js b/packages/react-server/src/ReactServerStreamConfigFB.js index 5dfdde467f751..c763bde102bf7 100644 --- a/packages/react-server/src/ReactServerStreamConfigFB.js +++ b/packages/react-server/src/ReactServerStreamConfigFB.js @@ -21,7 +21,7 @@ export opaque type BinaryChunk = string; export function flushBuffered(destination: Destination) {} export const supportsRequestStorage = false; -export const requestStorage: AsyncLocalStorage = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export function beginWriting(destination: Destination) {} diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index eb76985c49218..27214b525b8cb 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -38,7 +38,7 @@ export type {TransitionStatus}; export const isPrimaryRenderer = false; export const supportsRequestStorage = false; -export const requestStorage: AsyncLocalStorage = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export const resetResumableState = $$$config.resetResumableState; export const completeResumableState = $$$config.completeResumableState; diff --git a/packages/react-server/src/forks/ReactFizzConfig.dom-edge.js b/packages/react-server/src/forks/ReactFizzConfig.dom-edge.js index 67c8d7c13a78c..7c5ba9bce7e27 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.dom-edge.js +++ b/packages/react-server/src/forks/ReactFizzConfig.dom-edge.js @@ -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 = supportsRequestStorage - ? new AsyncLocalStorage() - : (null: any); +export const requestStorage: AsyncLocalStorage = + supportsRequestStorage ? new AsyncLocalStorage() : (null: any); diff --git a/packages/react-server/src/forks/ReactFizzConfig.dom-legacy.js b/packages/react-server/src/forks/ReactFizzConfig.dom-legacy.js index 903250ce22db2..84d49396efcdf 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.dom-legacy.js +++ b/packages/react-server/src/forks/ReactFizzConfig.dom-legacy.js @@ -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 = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); diff --git a/packages/react-server/src/forks/ReactFizzConfig.dom-node.js b/packages/react-server/src/forks/ReactFizzConfig.dom-node.js index 99d0d74a7b76a..8c9718e8234c3 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.dom-node.js +++ b/packages/react-server/src/forks/ReactFizzConfig.dom-node.js @@ -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 = +export const requestStorage: AsyncLocalStorage = new AsyncLocalStorage(); diff --git a/packages/react-server/src/forks/ReactFizzConfig.dom.js b/packages/react-server/src/forks/ReactFizzConfig.dom.js index 5f887770d211e..2bf9be13273d6 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.dom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.dom.js @@ -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 = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.custom.js b/packages/react-server/src/forks/ReactFlightServerConfig.custom.js index 9c00e67bb67b1..953f83bf4a4f1 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.custom.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.custom.js @@ -23,7 +23,7 @@ export const isPrimaryRenderer = false; export const prepareHostDispatcher = () => {}; export const supportsRequestStorage = false; -export const requestStorage: AsyncLocalStorage = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export function createHints(): any { return null; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-esm.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-esm.js index ffcf103c6a0e8..929c2707c38d9 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-esm.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-esm.js @@ -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 = +export const requestStorage: AsyncLocalStorage = new AsyncLocalStorage(); export * from '../ReactFlightServerConfigDebugNoop'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-turbopack.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-turbopack.js index 4773085ba5c2b..205a7add5fdfc 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-turbopack.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser-turbopack.js @@ -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 = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export * from '../ReactFlightServerConfigDebugNoop'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser.js index 6f209caaaf590..c5f4407796161 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-browser.js @@ -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 = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export * from '../ReactFlightServerConfigDebugNoop'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-bun.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-bun.js index cfdb89861ebff..ad1743a43d18a 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-bun.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-bun.js @@ -13,6 +13,6 @@ export * from '../ReactFlightServerConfigBundlerCustom'; export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM'; export const supportsRequestStorage = false; -export const requestStorage: AsyncLocalStorage = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export * from '../ReactFlightServerConfigDebugNoop'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge-turbopack.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge-turbopack.js index 64a329a341f11..8b33307667574 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge-turbopack.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge-turbopack.js @@ -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 = supportsRequestStorage - ? new AsyncLocalStorage() - : (null: any); +export const requestStorage: AsyncLocalStorage = + 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'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge.js index b15e88ed11e46..e62460390d19e 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-edge.js @@ -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 = supportsRequestStorage - ? new AsyncLocalStorage() - : (null: any); +export const requestStorage: AsyncLocalStorage = + 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'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-fb-experimental.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-fb-experimental.js index 5239248d677f0..b26b825ea8158 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-fb-experimental.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-fb-experimental.js @@ -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 = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export * from '../ReactFlightServerConfigDebugNoop'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js index cfdb89861ebff..ad1743a43d18a 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js @@ -13,6 +13,6 @@ export * from '../ReactFlightServerConfigBundlerCustom'; export * from 'react-dom-bindings/src/server/ReactFlightServerConfigDOM'; export const supportsRequestStorage = false; -export const requestStorage: AsyncLocalStorage = (null: any); +export const requestStorage: AsyncLocalStorage = (null: any); export * from '../ReactFlightServerConfigDebugNoop'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-esm.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-esm.js index f3460fd71b925..528c3cdb3b23b 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-esm.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-esm.js @@ -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 = +export const requestStorage: AsyncLocalStorage = new AsyncLocalStorage(); export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-turbopack.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-turbopack.js index d9eb6a46e4e71..a19e29222403c 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-turbopack.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-node-turbopack.js @@ -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 = +export const requestStorage: AsyncLocalStorage = new AsyncLocalStorage(); export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks'; diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-node.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-node.js index d716d502f7598..b0da1d9926e68 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-node.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-node.js @@ -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 = +export const requestStorage: AsyncLocalStorage = new AsyncLocalStorage(); export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks'; diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js index 19adb73dd7334..255ed4a2b99ba 100644 --- a/scripts/flow/environment.js +++ b/scripts/flow/environment.js @@ -286,7 +286,7 @@ declare module 'async_hooks' { declare class AsyncLocalStorage { disable(): void; getStore(): T | void; - run(store: T, callback: (...args: any[]) => void, ...args: any[]): void; + run(store: T, callback: (...args: any[]) => R, ...args: any[]): R; enterWith(store: T): void; } declare interface AsyncResource {} @@ -316,7 +316,7 @@ declare module 'async_hooks' { declare class AsyncLocalStorage { disable(): void; getStore(): T | void; - run(store: T, callback: (...args: any[]) => void, ...args: any[]): void; + run(store: T, callback: (...args: any[]) => R, ...args: any[]): R; enterWith(store: T): void; }