Skip to content

Commit

Permalink
useActionState: On error, cancel remaining actions (#29695)
Browse files Browse the repository at this point in the history
Based on

- #29694 

---

If an action in the useActionState queue errors, we shouldn't run any
subsequent actions. The contract of useActionState is that the actions
run in sequence, and that one action can assume that all previous
actions have completed successfully.

For example, in a shopping cart UI, you might dispatch an "Add to cart"
action followed by a "Checkout" action. If the "Add to cart" action
errors, the "Checkout" action should not run.

An implication of this change is that once useActionState falls into an
error state, the only way to recover is to reset the component tree,
i.e. by unmounting and remounting. The way to customize the error
handling behavior is to wrap the action body in a try/catch.
  • Loading branch information
acdlite authored Jun 3, 2024
1 parent 67b05be commit 9598c41
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 49 deletions.
85 changes: 56 additions & 29 deletions packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1237,14 +1237,12 @@ describe('ReactDOMForm', () => {

// @gate enableAsyncActions
test('useActionState: error handling (sync action)', async () => {
let resetErrorBoundary;
class ErrorBoundary extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
return {error};
}
render() {
resetErrorBoundary = () => this.setState({error: null});
if (this.state.error !== null) {
return <Text text={'Caught an error: ' + this.state.error.message} />;
}
Expand Down Expand Up @@ -1284,31 +1282,16 @@ describe('ReactDOMForm', () => {
'Caught an error: Oops!',
]);
expect(container.textContent).toBe('Caught an error: Oops!');

// Reset the error boundary
await act(() => resetErrorBoundary());
assertLog(['A']);

// Trigger an error again, but this time, perform another action that
// overrides the first one and fixes the error
await act(() => {
startTransition(() => action('Oops!'));
startTransition(() => action('B'));
});
assertLog(['Pending A', 'B']);
expect(container.textContent).toBe('B');
});

// @gate enableAsyncActions
test('useActionState: error handling (async action)', async () => {
let resetErrorBoundary;
class ErrorBoundary extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
return {error};
}
render() {
resetErrorBoundary = () => this.setState({error: null});
if (this.state.error !== null) {
return <Text text={'Caught an error: ' + this.state.error.message} />;
}
Expand Down Expand Up @@ -1346,21 +1329,65 @@ describe('ReactDOMForm', () => {
await act(() => resolveText('Oops!'));
assertLog(['Caught an error: Oops!', 'Caught an error: Oops!']);
expect(container.textContent).toBe('Caught an error: Oops!');
});

test('useActionState: when an action errors, subsequent actions are canceled', async () => {
class ErrorBoundary extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
return {error};
}
render() {
if (this.state.error !== null) {
return <Text text={'Caught an error: ' + this.state.error.message} />;
}
return this.props.children;
}
}

let action;
function App() {
const [state, dispatch, isPending] = useActionState(async (s, a) => {
Scheduler.log('Start action: ' + a);
const text = await getText(a);
if (text.endsWith('!')) {
throw new Error(text);
}
return text;
}, 'A');
action = dispatch;
const pending = isPending ? 'Pending ' : '';
return <Text text={pending + state} />;
}

// Reset the error boundary
await act(() => resetErrorBoundary());
const root = ReactDOMClient.createRoot(container);
await act(() =>
root.render(
<ErrorBoundary>
<App />
</ErrorBoundary>,
),
);
assertLog(['A']);

// Trigger an error again, but this time, perform another action that
// overrides the first one and fixes the error
await act(() => {
startTransition(() => action('Oops!'));
startTransition(() => action('B'));
});
assertLog(['Pending A']);
await act(() => resolveText('B'));
assertLog(['B']);
expect(container.textContent).toBe('B');
await act(() => startTransition(() => action('Oops!')));
assertLog(['Start action: Oops!', 'Pending A']);

// Queue up another action after the one will error.
await act(() => startTransition(() => action('Should never run')));
assertLog([]);

// The first dispatch will update the pending state.
await act(() => resolveText('Oops!'));
assertLog(['Caught an error: Oops!', 'Caught an error: Oops!']);
expect(container.textContent).toBe('Caught an error: Oops!');

// Attempt to dispatch another action. This should not run either.
await act(() =>
startTransition(() => action('This also should never run')),
);
assertLog([]);
expect(container.textContent).toBe('Caught an error: Oops!');
});

// @gate enableAsyncActions
Expand Down
41 changes: 21 additions & 20 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1994,7 +1994,9 @@ type ActionStateQueue<S, P> = {
dispatch: Dispatch<P>,
// This is the most recent action function that was rendered. It's updated
// during the commit phase.
action: (Awaited<S>, P) => S,
// If it's null, it means the action queue errored and subsequent actions
// should not run.
action: ((Awaited<S>, P) => S) | null,
// This is a circular linked list of pending action payloads. It incudes the
// action that is currently running.
pending: ActionStateQueueNode<S, P> | null,
Expand Down Expand Up @@ -2033,9 +2035,15 @@ function dispatchActionState<S, P>(
throw new Error('Cannot update form state while rendering.');
}

const currentAction = actionQueue.action;
if (currentAction === null) {
// An earlier action errored. Subsequent actions should not run.
return;
}

const actionNode: ActionStateQueueNode<S, P> = {
payload,
action: actionQueue.action,
action: currentAction,
next: (null: any), // circular

isTransition: true,
Expand Down Expand Up @@ -2218,28 +2226,21 @@ function onActionError<S, P>(
actionNode: ActionStateQueueNode<S, P>,
error: mixed,
) {
actionNode.status = 'rejected';
actionNode.reason = error;
notifyActionListeners(actionNode);

// Pop the action from the queue and run the next pending action, if there
// are any.
// TODO: We should instead abort all the remaining actions in the queue.
// Mark all the following actions as rejected.
const last = actionQueue.pending;
actionQueue.pending = null;
if (last !== null) {
const first = last.next;
if (first === last) {
// This was the last action in the queue.
actionQueue.pending = null;
} else {
// Remove the first node from the circular queue.
const next = first.next;
last.next = next;

// Run the next action.
runActionStateAction(actionQueue, next);
}
do {
actionNode.status = 'rejected';
actionNode.reason = error;
notifyActionListeners(actionNode);
actionNode = actionNode.next;
} while (actionNode !== first);
}

// Prevent subsequent actions from being dispatched.
actionQueue.action = null;
}

function notifyActionListeners<S, P>(actionNode: ActionStateQueueNode<S, P>) {
Expand Down

0 comments on commit 9598c41

Please sign in to comment.