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`" }