Skip to content

Commit

Permalink
Track separate SuspendedOnAction flag by rethrowing a separate Suspen…
Browse files Browse the repository at this point in the history
…seActionException sentinel (#31554)

This lets us track separately if something was suspended on an Action
using useActionState rather than suspended on Data.

This approach feels quite bloated and it seems like we'd eventually
might want to read more information about the Promise that suspended and
the context it suspended in. As a more general reason for suspending.

The way useActionState works in combination with the prewarming is quite
unfortunate because 1) it renders blocking to update the isPending flag
whether you use it or not 2) it prewarms and suspends the useActionState
3) then it does another third render to get back into the useActionState
position again.
  • Loading branch information
sebmarkbage authored Nov 15, 2024
1 parent 053b3cb commit 92c0f5f
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(usable: Usable<T>): T {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {getIsHydrating} from './ReactFiberHydrationContext';
import {pushTreeFork} from './ReactFiberTreeContext';
import {
SuspenseException,
SuspenseActionException,
createThenableState,
trackUsedThenable,
} from './ReactFiberThenable';
Expand Down Expand Up @@ -1950,6 +1951,7 @@ function createChildReconciler(
} catch (x) {
if (
x === SuspenseException ||
x === SuspenseActionException ||
(!disableLegacyMode &&
(returnFiber.mode & ConcurrentMode) === NoMode &&
typeof x === 'object' &&
Expand Down
22 changes: 19 additions & 3 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ import {
trackUsedThenable,
checkIfUseWrappedInTryCatch,
createThenableState,
SuspenseException,
SuspenseActionException,
} from './ReactFiberThenable';
import type {ThenableState} from './ReactFiberThenable';
import type {BatchConfigTransition} from './ReactFiberTracingMarkerComponent';
Expand Down Expand Up @@ -2433,13 +2435,27 @@ function updateActionStateImpl<S, P>(
const [isPending] = updateState(false);

// This will suspend until the action finishes.
const state: Awaited<S> =
let state: Awaited<S>;
if (
typeof actionResult === 'object' &&
actionResult !== null &&
// $FlowFixMe[method-unbinding]
typeof actionResult.then === 'function'
? useThenable(((actionResult: any): Thenable<Awaited<S>>))
: (actionResult: any);
) {
try {
state = useThenable(((actionResult: any): Thenable<Awaited<S>>));
} 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;
Expand Down
15 changes: 13 additions & 2 deletions packages/react-reconciler/src/ReactFiberThenable.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,22 @@ 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(
'Suspense Exception: This is not a real error, and should not leak into ' +
"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
Expand Down Expand Up @@ -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'` " +
Expand Down
30 changes: 23 additions & 7 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ import {
import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent';
import {
SuspenseException,
SuspenseActionException,
SuspenseyCommitException,
getSuspendedThenable,
isThenableResolved,
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -638,7 +640,10 @@ export function getWorkInProgressRootRenderLanes(): Lanes {
}

export function isWorkLoopSuspendedOnData(): boolean {
return workInProgressSuspendedReason === SuspendedOnData;
return (
workInProgressSuspendedReason === SuspendedOnData ||
workInProgressSuspendedReason === SuspendedOnAction
);
}

export function getCurrentTime(): number {
Expand Down Expand Up @@ -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
) {
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -1903,6 +1914,7 @@ function handleThrow(root: FiberRoot, thrownValue: any): void {
break;
}
case SuspendedOnData:
case SuspendedOnAction:
case SuspendedOnImmediate:
case SuspendedOnDeprecatedThrowPromise:
case SuspendedAndReadyToContinue: {
Expand Down Expand Up @@ -2185,6 +2197,7 @@ function renderRootSync(
}
case SuspendedOnImmediate:
case SuspendedOnData:
case SuspendedOnAction:
case SuspendedOnDeprecatedThrowPromise: {
if (getSuspenseHandler() === null) {
didSuspendInShell = true;
Expand Down Expand Up @@ -2348,7 +2361,8 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
);
break;
}
case SuspendedOnData: {
case SuspendedOnData:
case SuspendedOnAction: {
const thenable: Thenable<mixed> = (thrownValue: any);
if (isThenableResolved(thenable)) {
// The data resolved. Try rendering the component again.
Expand All @@ -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.
Expand Down Expand Up @@ -2814,6 +2829,7 @@ function throwAndUnwindWorkLoop(
// can prerender the siblings.
if (
suspendedReason === SuspendedOnData ||
suspendedReason === SuspendedOnAction ||
suspendedReason === SuspendedOnImmediate ||
suspendedReason === SuspendedOnDeprecatedThrowPromise
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-server/src/ReactFizzThenable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-server/src/ReactFlightThenable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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."
}

0 comments on commit 92c0f5f

Please sign in to comment.