From da38d043eaf23b4f017aeff7e989326b4e0f9472 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 22 Oct 2022 23:50:24 -0400 Subject: [PATCH] Throw custom error object to suspend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old (unstable) mechanism for suspending was to throw a promise. The purpose of throwing is to interrupt the component's execution, and also to signal to React that the interruption was caused by Suspense as opposed to some other error. A flaw is that throwing is meant to be an implementation details — if code in userspace catches the promise, it can lead to unexpected behavior. With `use`, userspace code does not throw promises directly, but `use` itself still needs to throw something to interrupt the component and unwind the stack. The solution is to throw an internal error. In development, we can detect whether the error was caught by a userspace try/catch block and log a warning — though it's not foolproof, since a clever user could catch the object and rethrow it later. The error message includes advice to move `use` outside of the try/catch block. I did not yet implement the warning in Flight. --- .../src/ReactFiberHooks.new.js | 19 +++++ .../src/ReactFiberHooks.old.js | 19 +++++ .../src/ReactFiberThenable.new.js | 83 ++++++++++++++----- .../src/ReactFiberThenable.old.js | 83 ++++++++++++++----- .../src/ReactFiberWorkLoop.new.js | 19 ++++- .../src/ReactFiberWorkLoop.old.js | 19 ++++- .../src/__tests__/ReactThenable-test.js | 36 ++++++++ packages/react-server/src/ReactFizzServer.js | 35 +++++++- .../react-server/src/ReactFizzThenable.js | 47 +++++++++-- .../react-server/src/ReactFlightServer.js | 25 +++++- .../react-server/src/ReactFlightThenable.js | 47 +++++++++-- scripts/error-codes/codes.json | 4 +- 12 files changed, 373 insertions(+), 63 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 762fb6a01ea88..c99c9a3407ce1 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -138,6 +138,7 @@ import {now} from './Scheduler'; import { prepareThenableState, trackUsedThenable, + checkIfUseWrappedInTryCatch, } from './ReactFiberThenable.new'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -160,8 +161,10 @@ export type UpdateQueue = { let didWarnAboutMismatchedHooksForComponent; let didWarnUncachedGetSnapshot; +let didWarnAboutUseWrappedInTryCatch; if (__DEV__) { didWarnAboutMismatchedHooksForComponent = new Set(); + didWarnAboutUseWrappedInTryCatch = new Set(); } export type Hook = { @@ -594,6 +597,22 @@ export function renderWithHooks( } } } + + if (__DEV__) { + if (checkIfUseWrappedInTryCatch()) { + const componentName = + getComponentNameFromFiber(workInProgress) || 'Unknown'; + if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) { + didWarnAboutUseWrappedInTryCatch.add(componentName); + console.error( + '`use` was called from inside a try/catch block. This is not allowed ' + + 'and can lead to unexpected behavior. To handle errors triggered ' + + 'by `use`, wrap your component in a error boundary.', + ); + } + } + } + return children; } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 974f0ef76b373..38ce674d4c273 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -138,6 +138,7 @@ import {now} from './Scheduler'; import { prepareThenableState, trackUsedThenable, + checkIfUseWrappedInTryCatch, } from './ReactFiberThenable.old'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -160,8 +161,10 @@ export type UpdateQueue = { let didWarnAboutMismatchedHooksForComponent; let didWarnUncachedGetSnapshot; +let didWarnAboutUseWrappedInTryCatch; if (__DEV__) { didWarnAboutMismatchedHooksForComponent = new Set(); + didWarnAboutUseWrappedInTryCatch = new Set(); } export type Hook = { @@ -594,6 +597,22 @@ export function renderWithHooks( } } } + + if (__DEV__) { + if (checkIfUseWrappedInTryCatch()) { + const componentName = + getComponentNameFromFiber(workInProgress) || 'Unknown'; + if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) { + didWarnAboutUseWrappedInTryCatch.add(componentName); + console.error( + '`use` was called from inside a try/catch block. This is not allowed ' + + 'and can lead to unexpected behavior. To handle errors triggered ' + + 'by `use`, wrap your component in a error boundary.', + ); + } + } + } + return children; } diff --git a/packages/react-reconciler/src/ReactFiberThenable.new.js b/packages/react-reconciler/src/ReactFiberThenable.new.js index c2de159f89ca9..d10c4d37e9ac2 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.new.js +++ b/packages/react-reconciler/src/ReactFiberThenable.new.js @@ -19,6 +19,18 @@ const {ReactCurrentActQueue} = ReactSharedInternals; export opaque type ThenableState = Array>; +// An error that is thrown (e.g. by `use`) to trigger Suspense. If we +// detect this is caught by userspace, we'll log a warning in development. +export const SuspenseException: mixed = new Error( + "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\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`", +); + let thenableState: ThenableState | null = null; export function createThenableState(): ThenableState { @@ -37,19 +49,9 @@ export function prepareThenableState(prevThenableState: ThenableState | null) { export function getThenableStateAfterSuspending(): ThenableState | null { // Called by the work loop so it can stash the thenable state. It will use // the state to replay the component when the promise resolves. - if ( - thenableState !== null && - // If we only `use`-ed resolved promises, then there is no suspended state - // TODO: The only reason we do this is to distinguish between throwing a - // promise (old Suspense pattern) versus `use`-ing one. A better solution is - // for `use` to throw a special, opaque value instead of a promise. - !isThenableStateResolved(thenableState) - ) { - const state = thenableState; - thenableState = null; - return state; - } - return null; + const state = thenableState; + thenableState = null; + return state; } export function isThenableStateResolved(thenables: ThenableState): boolean { @@ -129,13 +131,54 @@ export function trackUsedThenable(thenable: Thenable, index: number): T { } // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; + // + // Throwing here is an implementation detail that allows us to unwind the + // call stack. But we shouldn't allow it to leak into userspace. Throw an + // opaque placeholder value instead of the actual thenable. If it doesn't + // get captured by the work loop, log a warning, because that means + // something in userspace must have caught it. + suspendedThenable = thenable; + if (__DEV__) { + needsToResetSuspendedThenableDEV = true; + } + throw SuspenseException; + } + } +} + +// This is used to track the actual thenable that suspended so it can be +// passed to the rest of the Suspense implementation — which, for historical +// reasons, expects to receive a thenable. +let suspendedThenable: Thenable | null = null; +let needsToResetSuspendedThenableDEV = false; +export function getSuspendedThenable(): Thenable { + // This is called right after `use` suspends by throwing an exception. `use` + // throws an opaque value instead of the thenable itself so that it can't be + // caught in userspace. Then the work loop accesses the actual thenable using + // this function. + if (suspendedThenable === null) { + throw new Error( + 'Expected a suspended thenable. This is a bug in React. Please file ' + + 'an issue.', + ); + } + const thenable = suspendedThenable; + suspendedThenable = null; + if (__DEV__) { + needsToResetSuspendedThenableDEV = false; + } + return thenable; +} + +export function checkIfUseWrappedInTryCatch(): boolean { + if (__DEV__) { + // This was set right before SuspenseException was thrown, and it should + // have been cleared when the exception was handled. If it wasn't, + // it must have been caught by userspace. + if (needsToResetSuspendedThenableDEV) { + needsToResetSuspendedThenableDEV = false; + return true; } } + return false; } diff --git a/packages/react-reconciler/src/ReactFiberThenable.old.js b/packages/react-reconciler/src/ReactFiberThenable.old.js index c2de159f89ca9..d10c4d37e9ac2 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.old.js +++ b/packages/react-reconciler/src/ReactFiberThenable.old.js @@ -19,6 +19,18 @@ const {ReactCurrentActQueue} = ReactSharedInternals; export opaque type ThenableState = Array>; +// An error that is thrown (e.g. by `use`) to trigger Suspense. If we +// detect this is caught by userspace, we'll log a warning in development. +export const SuspenseException: mixed = new Error( + "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\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`", +); + let thenableState: ThenableState | null = null; export function createThenableState(): ThenableState { @@ -37,19 +49,9 @@ export function prepareThenableState(prevThenableState: ThenableState | null) { export function getThenableStateAfterSuspending(): ThenableState | null { // Called by the work loop so it can stash the thenable state. It will use // the state to replay the component when the promise resolves. - if ( - thenableState !== null && - // If we only `use`-ed resolved promises, then there is no suspended state - // TODO: The only reason we do this is to distinguish between throwing a - // promise (old Suspense pattern) versus `use`-ing one. A better solution is - // for `use` to throw a special, opaque value instead of a promise. - !isThenableStateResolved(thenableState) - ) { - const state = thenableState; - thenableState = null; - return state; - } - return null; + const state = thenableState; + thenableState = null; + return state; } export function isThenableStateResolved(thenables: ThenableState): boolean { @@ -129,13 +131,54 @@ export function trackUsedThenable(thenable: Thenable, index: number): T { } // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; + // + // Throwing here is an implementation detail that allows us to unwind the + // call stack. But we shouldn't allow it to leak into userspace. Throw an + // opaque placeholder value instead of the actual thenable. If it doesn't + // get captured by the work loop, log a warning, because that means + // something in userspace must have caught it. + suspendedThenable = thenable; + if (__DEV__) { + needsToResetSuspendedThenableDEV = true; + } + throw SuspenseException; + } + } +} + +// This is used to track the actual thenable that suspended so it can be +// passed to the rest of the Suspense implementation — which, for historical +// reasons, expects to receive a thenable. +let suspendedThenable: Thenable | null = null; +let needsToResetSuspendedThenableDEV = false; +export function getSuspendedThenable(): Thenable { + // This is called right after `use` suspends by throwing an exception. `use` + // throws an opaque value instead of the thenable itself so that it can't be + // caught in userspace. Then the work loop accesses the actual thenable using + // this function. + if (suspendedThenable === null) { + throw new Error( + 'Expected a suspended thenable. This is a bug in React. Please file ' + + 'an issue.', + ); + } + const thenable = suspendedThenable; + suspendedThenable = null; + if (__DEV__) { + needsToResetSuspendedThenableDEV = false; + } + return thenable; +} + +export function checkIfUseWrappedInTryCatch(): boolean { + if (__DEV__) { + // This was set right before SuspenseException was thrown, and it should + // have been cleared when the exception was handled. If it wasn't, + // it must have been caught by userspace. + if (needsToResetSuspendedThenableDEV) { + needsToResetSuspendedThenableDEV = false; + return true; } } + return false; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index d94060cdac43c..80ab4cb37357a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -266,6 +266,8 @@ import { } from './ReactFiberAct.new'; import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new'; import { + SuspenseException, + getSuspendedThenable, getThenableStateAfterSuspending, isThenableStateResolved, } from './ReactFiberThenable.new'; @@ -1722,13 +1724,26 @@ function handleThrow(root, thrownValue): void { // separate issue. Write a regression test using string refs. ReactCurrentOwner.current = null; + if (thrownValue === SuspenseException) { + // 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) + // API for suspending. This implementation detail can change later, once we + // deprecate the old API in favor of `use`. + thrownValue = getSuspendedThenable(); + workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); + } else { + // This is a regular error. If something earlier in the component already + // suspended, we must clear the thenable state to unblock the work loop. + workInProgressSuspendedThenableState = null; + } + // Setting this to `true` tells the work loop to unwind the stack instead // of entering the begin phase. It's called "suspended" because it usually // happens because of Suspense, but it also applies to errors. Think of it // as suspending the execution of the work loop. workInProgressIsSuspended = true; workInProgressThrownValue = thrownValue; - workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); const erroredWork = workInProgress; if (erroredWork === null) { @@ -1750,6 +1765,7 @@ function handleThrow(root, thrownValue): void { if ( thrownValue !== null && typeof thrownValue === 'object' && + // $FlowFixMe[method-unbinding] typeof thrownValue.then === 'function' ) { const wakeable: Wakeable = (thrownValue: any); @@ -3503,6 +3519,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { } catch (originalError) { if ( didSuspendOrErrorWhileHydratingDEV() || + originalError === SuspenseException || (originalError !== null && typeof originalError === 'object' && typeof originalError.then === 'function') diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index ae24f0f5af572..db7f8585246e6 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -266,6 +266,8 @@ import { } from './ReactFiberAct.old'; import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old'; import { + SuspenseException, + getSuspendedThenable, getThenableStateAfterSuspending, isThenableStateResolved, } from './ReactFiberThenable.old'; @@ -1722,13 +1724,26 @@ function handleThrow(root, thrownValue): void { // separate issue. Write a regression test using string refs. ReactCurrentOwner.current = null; + if (thrownValue === SuspenseException) { + // 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) + // API for suspending. This implementation detail can change later, once we + // deprecate the old API in favor of `use`. + thrownValue = getSuspendedThenable(); + workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); + } else { + // This is a regular error. If something earlier in the component already + // suspended, we must clear the thenable state to unblock the work loop. + workInProgressSuspendedThenableState = null; + } + // Setting this to `true` tells the work loop to unwind the stack instead // of entering the begin phase. It's called "suspended" because it usually // happens because of Suspense, but it also applies to errors. Think of it // as suspending the execution of the work loop. workInProgressIsSuspended = true; workInProgressThrownValue = thrownValue; - workInProgressSuspendedThenableState = getThenableStateAfterSuspending(); const erroredWork = workInProgress; if (erroredWork === null) { @@ -1750,6 +1765,7 @@ function handleThrow(root, thrownValue): void { if ( thrownValue !== null && typeof thrownValue === 'object' && + // $FlowFixMe[method-unbinding] typeof thrownValue.then === 'function' ) { const wakeable: Wakeable = (thrownValue: any); @@ -3503,6 +3519,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) { } catch (originalError) { if ( didSuspendOrErrorWhileHydratingDEV() || + originalError === SuspenseException || (originalError !== null && typeof originalError === 'object' && typeof originalError.then === 'function') diff --git a/packages/react-reconciler/src/__tests__/ReactThenable-test.js b/packages/react-reconciler/src/__tests__/ReactThenable-test.js index bc18fbf7632c1..fd95a43bfab8f 100644 --- a/packages/react-reconciler/src/__tests__/ReactThenable-test.js +++ b/packages/react-reconciler/src/__tests__/ReactThenable-test.js @@ -461,4 +461,40 @@ describe('ReactThenable', () => { expect(root).toMatchRenderedOutput(
Hello world!
); }); + + // @gate enableUseHook || !__DEV__ + test('warns if use(promise) is wrapped with try/catch block', async () => { + function Async() { + try { + return ; + } catch (e) { + return ; + } + } + + spyOnDev(console, 'error'); + function App() { + return ( + }> + + + ); + } + + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'Warning: `use` was called from inside a try/catch block. This is not ' + + 'allowed and can lead to unexpected behavior. To handle errors ' + + 'triggered by `use`, wrap your component in a error boundary.', + ); + } + }); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index aaf0fc2f5b33c..b96fce2984e98 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -17,6 +17,7 @@ import type { ReactContext, ReactProviderType, OffscreenMode, + Wakeable, } from 'shared/ReactTypes'; import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type { @@ -138,6 +139,7 @@ import { import assign from 'shared/assign'; import getComponentNameFromType from 'shared/getComponentNameFromType'; import isArray from 'shared/isArray'; +import {SuspenseException, getSuspendedThenable} from './ReactFizzThenable'; const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; const ReactCurrentCache = ReactSharedInternals.ReactCurrentCache; @@ -1522,7 +1524,7 @@ function spawnNewSuspendedTask( request: Request, task: Task, thenableState: ThenableState | null, - x: Promise, + x: Wakeable, ): void { // Something suspended, we'll need to create a new segment and resolve it later. const segment = task.blockedSegment; @@ -1580,11 +1582,24 @@ function renderNode(request: Request, task: Task, node: ReactNodeList): void { } try { return renderNodeDestructive(request, task, null, node); - } catch (x) { + } catch (thrownValue) { resetHooksState(); + + const x = + thrownValue === SuspenseException + ? // 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) API for suspending. This implementation detail can change + // later, once we deprecate the old API in favor of `use`. + getSuspendedThenable() + : thrownValue; + + // $FlowFixMe[method-unbinding] if (typeof x === 'object' && x !== null && typeof x.then === 'function') { + const wakeable: Wakeable = (x: any); const thenableState = getThenableStateAfterSuspending(); - spawnNewSuspendedTask(request, task, thenableState, x); + spawnNewSuspendedTask(request, task, thenableState, wakeable); // Restore the context. We assume that this will be restored by the inner // functions in case nothing throws so we don't use "finally" here. @@ -1869,8 +1884,20 @@ function retryTask(request: Request, task: Task): void { task.abortSet.delete(task); segment.status = COMPLETED; finishedTask(request, task.blockedBoundary, segment); - } catch (x) { + } catch (thrownValue) { resetHooksState(); + + const x = + thrownValue === SuspenseException + ? // 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) API for suspending. This implementation detail can change + // later, once we deprecate the old API in favor of `use`. + getSuspendedThenable() + : thrownValue; + + // $FlowFixMe[method-unbinding] if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended again, let's pick it back up later. const ping = task.ping; diff --git a/packages/react-server/src/ReactFizzThenable.js b/packages/react-server/src/ReactFizzThenable.js index 267ee6036041d..415ddc119c781 100644 --- a/packages/react-server/src/ReactFizzThenable.js +++ b/packages/react-server/src/ReactFizzThenable.js @@ -22,6 +22,18 @@ import type { export opaque type ThenableState = Array>; +// An error that is thrown (e.g. by `use`) to trigger Suspense. If we +// detect this is caught by userspace, we'll log a warning in development. +export const SuspenseException: mixed = new Error( + "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\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`", +); + export function createThenableState(): ThenableState { // The ThenableState is created the first time a component suspends. If it // suspends again, we'll reuse the same state. @@ -92,13 +104,34 @@ export function trackUsedThenable( } // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; + // + // Throwing here is an implementation detail that allows us to unwind the + // call stack. But we shouldn't allow it to leak into userspace. Throw an + // opaque placeholder value instead of the actual thenable. If it doesn't + // get captured by the work loop, log a warning, because that means + // something in userspace must have caught it. + suspendedThenable = thenable; + throw SuspenseException; } } } + +// This is used to track the actual thenable that suspended so it can be +// passed to the rest of the Suspense implementation — which, for historical +// reasons, expects to receive a thenable. +let suspendedThenable: Thenable | null = null; +export function getSuspendedThenable(): Thenable { + // This is called right after `use` suspends by throwing an exception. `use` + // throws an opaque value instead of the thenable itself so that it can't be + // caught in userspace. Then the work loop accesses the actual thenable using + // this function. + if (suspendedThenable === null) { + throw new Error( + 'Expected a suspended thenable. This is a bug in React. Please file ' + + 'an issue.', + ); + } + const thenable = suspendedThenable; + suspendedThenable = null; + return thenable; +} diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index b91b8db2a9694..575a19939f88f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -84,6 +84,7 @@ import { import {getOrCreateServerContext} from 'shared/ReactServerContextRegistry'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import isArray from 'shared/isArray'; +import {SuspenseException, getSuspendedThenable} from './ReactFlightThenable'; type ReactJSONValue = | string @@ -842,7 +843,17 @@ export function resolveModelToJSON( break; } } - } catch (x) { + } catch (thrownValue) { + const x = + thrownValue === SuspenseException + ? // 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) API for suspending. This implementation detail can change + // later, once we deprecate the old API in favor of `use`. + getSuspendedThenable() + : thrownValue; + // $FlowFixMe[method-unbinding] if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended, we'll need to create a new task and resolve it later. request.pendingChunks++; @@ -1173,7 +1184,17 @@ function retryTask(request: Request, task: Task): void { request.completedJSONChunks.push(processedChunk); request.abortableTasks.delete(task); task.status = COMPLETED; - } catch (x) { + } catch (thrownValue) { + const x = + thrownValue === SuspenseException + ? // 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) API for suspending. This implementation detail can change + // later, once we deprecate the old API in favor of `use`. + getSuspendedThenable() + : thrownValue; + // $FlowFixMe[method-unbinding] if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended again, let's pick it back up later. const ping = task.ping; diff --git a/packages/react-server/src/ReactFlightThenable.js b/packages/react-server/src/ReactFlightThenable.js index 9a91587b04fd7..02d24b1c7665c 100644 --- a/packages/react-server/src/ReactFlightThenable.js +++ b/packages/react-server/src/ReactFlightThenable.js @@ -22,6 +22,18 @@ import type { export opaque type ThenableState = Array>; +// An error that is thrown (e.g. by `use`) to trigger Suspense. If we +// detect this is caught by userspace, we'll log a warning in development. +export const SuspenseException: mixed = new Error( + "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\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`", +); + export function createThenableState(): ThenableState { // The ThenableState is created the first time a component suspends. If it // suspends again, we'll reuse the same state. @@ -92,13 +104,34 @@ export function trackUsedThenable( } // Suspend. - // TODO: Throwing here is an implementation detail that allows us to - // unwind the call stack. But we shouldn't allow it to leak into - // userspace. Throw an opaque placeholder value instead of the - // actual thenable. If it doesn't get captured by the work loop, log - // a warning, because that means something in userspace must have - // caught it. - throw thenable; + // + // Throwing here is an implementation detail that allows us to unwind the + // call stack. But we shouldn't allow it to leak into userspace. Throw an + // opaque placeholder value instead of the actual thenable. If it doesn't + // get captured by the work loop, log a warning, because that means + // something in userspace must have caught it. + suspendedThenable = thenable; + throw SuspenseException; } } } + +// This is used to track the actual thenable that suspended so it can be +// passed to the rest of the Suspense implementation — which, for historical +// reasons, expects to receive a thenable. +let suspendedThenable: Thenable | null = null; +export function getSuspendedThenable(): Thenable { + // This is called right after `use` suspends by throwing an exception. `use` + // throws an opaque value instead of the thenable itself so that it can't be + // caught in userspace. Then the work loop accesses the actual thenable using + // this function. + if (suspendedThenable === null) { + throw new Error( + 'Expected a suspended thenable. This is a bug in React. Please file ' + + 'an issue.', + ); + } + const thenable = suspendedThenable; + suspendedThenable = null; + return thenable; +} diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 9400521ca3c8f..4a3a856d15eaa 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -443,5 +443,7 @@ "455": "This CacheSignal was requested outside React which means that it is immediately aborted.", "456": "Calling Offscreen.detach before instance handle has been set.", "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." + "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`" }