Skip to content

Commit d2a0176

Browse files
acdliteacdlite
authored
Detect and warn if use(promise) is wrapped with try/catch block (#25543)
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 detail — 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.
1 parent cf3932b commit d2a0176

File tree

12 files changed

+373
-63
lines changed

12 files changed

+373
-63
lines changed

‎packages/react-reconciler/src/ReactFiberHooks.new.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ import {now} from './Scheduler';
138138
import {
139139
prepareThenableState,
140140
trackUsedThenable,
141+
checkIfUseWrappedInTryCatch,
141142
} from './ReactFiberThenable.new';
142143

143144
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
@@ -160,8 +161,10 @@ export type UpdateQueue<S, A> = {
160161

161162
let didWarnAboutMismatchedHooksForComponent;
162163
let didWarnUncachedGetSnapshot;
164+
let didWarnAboutUseWrappedInTryCatch;
163165
if (__DEV__) {
164166
didWarnAboutMismatchedHooksForComponent = new Set();
167+
didWarnAboutUseWrappedInTryCatch = new Set();
165168
}
166169

167170
export type Hook = {
@@ -594,6 +597,22 @@ export function renderWithHooks<Props, SecondArg>(
594597
}
595598
}
596599
}
600+
601+
if (__DEV__) {
602+
if (checkIfUseWrappedInTryCatch()) {
603+
const componentName =
604+
getComponentNameFromFiber(workInProgress) || 'Unknown';
605+
if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) {
606+
didWarnAboutUseWrappedInTryCatch.add(componentName);
607+
console.error(
608+
'`use` was called from inside a try/catch block. This is not allowed ' +
609+
'and can lead to unexpected behavior. To handle errors triggered ' +
610+
'by `use`, wrap your component in a error boundary.',
611+
);
612+
}
613+
}
614+
}
615+
597616
return children;
598617
}
599618

‎packages/react-reconciler/src/ReactFiberHooks.old.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ import {now} from './Scheduler';
138138
import {
139139
prepareThenableState,
140140
trackUsedThenable,
141+
checkIfUseWrappedInTryCatch,
141142
} from './ReactFiberThenable.old';
142143

143144
const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals;
@@ -160,8 +161,10 @@ export type UpdateQueue<S, A> = {
160161

161162
let didWarnAboutMismatchedHooksForComponent;
162163
let didWarnUncachedGetSnapshot;
164+
let didWarnAboutUseWrappedInTryCatch;
163165
if (__DEV__) {
164166
didWarnAboutMismatchedHooksForComponent = new Set();
167+
didWarnAboutUseWrappedInTryCatch = new Set();
165168
}
166169

167170
export type Hook = {
@@ -594,6 +597,22 @@ export function renderWithHooks<Props, SecondArg>(
594597
}
595598
}
596599
}
600+
601+
if (__DEV__) {
602+
if (checkIfUseWrappedInTryCatch()) {
603+
const componentName =
604+
getComponentNameFromFiber(workInProgress) || 'Unknown';
605+
if (!didWarnAboutUseWrappedInTryCatch.has(componentName)) {
606+
didWarnAboutUseWrappedInTryCatch.add(componentName);
607+
console.error(
608+
'`use` was called from inside a try/catch block. This is not allowed ' +
609+
'and can lead to unexpected behavior. To handle errors triggered ' +
610+
'by `use`, wrap your component in a error boundary.',
611+
);
612+
}
613+
}
614+
}
615+
597616
return children;
598617
}
599618

‎packages/react-reconciler/src/ReactFiberThenable.new.js‎

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,18 @@ const {ReactCurrentActQueue} = ReactSharedInternals;
1919

2020
export opaque type ThenableState = Array<Thenable<any>>;
2121

22+
// An error that is thrown (e.g. by `use`) to trigger Suspense. If we
23+
// detect this is caught by userspace, we'll log a warning in development.
24+
export const SuspenseException: mixed = new Error(
25+
"Suspense Exception: This is not a real error! It's an implementation " +
26+
'detail of `use` to interrupt the current render. You must either ' +
27+
'rethrow it immediately, or move the `use` call outside of the ' +
28+
'`try/catch` block. Capturing without rethrowing will lead to ' +
29+
'unexpected behavior.\n\n' +
30+
'To handle async errors, wrap your component in an error boundary, or ' +
31+
"call the promise's `.catch` method and pass the result to `use`",
32+
);
33+
2234
let thenableState: ThenableState | null = null;
2335

2436
export function createThenableState(): ThenableState {
@@ -37,19 +49,9 @@ export function prepareThenableState(prevThenableState: ThenableState | null) {
3749
export function getThenableStateAfterSuspending(): ThenableState | null {
3850
// Called by the work loop so it can stash the thenable state. It will use
3951
// the state to replay the component when the promise resolves.
40-
if (
41-
thenableState !== null &&
42-
// If we only `use`-ed resolved promises, then there is no suspended state
43-
// TODO: The only reason we do this is to distinguish between throwing a
44-
// promise (old Suspense pattern) versus `use`-ing one. A better solution is
45-
// for `use` to throw a special, opaque value instead of a promise.
46-
!isThenableStateResolved(thenableState)
47-
) {
48-
const state = thenableState;
49-
thenableState = null;
50-
return state;
51-
}
52-
return null;
52+
const state = thenableState;
53+
thenableState = null;
54+
return state;
5355
}
5456

5557
export function isThenableStateResolved(thenables: ThenableState): boolean {
@@ -129,13 +131,54 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
129131
}
130132

131133
// Suspend.
132-
// TODO: Throwing here is an implementation detail that allows us to
133-
// unwind the call stack. But we shouldn't allow it to leak into
134-
// userspace. Throw an opaque placeholder value instead of the
135-
// actual thenable. If it doesn't get captured by the work loop, log
136-
// a warning, because that means something in userspace must have
137-
// caught it.
138-
throw thenable;
134+
//
135+
// Throwing here is an implementation detail that allows us to unwind the
136+
// call stack. But we shouldn't allow it to leak into userspace. Throw an
137+
// opaque placeholder value instead of the actual thenable. If it doesn't
138+
// get captured by the work loop, log a warning, because that means
139+
// something in userspace must have caught it.
140+
suspendedThenable = thenable;
141+
if (__DEV__) {
142+
needsToResetSuspendedThenableDEV = true;
143+
}
144+
throw SuspenseException;
145+
}
146+
}
147+
}
148+
149+
// This is used to track the actual thenable that suspended so it can be
150+
// passed to the rest of the Suspense implementation — which, for historical
151+
// reasons, expects to receive a thenable.
152+
let suspendedThenable: Thenable<any> | null = null;
153+
let needsToResetSuspendedThenableDEV = false;
154+
export function getSuspendedThenable(): Thenable<mixed> {
155+
// This is called right after `use` suspends by throwing an exception. `use`
156+
// throws an opaque value instead of the thenable itself so that it can't be
157+
// caught in userspace. Then the work loop accesses the actual thenable using
158+
// this function.
159+
if (suspendedThenable === null) {
160+
throw new Error(
161+
'Expected a suspended thenable. This is a bug in React. Please file ' +
162+
'an issue.',
163+
);
164+
}
165+
const thenable = suspendedThenable;
166+
suspendedThenable = null;
167+
if (__DEV__) {
168+
needsToResetSuspendedThenableDEV = false;
169+
}
170+
return thenable;
171+
}
172+
173+
export function checkIfUseWrappedInTryCatch(): boolean {
174+
if (__DEV__) {
175+
// This was set right before SuspenseException was thrown, and it should
176+
// have been cleared when the exception was handled. If it wasn't,
177+
// it must have been caught by userspace.
178+
if (needsToResetSuspendedThenableDEV) {
179+
needsToResetSuspendedThenableDEV = false;
180+
return true;
139181
}
140182
}
183+
return false;
141184
}

‎packages/react-reconciler/src/ReactFiberThenable.old.js‎

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,18 @@ const {ReactCurrentActQueue} = ReactSharedInternals;
1919

2020
export opaque type ThenableState = Array<Thenable<any>>;
2121

22+
// An error that is thrown (e.g. by `use`) to trigger Suspense. If we
23+
// detect this is caught by userspace, we'll log a warning in development.
24+
export const SuspenseException: mixed = new Error(
25+
"Suspense Exception: This is not a real error! It's an implementation " +
26+
'detail of `use` to interrupt the current render. You must either ' +
27+
'rethrow it immediately, or move the `use` call outside of the ' +
28+
'`try/catch` block. Capturing without rethrowing will lead to ' +
29+
'unexpected behavior.\n\n' +
30+
'To handle async errors, wrap your component in an error boundary, or ' +
31+
"call the promise's `.catch` method and pass the result to `use`",
32+
);
33+
2234
let thenableState: ThenableState | null = null;
2335

2436
export function createThenableState(): ThenableState {
@@ -37,19 +49,9 @@ export function prepareThenableState(prevThenableState: ThenableState | null) {
3749
export function getThenableStateAfterSuspending(): ThenableState | null {
3850
// Called by the work loop so it can stash the thenable state. It will use
3951
// the state to replay the component when the promise resolves.
40-
if (
41-
thenableState !== null &&
42-
// If we only `use`-ed resolved promises, then there is no suspended state
43-
// TODO: The only reason we do this is to distinguish between throwing a
44-
// promise (old Suspense pattern) versus `use`-ing one. A better solution is
45-
// for `use` to throw a special, opaque value instead of a promise.
46-
!isThenableStateResolved(thenableState)
47-
) {
48-
const state = thenableState;
49-
thenableState = null;
50-
return state;
51-
}
52-
return null;
52+
const state = thenableState;
53+
thenableState = null;
54+
return state;
5355
}
5456

5557
export function isThenableStateResolved(thenables: ThenableState): boolean {
@@ -129,13 +131,54 @@ export function trackUsedThenable<T>(thenable: Thenable<T>, index: number): T {
129131
}
130132

131133
// Suspend.
132-
// TODO: Throwing here is an implementation detail that allows us to
133-
// unwind the call stack. But we shouldn't allow it to leak into
134-
// userspace. Throw an opaque placeholder value instead of the
135-
// actual thenable. If it doesn't get captured by the work loop, log
136-
// a warning, because that means something in userspace must have
137-
// caught it.
138-
throw thenable;
134+
//
135+
// Throwing here is an implementation detail that allows us to unwind the
136+
// call stack. But we shouldn't allow it to leak into userspace. Throw an
137+
// opaque placeholder value instead of the actual thenable. If it doesn't
138+
// get captured by the work loop, log a warning, because that means
139+
// something in userspace must have caught it.
140+
suspendedThenable = thenable;
141+
if (__DEV__) {
142+
needsToResetSuspendedThenableDEV = true;
143+
}
144+
throw SuspenseException;
145+
}
146+
}
147+
}
148+
149+
// This is used to track the actual thenable that suspended so it can be
150+
// passed to the rest of the Suspense implementation — which, for historical
151+
// reasons, expects to receive a thenable.
152+
let suspendedThenable: Thenable<any> | null = null;
153+
let needsToResetSuspendedThenableDEV = false;
154+
export function getSuspendedThenable(): Thenable<mixed> {
155+
// This is called right after `use` suspends by throwing an exception. `use`
156+
// throws an opaque value instead of the thenable itself so that it can't be
157+
// caught in userspace. Then the work loop accesses the actual thenable using
158+
// this function.
159+
if (suspendedThenable === null) {
160+
throw new Error(
161+
'Expected a suspended thenable. This is a bug in React. Please file ' +
162+
'an issue.',
163+
);
164+
}
165+
const thenable = suspendedThenable;
166+
suspendedThenable = null;
167+
if (__DEV__) {
168+
needsToResetSuspendedThenableDEV = false;
169+
}
170+
return thenable;
171+
}
172+
173+
export function checkIfUseWrappedInTryCatch(): boolean {
174+
if (__DEV__) {
175+
// This was set right before SuspenseException was thrown, and it should
176+
// have been cleared when the exception was handled. If it wasn't,
177+
// it must have been caught by userspace.
178+
if (needsToResetSuspendedThenableDEV) {
179+
needsToResetSuspendedThenableDEV = false;
180+
return true;
139181
}
140182
}
183+
return false;
141184
}

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ import {
266266
} from './ReactFiberAct.new';
267267
import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.new';
268268
import {
269+
SuspenseException,
270+
getSuspendedThenable,
269271
getThenableStateAfterSuspending,
270272
isThenableStateResolved,
271273
} from './ReactFiberThenable.new';
@@ -1722,13 +1724,26 @@ function handleThrow(root, thrownValue): void {
17221724
// separate issue. Write a regression test using string refs.
17231725
ReactCurrentOwner.current = null;
17241726

1727+
if (thrownValue === SuspenseException) {
1728+
// This is a special type of exception used for Suspense. For historical
1729+
// reasons, the rest of the Suspense implementation expects the thrown value
1730+
// to be a thenable, because before `use` existed that was the (unstable)
1731+
// API for suspending. This implementation detail can change later, once we
1732+
// deprecate the old API in favor of `use`.
1733+
thrownValue = getSuspendedThenable();
1734+
workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
1735+
} else {
1736+
// This is a regular error. If something earlier in the component already
1737+
// suspended, we must clear the thenable state to unblock the work loop.
1738+
workInProgressSuspendedThenableState = null;
1739+
}
1740+
17251741
// Setting this to `true` tells the work loop to unwind the stack instead
17261742
// of entering the begin phase. It's called "suspended" because it usually
17271743
// happens because of Suspense, but it also applies to errors. Think of it
17281744
// as suspending the execution of the work loop.
17291745
workInProgressIsSuspended = true;
17301746
workInProgressThrownValue = thrownValue;
1731-
workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
17321747

17331748
const erroredWork = workInProgress;
17341749
if (erroredWork === null) {
@@ -1750,6 +1765,7 @@ function handleThrow(root, thrownValue): void {
17501765
if (
17511766
thrownValue !== null &&
17521767
typeof thrownValue === 'object' &&
1768+
// $FlowFixMe[method-unbinding]
17531769
typeof thrownValue.then === 'function'
17541770
) {
17551771
const wakeable: Wakeable = (thrownValue: any);
@@ -3503,6 +3519,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
35033519
} catch (originalError) {
35043520
if (
35053521
didSuspendOrErrorWhileHydratingDEV() ||
3522+
originalError === SuspenseException ||
35063523
(originalError !== null &&
35073524
typeof originalError === 'object' &&
35083525
typeof originalError.then === 'function')

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ import {
266266
} from './ReactFiberAct.old';
267267
import {processTransitionCallbacks} from './ReactFiberTracingMarkerComponent.old';
268268
import {
269+
SuspenseException,
270+
getSuspendedThenable,
269271
getThenableStateAfterSuspending,
270272
isThenableStateResolved,
271273
} from './ReactFiberThenable.old';
@@ -1722,13 +1724,26 @@ function handleThrow(root, thrownValue): void {
17221724
// separate issue. Write a regression test using string refs.
17231725
ReactCurrentOwner.current = null;
17241726

1727+
if (thrownValue === SuspenseException) {
1728+
// This is a special type of exception used for Suspense. For historical
1729+
// reasons, the rest of the Suspense implementation expects the thrown value
1730+
// to be a thenable, because before `use` existed that was the (unstable)
1731+
// API for suspending. This implementation detail can change later, once we
1732+
// deprecate the old API in favor of `use`.
1733+
thrownValue = getSuspendedThenable();
1734+
workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
1735+
} else {
1736+
// This is a regular error. If something earlier in the component already
1737+
// suspended, we must clear the thenable state to unblock the work loop.
1738+
workInProgressSuspendedThenableState = null;
1739+
}
1740+
17251741
// Setting this to `true` tells the work loop to unwind the stack instead
17261742
// of entering the begin phase. It's called "suspended" because it usually
17271743
// happens because of Suspense, but it also applies to errors. Think of it
17281744
// as suspending the execution of the work loop.
17291745
workInProgressIsSuspended = true;
17301746
workInProgressThrownValue = thrownValue;
1731-
workInProgressSuspendedThenableState = getThenableStateAfterSuspending();
17321747

17331748
const erroredWork = workInProgress;
17341749
if (erroredWork === null) {
@@ -1750,6 +1765,7 @@ function handleThrow(root, thrownValue): void {
17501765
if (
17511766
thrownValue !== null &&
17521767
typeof thrownValue === 'object' &&
1768+
// $FlowFixMe[method-unbinding]
17531769
typeof thrownValue.then === 'function'
17541770
) {
17551771
const wakeable: Wakeable = (thrownValue: any);
@@ -3503,6 +3519,7 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
35033519
} catch (originalError) {
35043520
if (
35053521
didSuspendOrErrorWhileHydratingDEV() ||
3522+
originalError === SuspenseException ||
35063523
(originalError !== null &&
35073524
typeof originalError === 'object' &&
35083525
typeof originalError.then === 'function')

0 commit comments

Comments
 (0)