Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track separate SuspendedOnAction flag by rethrowing a separate SuspenseActionException sentinel #31554

Merged
merged 1 commit into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -2432,13 +2434,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;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the magic happens.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah meh this is fine 😆

} 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."
}
Loading