From 404fb64cf1d34184809e30e4dd9e1044dbdf567d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 15 Nov 2024 15:36:56 -0500 Subject: [PATCH] Track separate SuspendedOnAction flag by rethrowing a separate SuspenseActionException sentinel This lets us track separately if something was suspended on an Action using useActionState. --- .../react-debug-tools/src/ReactDebugHooks.js | 2 +- .../react-reconciler/src/ReactChildFiber.js | 2 ++ .../react-reconciler/src/ReactFiberHooks.js | 22 ++++++++++++-- .../src/ReactFiberThenable.js | 15 ++++++++-- .../src/ReactFiberWorkLoop.js | 30 ++++++++++++++----- .../react-server/src/ReactFizzThenable.js | 2 +- .../react-server/src/ReactFlightThenable.js | 2 +- scripts/error-codes/codes.json | 5 ++-- 8 files changed, 63 insertions(+), 17 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index b1e7a0272acea..2a0c57154e6f9 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -214,7 +214,7 @@ const SuspenseException: mixed = new Error( '`try/catch` block. Capturing without rethrowing will lead to ' + 'unexpected behavior.\n\n' + 'To handle async errors, wrap your component in an error boundary, or ' + - "call the promise's `.catch` method and pass the result to `use`", + "call the promise's `.catch` method and pass the result to `use`.", ); function use(usable: Usable): T { diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 96512d9dce282..6613c8d43bba0 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -64,6 +64,7 @@ import {getIsHydrating} from './ReactFiberHydrationContext'; import {pushTreeFork} from './ReactFiberTreeContext'; import { SuspenseException, + SuspenseActionException, createThenableState, trackUsedThenable, } from './ReactFiberThenable'; @@ -1950,6 +1951,7 @@ function createChildReconciler( } catch (x) { if ( x === SuspenseException || + x === SuspenseActionException || (!disableLegacyMode && (returnFiber.mode & ConcurrentMode) === NoMode && typeof x === 'object' && diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 174235fc76d65..461743ed63921 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -149,6 +149,8 @@ import { trackUsedThenable, checkIfUseWrappedInTryCatch, createThenableState, + SuspenseException, + SuspenseActionException, } from './ReactFiberThenable'; import type {ThenableState} from './ReactFiberThenable'; import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent'; @@ -2432,13 +2434,27 @@ function updateActionStateImpl( const [isPending] = updateState(false); // This will suspend until the action finishes. - const state: Awaited = + let state: Awaited; + if ( typeof actionResult === 'object' && actionResult !== null && // $FlowFixMe[method-unbinding] typeof actionResult.then === 'function' - ? useThenable(((actionResult: any): Thenable>)) - : (actionResult: any); + ) { + try { + state = useThenable(((actionResult: any): Thenable>)); + } catch (x) { + if (x === SuspenseException) { + // If we Suspend here, mark this separately so that we can track this + // as an Action in Profiling tools. + throw SuspenseActionException; + } else { + throw x; + } + } + } else { + state = (actionResult: any); + } const actionQueueHook = updateWorkInProgressHook(); const actionQueue = actionQueueHook.queue; diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 2f0ac5637afba..88d32dc530790 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -46,7 +46,7 @@ export const SuspenseException: mixed = new Error( '`try/catch` block. Capturing without rethrowing will lead to ' + 'unexpected behavior.\n\n' + 'To handle async errors, wrap your component in an error boundary, or ' + - "call the promise's `.catch` method and pass the result to `use`", + "call the promise's `.catch` method and pass the result to `use`.", ); export const SuspenseyCommitException: mixed = new Error( @@ -54,6 +54,14 @@ export const SuspenseyCommitException: mixed = new Error( "userspace. If you're seeing this, it's likely a bug in React.", ); +export const SuspenseActionException: mixed = new Error( + "Suspense Exception: This is not a real error! It's an implementation " + + 'detail of `useActionState` to interrupt the current render. You must either ' + + 'rethrow it immediately, or move the `useActionState` call outside of the ' + + '`try/catch` block. Capturing without rethrowing will lead to ' + + 'unexpected behavior.\n\n' + + 'To handle async errors, wrap your component in an error boundary.', +); // This is a noop thenable that we use to trigger a fallback in throwException. // TODO: It would be better to refactor throwException into multiple functions // so we can trigger a fallback directly without having to check the type. But @@ -296,7 +304,10 @@ export function checkIfUseWrappedInAsyncCatch(rejectedReason: any) { // execution context is to check the dispatcher every time `use` is called, // or some equivalent. That might be preferable for other reasons, too, since // it matches how we prevent similar mistakes for other hooks. - if (rejectedReason === SuspenseException) { + if ( + rejectedReason === SuspenseException || + rejectedReason === SuspenseActionException + ) { throw new Error( 'Hooks are not supported inside an async component. This ' + "error is often caused by accidentally adding `'use client'` " + diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 7a5409a73559a..d6f2c48f8ce11 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -298,6 +298,7 @@ import { import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent'; import { SuspenseException, + SuspenseActionException, SuspenseyCommitException, getSuspendedThenable, isThenableResolved, @@ -346,7 +347,7 @@ let workInProgress: Fiber | null = null; // The lanes we're rendering let workInProgressRootRenderLanes: Lanes = NoLanes; -opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8; +opaque type SuspendedReason = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9; const NotSuspended: SuspendedReason = 0; const SuspendedOnError: SuspendedReason = 1; const SuspendedOnData: SuspendedReason = 2; @@ -356,6 +357,7 @@ const SuspendedOnInstanceAndReadyToContinue: SuspendedReason = 5; const SuspendedOnDeprecatedThrowPromise: SuspendedReason = 6; const SuspendedAndReadyToContinue: SuspendedReason = 7; const SuspendedOnHydration: SuspendedReason = 8; +const SuspendedOnAction: SuspendedReason = 9; // When this is true, the work-in-progress fiber just suspended (or errored) and // we've yet to unwind the stack. In some cases, we may yield to the main thread @@ -638,7 +640,10 @@ export function getWorkInProgressRootRenderLanes(): Lanes { } export function isWorkLoopSuspendedOnData(): boolean { - return workInProgressSuspendedReason === SuspendedOnData; + return ( + workInProgressSuspendedReason === SuspendedOnData || + workInProgressSuspendedReason === SuspendedOnAction + ); } export function getCurrentTime(): number { @@ -767,7 +772,8 @@ export function scheduleUpdateOnFiber( if ( // Suspended render phase (root === workInProgressRoot && - workInProgressSuspendedReason === SuspendedOnData) || + (workInProgressSuspendedReason === SuspendedOnData || + workInProgressSuspendedReason === SuspendedOnAction)) || // Suspended commit phase root.cancelPendingCommit !== null ) { @@ -1815,7 +1821,10 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { resetCurrentFiber(); } - if (thrownValue === SuspenseException) { + if ( + thrownValue === SuspenseException || + thrownValue === SuspenseActionException + ) { // This is a special type of exception used for Suspense. For historical // reasons, the rest of the Suspense implementation expects the thrown value // to be a thenable, because before `use` existed that was the (unstable) @@ -1836,7 +1845,9 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { !includesNonIdleWork(workInProgressRootSkippedLanes) && !includesNonIdleWork(workInProgressRootInterleavedUpdatedLanes) ? // Suspend work loop until data resolves - SuspendedOnData + thrownValue === SuspenseActionException + ? SuspendedOnAction + : SuspendedOnData : // Don't suspend work loop, except to check if the data has // immediately resolved (i.e. in a microtask). Otherwise, trigger the // nearest Suspense fallback. @@ -1903,6 +1914,7 @@ function handleThrow(root: FiberRoot, thrownValue: any): void { break; } case SuspendedOnData: + case SuspendedOnAction: case SuspendedOnImmediate: case SuspendedOnDeprecatedThrowPromise: case SuspendedAndReadyToContinue: { @@ -2185,6 +2197,7 @@ function renderRootSync( } case SuspendedOnImmediate: case SuspendedOnData: + case SuspendedOnAction: case SuspendedOnDeprecatedThrowPromise: { if (getSuspenseHandler() === null) { didSuspendInShell = true; @@ -2348,7 +2361,8 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { ); break; } - case SuspendedOnData: { + case SuspendedOnData: + case SuspendedOnAction: { const thenable: Thenable = (thrownValue: any); if (isThenableResolved(thenable)) { // The data resolved. Try rendering the component again. @@ -2366,7 +2380,8 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { const onResolution = () => { // Check if the root is still suspended on this promise. if ( - workInProgressSuspendedReason === SuspendedOnData && + (workInProgressSuspendedReason === SuspendedOnData || + workInProgressSuspendedReason === SuspendedOnAction) && workInProgressRoot === root ) { // Mark the root as ready to continue rendering. @@ -2814,6 +2829,7 @@ function throwAndUnwindWorkLoop( // can prerender the siblings. if ( suspendedReason === SuspendedOnData || + suspendedReason === SuspendedOnAction || suspendedReason === SuspendedOnImmediate || suspendedReason === SuspendedOnDeprecatedThrowPromise ) { diff --git a/packages/react-server/src/ReactFizzThenable.js b/packages/react-server/src/ReactFizzThenable.js index 60117a6f5265b..f686b63d44db9 100644 --- a/packages/react-server/src/ReactFizzThenable.js +++ b/packages/react-server/src/ReactFizzThenable.js @@ -31,7 +31,7 @@ export const SuspenseException: mixed = new Error( '`try/catch` block. Capturing without rethrowing will lead to ' + 'unexpected behavior.\n\n' + 'To handle async errors, wrap your component in an error boundary, or ' + - "call the promise's `.catch` method and pass the result to `use`", + "call the promise's `.catch` method and pass the result to `use`.", ); export function createThenableState(): ThenableState { diff --git a/packages/react-server/src/ReactFlightThenable.js b/packages/react-server/src/ReactFlightThenable.js index d4b1f27395299..89912a7dd5d20 100644 --- a/packages/react-server/src/ReactFlightThenable.js +++ b/packages/react-server/src/ReactFlightThenable.js @@ -31,7 +31,7 @@ export const SuspenseException: mixed = new Error( '`try/catch` block. Capturing without rethrowing will lead to ' + 'unexpected behavior.\n\n' + 'To handle async errors, wrap your component in an error boundary, or ' + - "call the promise's `.catch` method and pass the result to `use`", + "call the promise's `.catch` method and pass the result to `use`.", ); export function createThenableState(): ThenableState { diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 572ffd21fd8b5..48b6ae91a1dc2 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -445,7 +445,7 @@ "457": "acquireHeadResource encountered a resource type it did not expect: \"%s\". This is a bug in React.", "458": "Currently React only supports one RSC renderer at a time.", "459": "Expected a suspended thenable. This is a bug in React. Please file an issue.", - "460": "Suspense Exception: This is not a real error! It's an implementation detail of `use` to interrupt the current render. You must either rethrow it immediately, or move the `use` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary, or call the promise's `.catch` method and pass the result to `use`", + "460": "Suspense Exception: This is not a real error! It's an implementation detail of `use` to interrupt the current render. You must either rethrow it immediately, or move the `use` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary, or call the promise's `.catch` method and pass the result to `use`.", "461": "This is not a real error. It's an implementation detail of React's selective hydration feature. If this leaks into userspace, it's a bug in React. Please file an issue.", "462": "Unexpected SuspendedReason. This is a bug in React.", "463": "ReactDOMServer.renderToNodeStream(): The Node Stream API is not available in Bun. Use ReactDOMServer.renderToReadableStream() instead.", @@ -526,5 +526,6 @@ "538": "Cannot use state or effect Hooks in renderToHTML because this component will never be hydrated.", "539": "Binary RSC chunks cannot be encoded as strings. This is a bug in the wiring of the React streams.", "540": "String chunks need to be passed in their original shape. Not split into smaller string chunks. This is a bug in the wiring of the React streams.", - "541": "Compared context values must be arrays" + "541": "Compared context values must be arrays", + "542": "Suspense Exception: This is not a real error! It's an implementation detail of `useActionState` to interrupt the current render. You must either rethrow it immediately, or move the `useActionState` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary." }