From 6e5b82e9a89478194b04f0502af3cce1029986ef Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 3 Jul 2024 14:05:17 -0400 Subject: [PATCH 1/3] Track the current owner/stack/task on the task This tracks it for attribution when serializing child properties. Unfortunately, because we don't track the stack we never pop it so it'll keep tracking for serializing sibling properties. We rely on "children" typically being the last property in the common case anyway. It also doesn't affect client errors - only attribution on the RSC server itself. --- .../react-server/src/ReactFlightServer.js | 123 +++++++++++------- 1 file changed, 79 insertions(+), 44 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 44873a927e83e..a7c5a579e2590 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -426,6 +426,9 @@ type Task = { implicitSlot: boolean, // true if the root server component of this sequence had a null key thenableState: ThenableState | null, environmentName: string, // DEV-only. Used to track if the environment for this task changed. + debugOwner: null | ReactComponentInfo, // DEV-only + debugStack: null | string, // DEV-only + debugTask: null | ConsoleTask, // DEV-only }; interface Reference {} @@ -577,7 +580,16 @@ function RequestInstance( : environmentName; this.didWarnForKey = null; } - const rootTask = createTask(this, model, null, false, abortSet); + const rootTask = createTask( + this, + model, + null, + false, + abortSet, + null, + null, + null, + ); pingedTasks.push(rootTask); } @@ -624,6 +636,9 @@ function serializeThenable( task.keyPath, // the server component sequence continues through Promise-as-a-child. task.implicitSlot, request.abortableTasks, + __DEV__ && enableOwnerStacks ? task.debugOwner : null, + __DEV__ && enableOwnerStacks ? task.debugStack : null, + __DEV__ && enableOwnerStacks ? task.debugTask : null, ); if (__DEV__) { // If this came from Flight, forward any debug info into this new row. @@ -753,6 +768,9 @@ function serializeReadableStream( task.keyPath, task.implicitSlot, request.abortableTasks, + __DEV__ && enableOwnerStacks ? task.debugOwner : null, + __DEV__ && enableOwnerStacks ? task.debugStack : null, + __DEV__ && enableOwnerStacks ? task.debugTask : null, ); request.abortableTasks.delete(streamTask); @@ -849,6 +867,9 @@ function serializeAsyncIterable( task.keyPath, task.implicitSlot, request.abortableTasks, + __DEV__ && enableOwnerStacks ? task.debugOwner : null, + __DEV__ && enableOwnerStacks ? task.debugStack : null, + __DEV__ && enableOwnerStacks ? task.debugTask : null, ); request.abortableTasks.delete(streamTask); @@ -1083,9 +1104,6 @@ function renderFunctionComponent( key: null | string, Component: (p: Props, arg: void) => any, props: Props, - owner: null | ReactComponentInfo, // DEV-only - stack: null | string, // DEV-only - debugTask: null | ConsoleTask, // DEV-only validated: number, // DEV-only ): ReactJSONValue { // Reset the task's thenable state before continuing, so that if a later @@ -1117,11 +1135,11 @@ function renderFunctionComponent( componentDebugInfo = ({ name: componentName, env: componentEnv, - owner: owner, + owner: task.debugOwner, }: ReactComponentInfo); if (enableOwnerStacks) { // $FlowFixMe[cannot-write] - componentDebugInfo.stack = stack; + componentDebugInfo.stack = task.debugStack; } // We outline this model eagerly so that we can refer to by reference as an owner. // If we had a smarter way to dedupe we might not have to do this if there ends up @@ -1138,7 +1156,7 @@ function renderFunctionComponent( key, validated, componentDebugInfo, - debugTask, + task.debugTask, ); } } @@ -1147,7 +1165,7 @@ function renderFunctionComponent( Component, props, componentDebugInfo, - debugTask, + task.debugTask, ); } else { prepareToUseHooksForComponent(prevThenableState, null); @@ -1489,8 +1507,6 @@ function renderClientElement( type: any, key: null | string, props: any, - owner: null | ReactComponentInfo, // DEV-only - stack: null | string, // DEV-only validated: number, // DEV-only ): ReactJSONValue { // We prepend the terminal client element that actually gets serialized with @@ -1503,8 +1519,16 @@ function renderClientElement( } const element = __DEV__ ? enableOwnerStacks - ? [REACT_ELEMENT_TYPE, type, key, props, owner, stack, validated] - : [REACT_ELEMENT_TYPE, type, key, props, owner] + ? [ + REACT_ELEMENT_TYPE, + type, + key, + props, + task.debugOwner, + task.debugStack, + validated, + ] + : [REACT_ELEMENT_TYPE, type, key, props, task.debugOwner] : [REACT_ELEMENT_TYPE, type, key, props]; if (task.implicitSlot && key !== null) { // The root Server Component had no key so it was in an implicit slot. @@ -1531,6 +1555,9 @@ function outlineTask(request: Request, task: Task): ReactJSONValue { task.keyPath, // unlike outlineModel this one carries along context task.implicitSlot, request.abortableTasks, + __DEV__ && enableOwnerStacks ? task.debugOwner : null, + __DEV__ && enableOwnerStacks ? task.debugStack : null, + __DEV__ && enableOwnerStacks ? task.debugTask : null, ); retryTask(request, newTask); @@ -1551,9 +1578,6 @@ function renderElement( key: null | string, ref: mixed, props: any, - owner: null | ReactComponentInfo, // DEV only - stack: null | string, // DEV only - debugTask: null | ConsoleTask, // DEV only validated: number, // DEV only ): ReactJSONValue { if (ref !== null && ref !== undefined) { @@ -1578,17 +1602,7 @@ function renderElement( !isOpaqueTemporaryReference(type) ) { // This is a Server Component. - return renderFunctionComponent( - request, - task, - key, - type, - props, - owner, - stack, - debugTask, - validated, - ); + return renderFunctionComponent(request, task, key, type, props, validated); } else if (type === REACT_FRAGMENT_TYPE && key === null) { // For key-less fragments, we add a small optimization to avoid serializing // it as a wrapper. @@ -1633,9 +1647,6 @@ function renderElement( key, ref, props, - owner, - stack, - debugTask, validated, ); } @@ -1646,9 +1657,6 @@ function renderElement( key, type.render, props, - owner, - stack, - debugTask, validated, ); } @@ -1660,9 +1668,6 @@ function renderElement( key, ref, props, - owner, - stack, - debugTask, validated, ); } @@ -1680,7 +1685,7 @@ function renderElement( // We don't know if the client will support it or not. This might error on the // client or error during serialization but the stack will point back to the // server. - return renderClientElement(task, type, key, props, owner, stack, validated); + return renderClientElement(task, type, key, props, validated); } function pingTask(request: Request, task: Task): void { @@ -1698,6 +1703,9 @@ function createTask( keyPath: null | string, implicitSlot: boolean, abortSet: Set, + debugOwner: null | ReactComponentInfo, // DEV-only + debugStack: null | string, // DEV-only + debugTask: null | ConsoleTask, // DEV-only ): Task { request.pendingChunks++; const id = request.nextChunkId++; @@ -1764,9 +1772,17 @@ function createTask( return renderModel(request, task, parent, parentPropertyName, value); }, thenableState: null, - }: Omit): any); + }: Omit< + Task, + 'environmentName' | 'debugOwner' | 'debugStack' | 'debugTask', + >): any); if (__DEV__) { task.environmentName = request.environmentName(); + if (enableOwnerStacks) { + task.debugOwner = debugOwner; + task.debugStack = debugStack; + task.debugTask = debugTask; + } } abortSet.add(task); return task; @@ -1897,6 +1913,9 @@ function outlineModel(request: Request, value: ReactClientValue): number { null, // The way we use outlining is for reusing an object. false, // It makes no sense for that use case to be contextual. request.abortableTasks, + null, // TODO: Currently we don't associate any debug information with + null, // this object on the server. If it ends up erroring, it won't + null, // have any context on the server but can on the client. ); retryTask(request, newTask); return newTask.id; @@ -1990,6 +2009,9 @@ function serializeBlob(request: Request, blob: Blob): string { null, false, request.abortableTasks, + null, // TODO: Currently we don't associate any debug information with + null, // this object on the server. If it ends up erroring, it won't + null, // have any context on the server but can on the client. ); const reader = blob.stream().getReader(); @@ -2098,6 +2120,9 @@ function renderModel( task.keyPath, task.implicitSlot, request.abortableTasks, + __DEV__ && enableOwnerStacks ? task.debugOwner : null, + __DEV__ && enableOwnerStacks ? task.debugStack : null, + __DEV__ && enableOwnerStacks ? task.debugTask : null, ); const ping = newTask.ping; (x: any).then(ping, ping); @@ -2252,6 +2277,23 @@ function renderModelDestructive( } // Attempt to render the Server Component. + + if (__DEV__) { + task.debugOwner = element._owner; + if (enableOwnerStacks) { + task.debugStack = + !element._debugStack || typeof element._debugStack === 'string' + ? element._debugStack + : filterDebugStack(element._debugStack); + task.debugTask = element._debugTask; + } + // TODO: Pop this. Since we currently don't have a point where we can pop the stack + // this debug information will be used for errors inside sibling properties that + // are not elements. Leading to the wrong attribution on the server. We could fix + // that if we switch to a proper stack instead of JSON.stringify's trampoline. + // Attribution on the client is still correct since it has a pop. + } + const newChild = renderElement( request, task, @@ -2260,13 +2302,6 @@ function renderModelDestructive( element.key, ref, props, - __DEV__ ? element._owner : null, - __DEV__ && enableOwnerStacks - ? !element._debugStack || typeof element._debugStack === 'string' - ? element._debugStack - : filterDebugStack(element._debugStack) - : null, - __DEV__ && enableOwnerStacks ? element._debugTask : null, __DEV__ && enableOwnerStacks ? element._store.validated : 0, ); if ( From af74df2381d6f33da211820b058c74ac5ee4ec71 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 3 Jul 2024 15:48:16 -0400 Subject: [PATCH 2/3] Run our own console.error in the context of the parent component It's annoying to have to remember to do this. We could always wrap the whole rendering in such as context but it would add more overhead since this rarely actually happens. It might make sense to track the whole current task instead to lower the overhead. That's what we do in Fizz. We'd still have to remember to restore the debug task though although Fizz doesn't do that neither. Also wrap onError and onPostpone with this context. --- .../src/__tests__/ReactFlight-test.js | 63 +++++- .../react-server/src/ReactFlightServer.js | 202 ++++++++++++------ 2 files changed, 199 insertions(+), 66 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 2293e9e3a77c0..11969871a92d9 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2761,10 +2761,61 @@ describe('ReactFlight', () => { ); }); + // @gate __DEV__ && enableOwnerStacks + it('can get the component owner stacks for onError in dev', async () => { + const thrownError = new Error('hi'); + let caughtError; + let ownerStack; + + function Foo() { + return ReactServer.createElement(Bar, null); + } + function Bar() { + return ReactServer.createElement( + 'div', + null, + ReactServer.createElement(Baz, null), + ); + } + function Baz() { + throw thrownError; + } + + ReactNoopFlightServer.render( + ReactServer.createElement( + 'div', + null, + ReactServer.createElement(Foo, null), + ), + { + onError(error, errorInfo) { + caughtError = error; + ownerStack = ReactServer.captureOwnerStack + ? ReactServer.captureOwnerStack() + : null; + }, + }, + ); + + expect(caughtError).toBe(thrownError); + expect(normalizeCodeLocInfo(ownerStack)).toBe( + '\n in Bar (at **)' + '\n in Foo (at **)', + ); + }); + // @gate (enableOwnerStacks && enableServerComponentLogs) || !__DEV__ it('should not include component stacks in replayed logs (unless DevTools add them)', () => { + class MyError extends Error { + toJSON() { + return 123; + } + } + function Foo() { - return 'hi'; + return ReactServer.createElement('div', null, [ + 'Womp womp: ', + new MyError('spaghetti'), + ]); } function Bar() { @@ -2781,11 +2832,18 @@ describe('ReactFlight', () => { const transport = ReactNoopFlightServer.render( ReactServer.createElement(App), ); + assertConsoleErrorDev([ 'Each child in a list should have a unique "key" prop.' + ' See https://react.dev/link/warning-keys for more information.\n' + ' in Bar (at **)\n' + ' in App (at **)', + 'Error objects cannot be rendered as text children. Try formatting it using toString().\n' + + '
Womp womp: {Error}
\n' + + ' ^^^^^^^\n' + + ' in Foo (at **)\n' + + ' in Bar (at **)\n' + + ' in App (at **)', ]); // Replay logs on the client @@ -2794,6 +2852,9 @@ describe('ReactFlight', () => { [ 'Each child in a list should have a unique "key" prop.' + ' See https://react.dev/link/warning-keys for more information.', + 'Error objects cannot be rendered as text children. Try formatting it using toString().\n' + + '
Womp womp: {Error}
\n' + + ' ^^^^^^^', ], // We should not have a stack in the replay because that should be added either by console.createTask // or React DevTools on the client. Neither of which we do here. diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index a7c5a579e2590..3decabdf058d1 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -664,10 +664,10 @@ function serializeThenable( (x: any).$$typeof === REACT_POSTPONE_TYPE ) { const postponeInstance: Postpone = (x: any); - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, newTask); emitPostponeChunk(request, newTask.id, postponeInstance); } else { - const digest = logRecoverableError(request, x); + const digest = logRecoverableError(request, x, null); emitErrorChunk(request, newTask.id, digest, x); } return newTask.id; @@ -723,11 +723,11 @@ function serializeThenable( (reason: any).$$typeof === REACT_POSTPONE_TYPE ) { const postponeInstance: Postpone = (reason: any); - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, newTask); emitPostponeChunk(request, newTask.id, postponeInstance); } else { newTask.status = ERRORED; - const digest = logRecoverableError(request, reason); + const digest = logRecoverableError(request, reason, newTask); emitErrorChunk(request, newTask.id, digest, reason); } request.abortableTasks.delete(newTask); @@ -819,10 +819,10 @@ function serializeReadableStream( (reason: any).$$typeof === REACT_POSTPONE_TYPE ) { const postponeInstance: Postpone = (reason: any); - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, streamTask); emitPostponeChunk(request, streamTask.id, postponeInstance); } else { - const digest = logRecoverableError(request, reason); + const digest = logRecoverableError(request, reason, streamTask); emitErrorChunk(request, streamTask.id, digest, reason); } enqueueFlush(request); @@ -951,10 +951,10 @@ function serializeAsyncIterable( (reason: any).$$typeof === REACT_POSTPONE_TYPE ) { const postponeInstance: Postpone = (reason: any); - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, streamTask); emitPostponeChunk(request, streamTask.id, postponeInstance); } else { - const digest = logRecoverableError(request, reason); + const digest = logRecoverableError(request, reason, streamTask); emitErrorChunk(request, streamTask.id, digest, reason); } enqueueFlush(request); @@ -1098,6 +1098,52 @@ function callLazyInitInDEV(lazy: LazyComponent): any { return init(payload); } +function callWithDebugContextInDEV( + task: Task, + callback: A => T, + arg: A, +): T { + // We don't have a Server Component instance associated with this callback and + // the nearest context is likely a Client Component being serialized. We create + // a fake owner during this callback so we can get the stack trace from it. + // This also gets sent to the client as the owner for the replaying log. + const componentDebugInfo: ReactComponentInfo = { + env: task.environmentName, + owner: task.debugOwner, + }; + if (enableOwnerStacks) { + // $FlowFixMe[cannot-write] + componentDebugInfo.stack = task.debugStack; + } + const debugTask = task.debugTask; + // We don't need the async component storage context here so we only set the + // synchronous tracking of owner. + setCurrentOwner(componentDebugInfo); + try { + if (enableOwnerStacks && debugTask) { + if (supportsRequestStorage) { + // Exit the request context while running callbacks. + return debugTask.run( + // $FlowFixMe[method-unbinding] + requestStorage.run.bind( + requestStorage, + undefined, + callback.bind(null, arg), + ), + ); + } + return debugTask.run(callback.bind(null, arg)); + } + if (supportsRequestStorage) { + // Exit the request context while running callbacks. + return requestStorage.run(undefined, callback, arg); + } + return callback(arg); + } finally { + setCurrentOwner(null); + } +} + function renderFunctionComponent( request: Request, task: Task, @@ -1240,10 +1286,12 @@ function renderFunctionComponent( Object.prototype.toString.call(iterableChild) === '[object Generator]'; if (!isGeneratorComponent) { - console.error( - 'Returning an Iterator from a Server Component is not supported ' + - 'since it cannot be looped over more than once. ', - ); + callWithDebugContextInDEV(task, () => { + console.error( + 'Returning an Iterator from a Server Component is not supported ' + + 'since it cannot be looped over more than once. ', + ); + }); } } } @@ -1277,10 +1325,12 @@ function renderFunctionComponent( Object.prototype.toString.call(iterableChild) === '[object AsyncGenerator]'; if (!isGeneratorComponent) { - console.error( - 'Returning an AsyncIterator from a Server Component is not supported ' + - 'since it cannot be looped over more than once. ', - ); + callWithDebugContextInDEV(task, () => { + console.error( + 'Returning an AsyncIterator from a Server Component is not supported ' + + 'since it cannot be looped over more than once. ', + ); + }); } } } @@ -1743,30 +1793,34 @@ function createTask( originalValue !== value && !(originalValue instanceof Date) ) { - if (objectName(originalValue) !== 'Object') { - const jsxParentType = jsxChildrenParents.get(parent); - if (typeof jsxParentType === 'string') { - console.error( - '%s objects cannot be rendered as text children. Try formatting it using toString().%s', - objectName(originalValue), - describeObjectForErrorMessage(parent, parentPropertyName), - ); + // Call with the server component as the currently rendering component + // for context. + callWithDebugContextInDEV(task, () => { + if (objectName(originalValue) !== 'Object') { + const jsxParentType = jsxChildrenParents.get(parent); + if (typeof jsxParentType === 'string') { + console.error( + '%s objects cannot be rendered as text children. Try formatting it using toString().%s', + objectName(originalValue), + describeObjectForErrorMessage(parent, parentPropertyName), + ); + } else { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + '%s objects are not supported.%s', + objectName(originalValue), + describeObjectForErrorMessage(parent, parentPropertyName), + ); + } } else { console.error( 'Only plain objects can be passed to Client Components from Server Components. ' + - '%s objects are not supported.%s', - objectName(originalValue), + 'Objects with toJSON methods are not supported. Convert it manually ' + + 'to a simple value before passing it to props.%s', describeObjectForErrorMessage(parent, parentPropertyName), ); } - } else { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Objects with toJSON methods are not supported. Convert it manually ' + - 'to a simple value before passing it to props.%s', - describeObjectForErrorMessage(parent, parentPropertyName), - ); - } + }); } } return renderModel(request, task, parent, parentPropertyName, value); @@ -1900,7 +1954,7 @@ function serializeClientReference( } catch (x) { request.pendingChunks++; const errorId = request.nextChunkId++; - const digest = logRecoverableError(request, x); + const digest = logRecoverableError(request, x, null); emitErrorChunk(request, errorId, digest, x); return serializeByValueID(errorId); } @@ -2041,7 +2095,7 @@ function serializeBlob(request: Request, blob: Blob): string { } aborted = true; request.abortListeners.delete(error); - const digest = logRecoverableError(request, reason); + const digest = logRecoverableError(request, reason, newTask); emitErrorChunk(request, newTask.id, digest, reason); request.abortableTasks.delete(newTask); enqueueFlush(request); @@ -2143,7 +2197,7 @@ function renderModel( const postponeInstance: Postpone = (x: any); request.pendingChunks++; const postponeId = request.nextChunkId++; - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, task); emitPostponeChunk(request, postponeId, postponeInstance); // Restore the context. We assume that this will be restored by the inner @@ -2175,7 +2229,7 @@ function renderModel( // Something errored. We'll still send everything we have up until this point. request.pendingChunks++; const errorId = request.nextChunkId++; - const digest = logRecoverableError(request, x); + const digest = logRecoverableError(request, x, task); emitErrorChunk(request, errorId, digest, x); if (wasReactNode) { // We'll replace this element with a lazy reference that throws on the client @@ -2608,27 +2662,33 @@ function renderModelDestructive( } if (objectName(value) !== 'Object') { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - '%s objects are not supported.%s', - objectName(value), - describeObjectForErrorMessage(parent, parentPropertyName), - ); + callWithDebugContextInDEV(task, () => { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + '%s objects are not supported.%s', + objectName(value), + describeObjectForErrorMessage(parent, parentPropertyName), + ); + }); } else if (!isSimpleObject(value)) { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Classes or other objects with methods are not supported.%s', - describeObjectForErrorMessage(parent, parentPropertyName), - ); - } else if (Object.getOwnPropertySymbols) { - const symbols = Object.getOwnPropertySymbols(value); - if (symbols.length > 0) { + callWithDebugContextInDEV(task, () => { console.error( 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Objects with symbol properties like %s are not supported.%s', - symbols[0].description, + 'Classes or other objects with methods are not supported.%s', describeObjectForErrorMessage(parent, parentPropertyName), ); + }); + } else if (Object.getOwnPropertySymbols) { + const symbols = Object.getOwnPropertySymbols(value); + if (symbols.length > 0) { + callWithDebugContextInDEV(task, () => { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + 'Objects with symbol properties like %s are not supported.%s', + symbols[0].description, + describeObjectForErrorMessage(parent, parentPropertyName), + ); + }); } } } @@ -2784,12 +2844,18 @@ function renderModelDestructive( ); } -function logPostpone(request: Request, reason: string): void { +function logPostpone( + request: Request, + reason: string, + task: Task | null, // DEV-only +): void { const prevRequest = currentRequest; currentRequest = null; try { const onPostpone = request.onPostpone; - if (supportsRequestStorage) { + if (__DEV__ && task !== null) { + callWithDebugContextInDEV(task, onPostpone, reason); + } else if (supportsRequestStorage) { // Exit the request context while running callbacks. requestStorage.run(undefined, onPostpone, reason); } else { @@ -2800,13 +2866,19 @@ function logPostpone(request: Request, reason: string): void { } } -function logRecoverableError(request: Request, error: mixed): string { +function logRecoverableError( + request: Request, + error: mixed, + task: Task | null, // DEV-only +): string { const prevRequest = currentRequest; currentRequest = null; let errorDigest; try { const onError = request.onError; - if (supportsRequestStorage) { + if (__DEV__ && task !== null) { + errorDigest = callWithDebugContextInDEV(task, onError, error); + } else if (supportsRequestStorage) { // Exit the request context while running callbacks. errorDigest = requestStorage.run(undefined, onError, error); } else { @@ -3602,7 +3674,7 @@ function retryTask(request: Request, task: Task): void { request.abortableTasks.delete(task); task.status = ERRORED; const postponeInstance: Postpone = (x: any); - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, task); emitPostponeChunk(request, task.id, postponeInstance); return; } @@ -3619,7 +3691,7 @@ function retryTask(request: Request, task: Task): void { request.abortableTasks.delete(task); task.status = ERRORED; - const digest = logRecoverableError(request, x); + const digest = logRecoverableError(request, x, task); emitErrorChunk(request, task.id, digest, x); } finally { if (__DEV__) { @@ -3664,7 +3736,7 @@ function performWork(request: Request): void { flushCompletedChunks(request, request.destination); } } catch (error) { - logRecoverableError(request, error); + logRecoverableError(request, error, null); fatalError(request, error); } finally { ReactSharedInternals.H = prevDispatcher; @@ -3815,7 +3887,7 @@ export function startFlowing(request: Request, destination: Destination): void { try { flushCompletedChunks(request, destination); } catch (error) { - logRecoverableError(request, error); + logRecoverableError(request, error, null); fatalError(request, error); } } @@ -3842,7 +3914,7 @@ export function abort(request: Request, reason: mixed): void { (reason: any).$$typeof === REACT_POSTPONE_TYPE ) { const postponeInstance: Postpone = (reason: any); - logPostpone(request, postponeInstance.message); + logPostpone(request, postponeInstance.message, null); emitPostponeChunk(request, errorId, postponeInstance); } else { const error = @@ -3855,7 +3927,7 @@ export function abort(request: Request, reason: mixed): void { typeof reason.then === 'function' ? new Error('The render was aborted by the server with a promise.') : reason; - const digest = logRecoverableError(request, error); + const digest = logRecoverableError(request, error, null); emitErrorChunk(request, errorId, digest, error); } abortableTasks.forEach(task => abortTask(task, request, errorId)); @@ -3893,7 +3965,7 @@ export function abort(request: Request, reason: mixed): void { flushCompletedChunks(request, request.destination); } } catch (error) { - logRecoverableError(request, error); + logRecoverableError(request, error, null); fatalError(request, error); } } From e2e328c408713fa99735d26ce177bbbf214bad81 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 3 Jul 2024 19:36:13 -0400 Subject: [PATCH 3/3] Only clear requestStorage if we're in onError/onPostpone We only clear these to avoid replaying logs from onError on the client. This doesn't make a difference because we're not clearing currentRequest for console.error but it should line up. --- .../react-server/src/ReactFlightServer.js | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 3decabdf058d1..30a8ece0ad4ec 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1121,23 +1121,8 @@ function callWithDebugContextInDEV( setCurrentOwner(componentDebugInfo); try { if (enableOwnerStacks && debugTask) { - if (supportsRequestStorage) { - // Exit the request context while running callbacks. - return debugTask.run( - // $FlowFixMe[method-unbinding] - requestStorage.run.bind( - requestStorage, - undefined, - callback.bind(null, arg), - ), - ); - } return debugTask.run(callback.bind(null, arg)); } - if (supportsRequestStorage) { - // Exit the request context while running callbacks. - return requestStorage.run(undefined, callback, arg); - } return callback(arg); } finally { setCurrentOwner(null); @@ -2850,11 +2835,23 @@ function logPostpone( task: Task | null, // DEV-only ): void { const prevRequest = currentRequest; + // We clear the request context so that console.logs inside the callback doesn't + // get forwarded to the client. currentRequest = null; try { const onPostpone = request.onPostpone; if (__DEV__ && task !== null) { - callWithDebugContextInDEV(task, onPostpone, reason); + if (supportsRequestStorage) { + requestStorage.run( + undefined, + callWithDebugContextInDEV, + task, + onPostpone, + reason, + ); + } else { + callWithDebugContextInDEV(task, onPostpone, reason); + } } else if (supportsRequestStorage) { // Exit the request context while running callbacks. requestStorage.run(undefined, onPostpone, reason); @@ -2872,12 +2869,24 @@ function logRecoverableError( task: Task | null, // DEV-only ): string { const prevRequest = currentRequest; + // We clear the request context so that console.logs inside the callback doesn't + // get forwarded to the client. currentRequest = null; let errorDigest; try { const onError = request.onError; if (__DEV__ && task !== null) { - errorDigest = callWithDebugContextInDEV(task, onError, error); + if (supportsRequestStorage) { + errorDigest = requestStorage.run( + undefined, + callWithDebugContextInDEV, + task, + onError, + error, + ); + } else { + errorDigest = callWithDebugContextInDEV(task, onError, error); + } } else if (supportsRequestStorage) { // Exit the request context while running callbacks. errorDigest = requestStorage.run(undefined, onError, error);