From 325482bd5ed0523dd6ce50d52e526fb4af4f7283 Mon Sep 17 00:00:00 2001
From: Joe Reuter <johannes.reuter@elastic.co>
Date: Tue, 21 Jan 2025 18:45:43 +0100
Subject: [PATCH] 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.

(cherry picked from commit 8ff18e25758a05d4deff76ffa4b3407d98722a3c)
---
 .../utils_browser/hooks/use_abortable_async.ts        | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/x-pack/solutions/observability/packages/utils_browser/hooks/use_abortable_async.ts b/x-pack/solutions/observability/packages/utils_browser/hooks/use_abortable_async.ts
index f0d2bf4a05872..17dfde0736477 100644
--- a/x-pack/solutions/observability/packages/utils_browser/hooks/use_abortable_async.ts
+++ b/x-pack/solutions/observability/packages/utils_browser/hooks/use_abortable_async.ts
@@ -58,12 +58,17 @@ export function useAbortableAsync<T>(
     const controller = new AbortController();
     controllerRef.current = controller;
 
+    function isRequestStale() {
+      return controllerRef.current !== controller;
+    }
+
     if (clearValueOnNext) {
       setValue(undefined);
       setError(undefined);
     }
 
     function handleError(err: Error) {
+      if (isRequestStale()) return;
       setError(err);
       if (unsetValueOnError) {
         setValue(undefined);
@@ -78,11 +83,15 @@ export function useAbortableAsync<T>(
         setLoading(true);
         response
           .then((nextValue) => {
+            if (isRequestStale()) return;
             setError(undefined);
             setValue(nextValue);
           })
           .catch(handleError)
-          .finally(() => setLoading(false));
+          .finally(() => {
+            if (isRequestStale()) return;
+            setLoading(false);
+          });
       } else {
         setError(undefined);
         setValue(response);