From 970b2414a24c8cab859a583d1ac9bdfc35f65f98 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 18 Feb 2024 22:43:43 -0500 Subject: [PATCH 1/6] Disable log tear down check in tests --- scripts/jest/setupTests.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 0f2de09120714..0d23861568fe4 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -96,9 +96,9 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { console[methodName] !== mockMethod && !jest.isMockFunction(console[methodName]) ) { - throw new Error( - `Test did not tear down console.${methodName} mock properly.` - ); + // throw new Error( + // `Test did not tear down console.${methodName} mock properly.` + // ); } if (unexpectedConsoleCallStacks.length > 0) { const messages = unexpectedConsoleCallStacks.map( From 33e1cbff1955651da2815f8679bef55836a50983 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 18 Feb 2024 21:45:03 -0500 Subject: [PATCH 2/6] Add flag --- packages/shared/ReactFeatureFlags.js | 2 ++ packages/shared/forks/ReactFeatureFlags.native-fb.js | 1 + packages/shared/forks/ReactFeatureFlags.native-oss.js | 1 + packages/shared/forks/ReactFeatureFlags.test-renderer.js | 1 + packages/shared/forks/ReactFeatureFlags.test-renderer.native.js | 1 + packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 1 + 7 files changed, 8 insertions(+) diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 8a503e433ff30..0f8d740f64fa9 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -123,6 +123,8 @@ export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__; export const enableRenderableContext = false; +export const enableServerComponentLogs = __EXPERIMENTAL__; + /** * Enables an expiration time for retry lanes to avoid starvation. */ diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 3a7dad1b9c8a5..f62604774ebd8 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -95,6 +95,7 @@ export const enableUseDeferredValueInitialArg = true; export const disableClientCache = true; export const enableServerComponentKeys = true; +export const enableServerComponentLogs = true; export const enableInfiniteRenderLoopDetection = false; // TODO: Roll out with GK. Don't keep as dynamic flag for too long, though, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index ac09110a4a517..fe0458e3ded62 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -88,6 +88,7 @@ export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__; export const disableClientCache = true; export const enableServerComponentKeys = true; +export const enableServerComponentLogs = true; // TODO: Should turn this on in next "major" RN release. export const enableRefAsProp = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index e5dc5fe25cfee..02d8e4cff408f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -87,6 +87,7 @@ export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__; export const disableClientCache = true; export const enableServerComponentKeys = true; +export const enableServerComponentLogs = true; export const enableInfiniteRenderLoopDetection = false; // TODO: This must be in sync with the main ReactFeatureFlags file because diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 6081e57da8ef4..f1d91a36e6456 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -85,6 +85,7 @@ export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__; export const disableClientCache = true; export const enableServerComponentKeys = true; +export const enableServerComponentLogs = true; export const enableRefAsProp = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 6694b6b8afeff..15a8ded7de0e7 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -87,6 +87,7 @@ export const enableUseDeferredValueInitialArg = true; export const disableClientCache = true; export const enableServerComponentKeys = true; +export const enableServerComponentLogs = true; export const enableInfiniteRenderLoopDetection = false; export const enableRefAsProp = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 2babd5bcd695c..6a49725070410 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -115,6 +115,7 @@ export const enableAsyncDebugInfo = false; export const disableClientCache = true; export const enableServerComponentKeys = true; +export const enableServerComponentLogs = true; // TODO: Roll out with GK. Don't keep as dynamic flag for too long, though, // because JSX is an extremely hot path. From 4d8861efb80570c8c3d753be7fb66a36b88973dc Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 18 Feb 2024 22:40:27 -0500 Subject: [PATCH 3/6] Instrument console in a Flight server environment and replay them on the client --- .../react-client/src/ReactFlightClient.js | 25 ++++++ .../react-server/src/ReactFlightServer.js | 85 +++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index a7476f5d972e2..d8f92e8261ad3 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -1063,6 +1063,23 @@ function resolveDebugInfo( chunkDebugInfo.push(debugInfo); } +function resolveConsoleEntry( + response: Response, + payload: [string /*methodName */, ...any], +): void { + if (!__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'resolveConsoleEntry should never be called in production mode. This is a bug in React.', + ); + } + const methodName = payload[0]; + const args = payload.slice(1); + // eslint-disable-next-line react-internal/no-production-logging + console[methodName].apply(console, args); +} + function mergeBuffer( buffer: Array, lastChunk: Uint8Array, @@ -1212,6 +1229,14 @@ function processFullRow( resolveDebugInfo(response, id, debugInfo); return; } + // Fallthrough to share the error with Console entries. + } + case 87 /* "W" */: { + if (__DEV__) { + const payload = JSON.parse(row); + resolveConsoleEntry(response, payload); + return; + } throw new Error( 'Failed to read a RSC payload created by a development version of React ' + 'on the server while using a production version on the client. Always use ' + diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 1444b6c6e6099..ef23879b01858 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -17,6 +17,7 @@ import { enableTaint, enableServerComponentKeys, enableRefAsProp, + enableServerComponentLogs, } from 'shared/ReactFeatureFlags'; import { @@ -111,6 +112,64 @@ import {SuspenseException, getSuspendedThenable} from './ReactFlightThenable'; initAsyncDebugInfo(); +function patchConsole(consoleInst: typeof console, methodName: string) { + const descriptor = Object.getOwnPropertyDescriptor(consoleInst, methodName); + if ( + descriptor && + (descriptor.configurable || descriptor.writable) && + typeof descriptor.value === 'function' + ) { + const originalMethod = descriptor.value; + // $FlowFixMe[incompatible-call]: We should be able to get descriptors from any function. + const originalName = Object.getOwnPropertyDescriptor(originalMethod, 'name'); + const wrapperMethod = function (this: typeof console) { + const request = resolveRequest(); + if (request !== null) { + request.pendingChunks++; + // We don't currently use this id for anything but we emit it so that we can later + // refer to previous logs in debug info to associate them with a component. + const id = request.nextChunkId++; + emitConsoleChunk(request, id, methodName, arguments); + } + // $FlowFixMe[prop-missing] + return originalMethod.apply(this, arguments); + }; + if (originalName) { + Object.defineProperty( + wrapperMethod, + // $FlowFixMe[cannot-write] yes it is + 'name', + originalName, + ); + } + Object.defineProperty(consoleInst, methodName, { + value: wrapperMethod, + }); + } +} + +if ( + enableServerComponentLogs && + __DEV__ && + typeof console === 'object' && + console !== null +) { + // Instrument console to capture logs for replaying on the client. + patchConsole(console, 'assert'); + patchConsole(console, 'debug'); + patchConsole(console, 'dir'); + patchConsole(console, 'dirxml'); + patchConsole(console, 'error'); + patchConsole(console, 'group'); + patchConsole(console, 'groupCollapsed'); + patchConsole(console, 'groupEnd'); + patchConsole(console, 'info'); + patchConsole(console, 'log'); + patchConsole(console, 'table'); + patchConsole(console, 'trace'); + patchConsole(console, 'warn'); +} + const ObjectPrototype = Object.prototype; type JSONValue = @@ -1782,6 +1841,32 @@ function emitDebugChunk( request.completedRegularChunks.push(processedChunk); } +function emitConsoleChunk( + request: Request, + id: number, + methodName: string, + args: Array +): void { + if (!__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'emitConsoleChunk should never be called in production mode. This is a bug in React.', + ); + } + const payload = [methodName]; + // $FlowFixMe[method-unbinding] + payload.push.apply(payload, args); + // $FlowFixMe[incompatible-type] stringify can return null + const json: string = stringify(payload); + const row = serializeRowHeader('W', id) + json + '\n'; + const processedChunk = stringToChunk(row); + // Add it to import chunks since this has the highest priority it ensures that + // the log comes before in the timeline any consequence that happens after. + // Ensuring ordering. + request.completedImportChunks.push(processedChunk); +} + function forwardDebugInfo( request: Request, id: number, From a5f55286b00067a3dd904449b406f9ffd3a6fd72 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 Feb 2024 18:34:42 -0500 Subject: [PATCH 4/6] Pass stack trace as part as part of the console entry --- .../react-client/src/ReactFlightClient.js | 10 ++++--- .../react-server/src/ReactFlightServer.js | 30 +++++++++++++++---- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index d8f92e8261ad3..25c193d8c7040 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -1065,7 +1065,7 @@ function resolveDebugInfo( function resolveConsoleEntry( response: Response, - payload: [string /*methodName */, ...any], + payload: [string /*methodName */, string /* stackTrace */, ...any], ): void { if (!__DEV__) { // These errors should never make it into a build so we don't need to encode them in codes.json @@ -1075,9 +1075,11 @@ function resolveConsoleEntry( ); } const methodName = payload[0]; - const args = payload.slice(1); - // eslint-disable-next-line react-internal/no-production-logging - console[methodName].apply(console, args); + // TODO: Restore the fake stack before logging. + // const stackTrace = payload[1]; + const args = payload.slice(2); + // eslint-disable-next-line react-internal/no-production-logging + console[methodName].apply(console, args); } function mergeBuffer( diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index ef23879b01858..9e8b5f9d83989 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -121,15 +121,34 @@ function patchConsole(consoleInst: typeof console, methodName: string) { ) { const originalMethod = descriptor.value; // $FlowFixMe[incompatible-call]: We should be able to get descriptors from any function. - const originalName = Object.getOwnPropertyDescriptor(originalMethod, 'name'); + const originalName = Object.getOwnPropertyDescriptor( + originalMethod, + 'name', + ); const wrapperMethod = function (this: typeof console) { const request = resolveRequest(); - if (request !== null) { + if (methodName === 'assert' && arguments[0]) { + // assert doesn't emit anything unless first argument is falsy so we can skip it. + } else if (request !== null) { + // Extract the stack. Not all console logs print the full stack but they have at + // least the line it was called from. We could optimize transfer by keeping just + // one stack frame but keeping it simple for now and include all frames. + let stack = new Error().stack; + if (stack.startsWith('Error: \n')) { + stack = stack.slice(8); + } + const firstLine = stack.indexOf('\n'); + if (firstLine === -1) { + stack = ''; + } else { + // Skip the console wrapper itself. + stack = stack.slice(firstLine + 1); + } request.pendingChunks++; // We don't currently use this id for anything but we emit it so that we can later // refer to previous logs in debug info to associate them with a component. const id = request.nextChunkId++; - emitConsoleChunk(request, id, methodName, arguments); + emitConsoleChunk(request, id, methodName, stack, arguments); } // $FlowFixMe[prop-missing] return originalMethod.apply(this, arguments); @@ -1845,7 +1864,8 @@ function emitConsoleChunk( request: Request, id: number, methodName: string, - args: Array + stackTrace: string, + args: Array, ): void { if (!__DEV__) { // These errors should never make it into a build so we don't need to encode them in codes.json @@ -1854,7 +1874,7 @@ function emitConsoleChunk( 'emitConsoleChunk should never be called in production mode. This is a bug in React.', ); } - const payload = [methodName]; + const payload = [methodName, stackTrace]; // $FlowFixMe[method-unbinding] payload.push.apply(payload, args); // $FlowFixMe[incompatible-type] stringify can return null From 2fe5fb065527eeca2ab6b238ee13a3cbc4d4d6f3 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 Feb 2024 21:47:19 -0500 Subject: [PATCH 5/6] Serialize console arguments using a special model serializer This allows complex objects to be logged and inspected on the client. We limit the depth of objects after 20 objects are written. This deals with cycles too. Because these objects aren't the complete model we can't cache them but if the full thing is already written, we can reuse that. This might not be fully ok since if there's an error:ed reference inside of it, it would error the log. This should never error nor suspend. Functions are emitted as eval of their source string to produce a similar looking function on the client. Unresolved Promises are serialized as infinite promises. --- .../react-client/src/ReactFlightClient.js | 26 +- .../react-server/src/ReactFlightServer.js | 325 +++++++++++++++++- 2 files changed, 342 insertions(+), 9 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 25c193d8c7040..4bdf579afd77b 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -670,6 +670,10 @@ function parseModelString( } case '@': { // Promise + if (value.length === 2) { + // Infinite promise that never resolves. + return new Promise(() => {}); + } const id = parseInt(value.slice(2), 16); const chunk = getChunk(response, id); return chunk; @@ -725,6 +729,21 @@ function parseModelString( // BigInt return BigInt(value.slice(2)); } + case 'E': { + if (__DEV__) { + // In DEV mode we allow indirect eval to produce functions for logging. + // This should not compile to eval() because then it has local scope access. + try { + // eslint-disable-next-line no-eval + return (0, eval)(value.slice(2)); + } catch (x) { + // We currently use this to express functions so we fail parsing it, + // let's just return a blank function as a place holder. + return function () {}; + } + } + // Fallthrough + } default: { // We assume that anything else is a reference ID. const id = parseInt(value.slice(1), 16); @@ -1065,7 +1084,7 @@ function resolveDebugInfo( function resolveConsoleEntry( response: Response, - payload: [string /*methodName */, string /* stackTrace */, ...any], + value: UninitializedModel, ): void { if (!__DEV__) { // These errors should never make it into a build so we don't need to encode them in codes.json @@ -1074,6 +1093,8 @@ function resolveConsoleEntry( 'resolveConsoleEntry should never be called in production mode. This is a bug in React.', ); } + + const payload: [string, string, mixed] = parseModel(response, value); const methodName = payload[0]; // TODO: Restore the fake stack before logging. // const stackTrace = payload[1]; @@ -1235,8 +1256,7 @@ function processFullRow( } case 87 /* "W" */: { if (__DEV__) { - const payload = JSON.parse(row); - resolveConsoleEntry(response, payload); + resolveConsoleEntry(response, row); return; } throw new Error( diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 9e8b5f9d83989..ecdd018be3c63 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -120,8 +120,8 @@ function patchConsole(consoleInst: typeof console, methodName: string) { typeof descriptor.value === 'function' ) { const originalMethod = descriptor.value; - // $FlowFixMe[incompatible-call]: We should be able to get descriptors from any function. const originalName = Object.getOwnPropertyDescriptor( + // $FlowFixMe[incompatible-call]: We should be able to get descriptors from any function. originalMethod, 'name', ); @@ -939,6 +939,10 @@ function serializeLazyID(id: number): string { return '$L' + id.toString(16); } +function serializeInfinitePromise(): string { + return '$@'; +} + function serializePromiseID(id: number): string { return '$@' + id.toString(16); } @@ -1853,6 +1857,7 @@ function emitDebugChunk( 'emitDebugChunk should never be called in production mode. This is a bug in React.', ); } + // $FlowFixMe[incompatible-type] stringify can return null const json: string = stringify(debugInfo); const row = serializeRowHeader('D', id) + json + '\n'; @@ -1860,6 +1865,295 @@ function emitDebugChunk( request.completedRegularChunks.push(processedChunk); } +function serializeEval(source: string): string { + if (!__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'serializeEval should never be called in production mode. This is a bug in React.', + ); + } + return '$E' + source; +} + +// This is a forked version of renderModel which should never error, never suspend and is limited +// in the depth it can encode. +function renderConsoleValue( + request: Request, + counter: {objectCount: number}, + parent: + | {+[propertyName: string | number]: ReactClientValue} + | $ReadOnlyArray, + parentPropertyName: string, + value: ReactClientValue, +): ReactJSONValue { + // Make sure that `parent[parentPropertyName]` wasn't JSONified before `value` was passed to us + // $FlowFixMe[incompatible-use] + const originalValue = parent[parentPropertyName]; + if ( + typeof originalValue === 'object' && + originalValue !== value && + !(originalValue instanceof Date) + ) { + } + + if (value === null) { + return null; + } + + if (typeof value === 'object') { + if (isClientReference(value)) { + // We actually have this value on the client so we could import it. + // This might be confusing though because on the Server it won't actually + // be this value, so if you're debugging client references maybe you'd be + // better with a place holder. + return serializeClientReference( + request, + parent, + parentPropertyName, + (value: any), + ); + } + + if (counter.objectCount > 20) { + // We've reached our max number of objects to serialize across the wire so we serialize this + // object but no properties inside of it, as a place holder. + return Array.isArray(value) ? [] : {}; + } + + counter.objectCount++; + + const writtenObjects = request.writtenObjects; + const existingId = writtenObjects.get(value); + // $FlowFixMe[method-unbinding] + if (typeof value.then === 'function') { + if (existingId !== undefined) { + // We've seen this promise before, so we can just refer to the same result. + return serializePromiseID(existingId); + } + + const thenable: Thenable = (value: any); + switch (thenable.status) { + case 'fulfilled': { + return serializePromiseID( + outlineConsoleValue(request, counter, thenable.value), + ); + } + case 'rejected': { + const x = thenable.reason; + request.pendingChunks++; + const errorId = request.nextChunkId++; + if ( + enablePostpone && + typeof x === 'object' && + x !== null && + (x: any).$$typeof === REACT_POSTPONE_TYPE + ) { + const postponeInstance: Postpone = (x: any); + // We don't log this postpone. + emitPostponeChunk(request, errorId, postponeInstance); + } else { + // We don't log these errors since they didn't actually throw into Flight. + const digest = ''; + emitErrorChunk(request, errorId, digest, x); + } + return serializePromiseID(errorId); + } + } + // If it hasn't already resolved (and been instrumented) we just encode an infinite + // promise that will never resolve. + return serializeInfinitePromise(); + } + + if (existingId !== undefined && existingId !== -1) { + // We've already emitted this as a real object, so we can + // just refer to that by its existing ID. + return serializeByValueID(existingId); + } + + if (isArray(value)) { + return value; + } + + if (value instanceof Map) { + return serializeMap(request, value); + } + if (value instanceof Set) { + return serializeSet(request, value); + } + + if (enableBinaryFlight) { + if (value instanceof ArrayBuffer) { + return serializeTypedArray(request, 'A', new Uint8Array(value)); + } + if (value instanceof Int8Array) { + // char + return serializeTypedArray(request, 'C', value); + } + if (value instanceof Uint8Array) { + // unsigned char + return serializeTypedArray(request, 'c', value); + } + if (value instanceof Uint8ClampedArray) { + // unsigned clamped char + return serializeTypedArray(request, 'U', value); + } + if (value instanceof Int16Array) { + // sort + return serializeTypedArray(request, 'S', value); + } + if (value instanceof Uint16Array) { + // unsigned short + return serializeTypedArray(request, 's', value); + } + if (value instanceof Int32Array) { + // long + return serializeTypedArray(request, 'L', value); + } + if (value instanceof Uint32Array) { + // unsigned long + return serializeTypedArray(request, 'l', value); + } + if (value instanceof Float32Array) { + // float + return serializeTypedArray(request, 'F', value); + } + if (value instanceof Float64Array) { + // double + return serializeTypedArray(request, 'd', value); + } + if (value instanceof BigInt64Array) { + // number + return serializeTypedArray(request, 'N', value); + } + if (value instanceof BigUint64Array) { + // unsigned number + // We use "m" instead of "n" since JSON can start with "null" + return serializeTypedArray(request, 'm', value); + } + if (value instanceof DataView) { + return serializeTypedArray(request, 'V', value); + } + } + + const iteratorFn = getIteratorFn(value); + if (iteratorFn) { + return Array.from((value: any)); + } + + // $FlowFixMe[incompatible-return] + return value; + } + + if (typeof value === 'string') { + if (value[value.length - 1] === 'Z') { + // Possibly a Date, whose toJSON automatically calls toISOString + if (originalValue instanceof Date) { + return serializeDateFromDateJSON(value); + } + } + if (value.length >= 1024) { + // For large strings, we encode them outside the JSON payload so that we + // don't have to double encode and double parse the strings. This can also + // be more compact in case the string has a lot of escaped characters. + return serializeLargeTextString(request, value); + } + return escapeStringValue(value); + } + + if (typeof value === 'boolean') { + return value; + } + + if (typeof value === 'number') { + return serializeNumber(value); + } + + if (typeof value === 'undefined') { + return serializeUndefined(); + } + + if (typeof value === 'function') { + if (isClientReference(value)) { + return serializeClientReference( + request, + parent, + parentPropertyName, + (value: any), + ); + } + + // Serialize the body of the function as an eval so it can be printed. + // $FlowFixMe[method-unbinding] + return serializeEval('(' + Function.prototype.toString.call(value) + ')'); + } + + if (typeof value === 'symbol') { + const writtenSymbols = request.writtenSymbols; + const existingId = writtenSymbols.get(value); + if (existingId !== undefined) { + return serializeByValueID(existingId); + } + // $FlowFixMe[incompatible-type] `description` might be undefined + const name: string = value.description; + // We use the Symbol.for version if it's not a global symbol. Close enough. + request.pendingChunks++; + const symbolId = request.nextChunkId++; + emitSymbolChunk(request, symbolId, name); + return serializeByValueID(symbolId); + } + + if (typeof value === 'bigint') { + return serializeBigInt(value); + } + + return 'unknown type ' + typeof value; +} + +function outlineConsoleValue( + request: Request, + counter: {objectCount: number}, + model: ReactClientValue, +): number { + if (!__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'outlineConsoleValue should never be called in production mode. This is a bug in React.', + ); + } + + function replacer( + this: + | {+[key: string | number]: ReactClientValue} + | $ReadOnlyArray, + parentPropertyName: string, + value: ReactClientValue, + ): ReactJSONValue { + try { + return renderConsoleValue( + request, + counter, + this, + parentPropertyName, + value, + ); + } catch (x) { + return 'unknown value'; + } + } + + // $FlowFixMe[incompatible-type] stringify can return null + const json: string = stringify(model, replacer); + + request.pendingChunks++; + const id = request.nextChunkId++; + const row = id.toString(16) + ':' + json + '\n'; + const processedChunk = stringToChunk(row); + request.completedRegularChunks.push(processedChunk); + return id; +} + function emitConsoleChunk( request: Request, id: number, @@ -1874,17 +2168,36 @@ function emitConsoleChunk( 'emitConsoleChunk should never be called in production mode. This is a bug in React.', ); } + + const counter = {objectCount: 0}; + function replacer( + this: + | {+[key: string | number]: ReactClientValue} + | $ReadOnlyArray, + parentPropertyName: string, + value: ReactClientValue, + ): ReactJSONValue { + try { + return renderConsoleValue( + request, + counter, + this, + parentPropertyName, + value, + ); + } catch (x) { + return 'unknown value'; + } + } + const payload = [methodName, stackTrace]; // $FlowFixMe[method-unbinding] payload.push.apply(payload, args); // $FlowFixMe[incompatible-type] stringify can return null - const json: string = stringify(payload); + const json: string = stringify(payload, replacer); const row = serializeRowHeader('W', id) + json + '\n'; const processedChunk = stringToChunk(row); - // Add it to import chunks since this has the highest priority it ensures that - // the log comes before in the timeline any consequence that happens after. - // Ensuring ordering. - request.completedImportChunks.push(processedChunk); + request.completedRegularChunks.push(processedChunk); } function forwardDebugInfo( From 609cb1951d8f31c6c6135b76f7125513fceece84 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 20 Feb 2024 12:50:48 -0500 Subject: [PATCH 6/6] 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 | 41 +++++++++++++++++++ .../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, 91 insertions(+), 30 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 5508a52c920d9..8c088b3deace7 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -1995,4 +1995,45 @@ describe('ReactFlight', () => { , ); }); + + // @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: }); + }).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; + } + }); }); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index ecdd018be3c63..887aa96370151 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1721,13 +1721,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; }