Skip to content

Commit 8ff18e2

Browse files
authored
Fix useAbortableAsync race condition (#207365)
`useAbortableAsync` can easily get confused about the current state - e.g. when a previous invocation gets aborted and a new one is started at the same time, the `loading` state gets set to false _after_ the next invocation got started, so it's false for the time it's running: ![old](https://github.com/user-attachments/assets/6a784b1a-58b2-4951-8d25-9f109bce39c5) You can see that while typing, the old slow request is aborted properly, but the `loading` state gets lost and the abort error from the last invocation is still set even though a new request is running already. This is not the only possible issue that could happen here - e.g. if the promise chain throws too late, an unrelated error could be set in the error handling logic, which is not related to the currently running `fn`. This is hard to fix because as the hook does not control the `fn`, it does not know at which point it resolves, even after a new invocation was started already. The abort signal asks the `fn` nicely to throw with an abort error, but it can't be controlled when that happens. This PR introduces a notion of the current "generation" and only accepts state updates from the most recent one. With this, the new invocation correctly sets the loading state after the abort - what happens to the old promise chain after the abort can't affect the state anymore: ![new](https://github.com/user-attachments/assets/b39dd725-6bf1-4ef1-9eb6-d1463e1ec146) I'm not sure whether this is the best way to resolve this issue, but I couldn't come up with a better way. Happy to adjust, but I think we need a solution that doesn't assume any special behavior of the passed in `fn`, otherwise this helper will always be super brittle.
1 parent 1ec6245 commit 8ff18e2

File tree

1 file changed

+10
-1
lines changed

1 file changed

+10
-1
lines changed

x-pack/solutions/observability/packages/utils_browser/hooks/use_abortable_async.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,17 @@ export function useAbortableAsync<T>(
5858
const controller = new AbortController();
5959
controllerRef.current = controller;
6060

61+
function isRequestStale() {
62+
return controllerRef.current !== controller;
63+
}
64+
6165
if (clearValueOnNext) {
6266
setValue(undefined);
6367
setError(undefined);
6468
}
6569

6670
function handleError(err: Error) {
71+
if (isRequestStale()) return;
6772
setError(err);
6873
if (unsetValueOnError) {
6974
setValue(undefined);
@@ -78,11 +83,15 @@ export function useAbortableAsync<T>(
7883
setLoading(true);
7984
response
8085
.then((nextValue) => {
86+
if (isRequestStale()) return;
8187
setError(undefined);
8288
setValue(nextValue);
8389
})
8490
.catch(handleError)
85-
.finally(() => setLoading(false));
91+
.finally(() => {
92+
if (isRequestStale()) return;
93+
setLoading(false);
94+
});
8695
} else {
8796
setError(undefined);
8897
setValue(response);

0 commit comments

Comments
 (0)