Skip to content

Commit 32d0c31

Browse files
acdliteAndyPengc12
authored andcommitted
Detect crashes caused by Async Client Components (facebook#27019)
Suspending with an uncached promise is not yet supported. We only support suspending on promises that are cached between render attempts. (We do plan to partially support this in the future, at least in certain constrained cases, like during a route transition.) This includes the case where a component returns an uncached promise, which is effectively what happens if a Client Component is authored using async/await syntax. This is an easy mistake to make in a Server Components app, because async/await _is_ available in Server Components. In the current behavior, this can sometimes cause the app to crash with an infinite loop, because React will repeatedly keep trying to render the component, which will result in a fresh promise, which will result in a new render attempt, and so on. We have some strategies we can use to prevent this — during a concurrent render, we can suspend the work loop until the promise resolves. If it's not a concurrent render, we can show a Suspense fallback and try again at concurrent priority. There's one case where neither of these strategies work, though: during a sync render when there's no parent Suspense boundary. (We refer to this as the "shell" of the app because it exists outside of any loading UI.) Since we don't have any great options for this scenario, we should at least error gracefully instead of crashing the app. So this commit adds a detection mechanism for render loops caused by async client components. The way it works is, if an app suspends repeatedly in the shell during a synchronous render, without committing anything in between, we will count the number of attempts and eventually trigger an error once the count exceeds a threshold. In the future, we will consider ways to make this case a warning instead of a hard error. See facebook#26801 for more details.
1 parent ffffcfe commit 32d0c31

File tree

7 files changed

+145
-1
lines changed

7 files changed

+145
-1
lines changed

packages/react-reconciler/src/ReactFiberLane.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
636636
root.entangledLanes &= remainingLanes;
637637

638638
root.errorRecoveryDisabledLanes &= remainingLanes;
639+
root.shellSuspendCounter = 0;
639640

640641
const entanglements = root.entanglements;
641642
const expirationTimes = root.expirationTimes;

packages/react-reconciler/src/ReactFiberRoot.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ function FiberRootNode(
7474
this.expiredLanes = NoLanes;
7575
this.finishedLanes = NoLanes;
7676
this.errorRecoveryDisabledLanes = NoLanes;
77+
this.shellSuspendCounter = 0;
7778

7879
this.entangledLanes = NoLanes;
7980
this.entanglements = createLaneMap(NoLanes);

packages/react-reconciler/src/ReactFiberThenable.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import type {
1414
RejectedThenable,
1515
} from 'shared/ReactTypes';
1616

17+
import {getWorkInProgressRoot} from './ReactFiberWorkLoop';
18+
1719
import ReactSharedInternals from 'shared/ReactSharedInternals';
1820
const {ReactCurrentActQueue} = ReactSharedInternals;
1921

@@ -103,6 +105,32 @@ export function trackUsedThenable<T>(
103105
// happen. Flight lazily parses JSON when the value is actually awaited.
104106
thenable.then(noop, noop);
105107
} else {
108+
// This is an uncached thenable that we haven't seen before.
109+
110+
// Detect infinite ping loops caused by uncached promises.
111+
const root = getWorkInProgressRoot();
112+
if (root !== null && root.shellSuspendCounter > 100) {
113+
// This root has suspended repeatedly in the shell without making any
114+
// progress (i.e. committing something). This is highly suggestive of
115+
// an infinite ping loop, often caused by an accidental Async Client
116+
// Component.
117+
//
118+
// During a transition, we can suspend the work loop until the promise
119+
// to resolve, but this is a sync render, so that's not an option. We
120+
// also can't show a fallback, because none was provided. So our last
121+
// resort is to throw an error.
122+
//
123+
// TODO: Remove this error in a future release. Other ways of handling
124+
// this case include forcing a concurrent render, or putting the whole
125+
// root into offscreen mode.
126+
throw new Error(
127+
'async/await is not yet supported in Client Components, only ' +
128+
'Server Components. This error is often caused by accidentally ' +
129+
"adding `'use client'` to a module that was originally written " +
130+
'for the server.',
131+
);
132+
}
133+
106134
const pendingThenable: PendingThenable<T> = (thenable: any);
107135
pendingThenable.status = 'pending';
108136
pendingThenable.then(

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,6 +1944,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
19441944
markRenderStarted(lanes);
19451945
}
19461946

1947+
let didSuspendInShell = false;
19471948
outer: do {
19481949
try {
19491950
if (
@@ -1969,6 +1970,13 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
19691970
workInProgressRootExitStatus = RootDidNotComplete;
19701971
break outer;
19711972
}
1973+
case SuspendedOnImmediate:
1974+
case SuspendedOnData: {
1975+
if (!didSuspendInShell && getSuspenseHandler() === null) {
1976+
didSuspendInShell = true;
1977+
}
1978+
// Intentional fallthrough
1979+
}
19721980
default: {
19731981
// Unwind then continue with the normal work loop.
19741982
workInProgressSuspendedReason = NotSuspended;
@@ -1984,6 +1992,17 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
19841992
handleThrow(root, thrownValue);
19851993
}
19861994
} while (true);
1995+
1996+
// Check if something suspended in the shell. We use this to detect an
1997+
// infinite ping loop caused by an uncached promise.
1998+
//
1999+
// Only increment this counter once per synchronous render attempt across the
2000+
// whole tree. Even if there are many sibling components that suspend, this
2001+
// counter only gets incremented once.
2002+
if (didSuspendInShell) {
2003+
root.shellSuspendCounter++;
2004+
}
2005+
19872006
resetContextDependencies();
19882007

19892008
executionContext = prevExecutionContext;

packages/react-reconciler/src/ReactInternalTypes.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ type BaseFiberRootProperties = {
248248
pingedLanes: Lanes,
249249
expiredLanes: Lanes,
250250
errorRecoveryDisabledLanes: Lanes,
251+
shellSuspendCounter: number,
251252

252253
finishedLanes: Lanes,
253254

packages/react-reconciler/src/__tests__/ReactUse-test.js

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ let waitFor;
1717
let waitForPaint;
1818
let assertLog;
1919
let waitForAll;
20+
let waitForMicrotasks;
2021

2122
describe('ReactUse', () => {
2223
beforeEach(() => {
@@ -40,6 +41,7 @@ describe('ReactUse', () => {
4041
assertLog = InternalTestUtils.assertLog;
4142
waitForPaint = InternalTestUtils.waitForPaint;
4243
waitFor = InternalTestUtils.waitFor;
44+
waitForMicrotasks = InternalTestUtils.waitForMicrotasks;
4345

4446
pendingTextRequests = new Map();
4547
});
@@ -1616,4 +1618,95 @@ describe('ReactUse', () => {
16161618
assertLog(['C']);
16171619
expect(root).toMatchRenderedOutput('C');
16181620
});
1621+
1622+
// @gate !forceConcurrentByDefaultForTesting
1623+
test('an async component outside of a Suspense boundary crashes with an error (resolves in microtask)', async () => {
1624+
class ErrorBoundary extends React.Component {
1625+
state = {error: null};
1626+
static getDerivedStateFromError(error) {
1627+
return {error};
1628+
}
1629+
render() {
1630+
if (this.state.error) {
1631+
return <Text text={this.state.error.message} />;
1632+
}
1633+
return this.props.children;
1634+
}
1635+
}
1636+
1637+
async function AsyncClientComponent() {
1638+
return <Text text="Hi" />;
1639+
}
1640+
1641+
const root = ReactNoop.createRoot();
1642+
await act(() => {
1643+
root.render(
1644+
<ErrorBoundary>
1645+
<AsyncClientComponent />
1646+
</ErrorBoundary>,
1647+
);
1648+
});
1649+
assertLog([
1650+
'async/await is not yet supported in Client Components, only Server ' +
1651+
'Components. This error is often caused by accidentally adding ' +
1652+
"`'use client'` to a module that was originally written for " +
1653+
'the server.',
1654+
'async/await is not yet supported in Client Components, only Server ' +
1655+
'Components. This error is often caused by accidentally adding ' +
1656+
"`'use client'` to a module that was originally written for " +
1657+
'the server.',
1658+
]);
1659+
expect(root).toMatchRenderedOutput(
1660+
'async/await is not yet supported in Client Components, only Server ' +
1661+
'Components. This error is often caused by accidentally adding ' +
1662+
"`'use client'` to a module that was originally written for " +
1663+
'the server.',
1664+
);
1665+
});
1666+
1667+
// @gate !forceConcurrentByDefaultForTesting
1668+
test('an async component outside of a Suspense boundary crashes with an error (resolves in macrotask)', async () => {
1669+
class ErrorBoundary extends React.Component {
1670+
state = {error: null};
1671+
static getDerivedStateFromError(error) {
1672+
return {error};
1673+
}
1674+
render() {
1675+
if (this.state.error) {
1676+
return <Text text={this.state.error.message} />;
1677+
}
1678+
return this.props.children;
1679+
}
1680+
}
1681+
1682+
async function AsyncClientComponent() {
1683+
await waitForMicrotasks();
1684+
return <Text text="Hi" />;
1685+
}
1686+
1687+
const root = ReactNoop.createRoot();
1688+
await act(() => {
1689+
root.render(
1690+
<ErrorBoundary>
1691+
<AsyncClientComponent />
1692+
</ErrorBoundary>,
1693+
);
1694+
});
1695+
assertLog([
1696+
'async/await is not yet supported in Client Components, only Server ' +
1697+
'Components. This error is often caused by accidentally adding ' +
1698+
"`'use client'` to a module that was originally written for " +
1699+
'the server.',
1700+
'async/await is not yet supported in Client Components, only Server ' +
1701+
'Components. This error is often caused by accidentally adding ' +
1702+
"`'use client'` to a module that was originally written for " +
1703+
'the server.',
1704+
]);
1705+
expect(root).toMatchRenderedOutput(
1706+
'async/await is not yet supported in Client Components, only Server ' +
1707+
'Components. This error is often caused by accidentally adding ' +
1708+
"`'use client'` to a module that was originally written for " +
1709+
'the server.',
1710+
);
1711+
});
16191712
});

scripts/error-codes/codes.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,5 +466,6 @@
466466
"478": "Thenable should have already resolved. This is a bug in React.",
467467
"479": "Cannot update optimistic state while rendering.",
468468
"480": "File/Blob fields are not yet supported in progressive forms. It probably means you are closing over binary data or FormData in a Server Action.",
469-
"481": "Tried to encode a Server Action from a different instance than the encoder is from. This is a bug in React."
469+
"481": "Tried to encode a Server Action from a different instance than the encoder is from. This is a bug in React.",
470+
"482": "async/await is not yet supported in Client Components, only Server Components. This error is often caused by accidentally adding `'use client'` to a module that was originally written for the server."
470471
}

0 commit comments

Comments
 (0)