Skip to content

Commit

Permalink
Detect crashes caused by Async Client Components
Browse files Browse the repository at this point in the history
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 #26801 for more details.
  • Loading branch information
acdlite committed Jun 27, 2023
1 parent a1c62b8 commit 18fced0
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 18 deletions.
1 change: 1 addition & 0 deletions packages/react-reconciler/src/ReactFiberLane.js
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
root.entangledLanes &= remainingLanes;

root.errorRecoveryDisabledLanes &= remainingLanes;
root.shellSuspendCounter = 0;

const entanglements = root.entanglements;
const expirationTimes = root.expirationTimes;
Expand Down
1 change: 1 addition & 0 deletions packages/react-reconciler/src/ReactFiberRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ function FiberRootNode(
this.expiredLanes = NoLanes;
this.finishedLanes = NoLanes;
this.errorRecoveryDisabledLanes = NoLanes;
this.shellSuspendCounter = 0;

this.entangledLanes = NoLanes;
this.entanglements = createLaneMap(NoLanes);
Expand Down
37 changes: 37 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -1944,6 +1944,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
markRenderStarted(lanes);
}

let didSuspendInShell = false;
outer: do {
try {
if (
Expand All @@ -1969,6 +1970,42 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
workInProgressRootExitStatus = RootDidNotComplete;
break outer;
}
case SuspendedOnImmediate:
case SuspendedOnData: {
if (!didSuspendInShell) {
const handler = getSuspenseHandler();
if (handler === null) {
didSuspendInShell = true;
const counter = ++root.shellSuspendCounter;
if (counter > 100) {
// This root has suspended repeatedly in the shell without
// making any progress (i.e. committing something). This is
// highly suggestive of an infinite ping loop, often caused by
// an accidental Async Client Component.
//
// During a transition, we can suspend the work loop until the
// promise to resolve, but this is a sync render, so that's
// not an option. We also can't show a fallback, because none
// was provided. So our last resort is to throw an error.
//
// TODO: Remove this error in a future release. Other ways
// of handling this case include forcing a concurrent render,
// or putting the whole root into offscreen mode.
const asyncClientComponentError = new Error(
'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.',
);
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
throwAndUnwindWorkLoop(unitOfWork, asyncClientComponentError);
break;
}
}
}
// Intentional fallthrough
}
default: {
// Unwind then continue with the normal work loop.
workInProgressSuspendedReason = NotSuspended;
Expand Down
1 change: 1 addition & 0 deletions packages/react-reconciler/src/ReactInternalTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ type BaseFiberRootProperties = {
pingedLanes: Lanes,
expiredLanes: Lanes,
errorRecoveryDisabledLanes: Lanes,
shellSuspendCounter: number,

finishedLanes: Lanes,

Expand Down
94 changes: 94 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactUse-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ let waitFor;
let waitForPaint;
let assertLog;
let waitForAll;
let waitForMicrotasks;

describe('ReactUse', () => {
beforeEach(() => {
Expand All @@ -40,6 +41,7 @@ describe('ReactUse', () => {
assertLog = InternalTestUtils.assertLog;
waitForPaint = InternalTestUtils.waitForPaint;
waitFor = InternalTestUtils.waitFor;
waitForMicrotasks = InternalTestUtils.waitForMicrotasks;

pendingTextRequests = new Map();
});
Expand Down Expand Up @@ -1616,4 +1618,96 @@ describe('ReactUse', () => {
assertLog(['C']);
expect(root).toMatchRenderedOutput('C');
});

// @gate !forceConcurrentByDefaultForTesting
test('an async component outside of a Suspense boundary crashes with an error (resolves in microtask)', async () => {
class ErrorBoundary extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
return {error};
}
render() {
if (this.state.error) {
return <Text text={this.state.error.message} />;
}
return this.props.children;
}
}

async function AsyncClientComponent() {
return <Text text="Hi" />;
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(
<ErrorBoundary>
<AsyncClientComponent />
</ErrorBoundary>,
);
});
assertLog([
'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.',
'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.',
]);
expect(root).toMatchRenderedOutput(
'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.',
);
});


// @gate !forceConcurrentByDefaultForTesting
test('an async component outside of a Suspense boundary crashes with an error (resolves in macrotask)', async () => {
class ErrorBoundary extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
return {error};
}
render() {
if (this.state.error) {
return <Text text={this.state.error.message} />;
}
return this.props.children;
}
}

async function AsyncClientComponent() {
await waitForMicrotasks();
return <Text text="Hi" />;
}

const root = ReactNoop.createRoot();
await act(() => {
root.render(
<ErrorBoundary>
<AsyncClientComponent />
</ErrorBoundary>,
);
});
assertLog([
'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.',
'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.',
]);
expect(root).toMatchRenderedOutput(
'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.',
);
});
});
34 changes: 17 additions & 17 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,30 @@
// Use __VARIANT__ to simulate a GK. The tests will be run twice: once
// with the __VARIANT__ set to `true`, and once set to `false`.

export const disableInputAttributeSyncing = __VARIANT__;
export const disableIEWorkarounds = __VARIANT__;
export const enableLegacyFBSupport = __VARIANT__;
export const enableUseRefAccessWarning = __VARIANT__;
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
export const enableLazyContextPropagation = __VARIANT__;
export const forceConcurrentByDefaultForTesting = __VARIANT__;
export const enableUnifiedSyncLane = __VARIANT__;
export const enableTransitionTracing = __VARIANT__;
export const enableCustomElementPropertySupport = __VARIANT__;
export const enableDeferRootSchedulingToMicrotask = __VARIANT__;
export const diffInCommitPhase = __VARIANT__;
export const enableAsyncActions = __VARIANT__;
export const alwaysThrottleRetries = __VARIANT__;
export const enableDO_NOT_USE_disableStrictPassiveEffect = __VARIANT__;
export const disableInputAttributeSyncing = !__VARIANT__;
export const disableIEWorkarounds = !__VARIANT__;
export const enableLegacyFBSupport = !__VARIANT__;
export const enableUseRefAccessWarning = !__VARIANT__;
export const enableProfilerNestedUpdateScheduledHook = !__VARIANT__;
export const disableSchedulerTimeoutInWorkLoop = !__VARIANT__;
export const enableLazyContextPropagation = !__VARIANT__;
export const forceConcurrentByDefaultForTesting = !__VARIANT__;
export const enableUnifiedSyncLane = !__VARIANT__;
export const enableTransitionTracing = !__VARIANT__;
export const enableCustomElementPropertySupport = !__VARIANT__;
export const enableDeferRootSchedulingToMicrotask = !__VARIANT__;
export const diffInCommitPhase = !__VARIANT__;
export const enableAsyncActions = !__VARIANT__;
export const alwaysThrottleRetries = !__VARIANT__;
export const enableDO_NOT_USE_disableStrictPassiveEffect = !__VARIANT__;

// Enable this flag to help with concurrent mode debugging.
// It logs information to the console about React scheduling, rendering, and commit phases.
//
// NOTE: This feature will only work in DEV mode; all callsites are wrapped with __DEV__.
export const enableDebugTracing = __EXPERIMENTAL__;

export const enableSchedulingProfiler = __VARIANT__;
export const enableSchedulingProfiler = !__VARIANT__;

// These are already tested in both modes using the build type dimension,
// so we don't need to use __VARIANT__ to get extra coverage.
Expand Down
3 changes: 2 additions & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -466,5 +466,6 @@
"478": "Thenable should have already resolved. This is a bug in React.",
"479": "Cannot update optimistic state while rendering.",
"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.",
"481": "Tried to encode a Server Action from a different instance than the encoder is from. This is a bug in React."
"481": "Tried to encode a Server Action from a different instance than the encoder is from. This is a bug in React.",
"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."
}

0 comments on commit 18fced0

Please sign in to comment.