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.
facebook · Feb 20, 2024 · a2ef462 · a2ef462
1 parent c054691
commit a2ef462
Show file tree

Hide file tree

Showing 21 changed files with 95 additions and 30 deletions.
diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -1978,4 +1978,49 @@ 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);
+
+      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
@@ -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
@@ -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) {}
 

diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (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
@@ -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);
diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);
diff --git 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<Request> =
+export const requestStorage: AsyncLocalStorage<Request | void> =
   new AsyncLocalStorage();
diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (null: any);
diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (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
@@ -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';
diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (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
@@ -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';
diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (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
@@ -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';

diff --git 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<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';

diff --git 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<Request> = (null: any);
+export const requestStorage: AsyncLocalStorage<Request | void> = (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
@@ -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';
diff --git 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<Request> =
+export const requestStorage: AsyncLocalStorage<Request | void> =
   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
@@ -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';

diff --git 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<Request> =
+export const requestStorage: AsyncLocalStorage<Request | void> =
   new AsyncLocalStorage();
 
 export {createHook as createAsyncHook, executionAsyncId} from 'async_hooks';

diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js
@@ -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 {}
@@ -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;
 }