feat(fe): abort outdated requests (#1317)

* feat: abort outdated requests

* test: abort on Data Fetch

* feat: use the aborted status instead of timestamps

* feat: remove unused variable

* test: add mocks

* feat: reset some values on request finished

* test: improve mock

* Revert "feat: reset some values on request finished"

This reverts commit b1eb250.

* test: improve mock further

frontend/src/components/DataFetcher.vue

@@ -25,7 +25,6 @@ const content = ref<any>(props.initValue);
 const response = ref<any>();
 const loading = ref<boolean>();
 
-const lastUpdateRequestTime = ref<number>(0);
 let debounceTimer: NodeJS.Timeout | null = null;
 
 const initialUrlValue = props.url;
@@ -48,33 +47,59 @@ const calculateStringDifference = (
 
 // If initial fetch is required, fetch
 if (!props.disabled && props.initFetch) {
-  fetch().then(() => {
+  fetch().asyncResponse.then(() => {
     content.value = response.value;
   });
 }
 
+const controllerList: Record<string, AbortController> = {};
+
+const abortOutdatedRequests = () => {
+  Object.values(controllerList).forEach((controller) => {
+    controller.abort();
+  });
+};
+
 // Watch for changes in the url, and if the difference is greater than the min length, fetch
 watch([() => props.url, () => props.disabled], () => {
   if (!props.disabled && calculateStringDifference(initialUrlValue, props.url) >= props.minLength) {
 
     // added a manual loading state to set the loading state when the user types
     loading.value = true;
     const curRequestTime = Date.now();
-    lastUpdateRequestTime.value = curRequestTime;
 
     if (debounceTimer) {
       clearTimeout(debounceTimer);
     }
     debounceTimer = setTimeout(() => {
       content.value = [];
-      fetch().then(() => {
-        // Discard the response from old request when a newer request has been made.
-        if (curRequestTime === lastUpdateRequestTime.value) {
-          loading.value = false;
-          content.value = response.value;
-        }
-      });
-  }, props.debounce); // Debounce time
+      const { asyncResponse, controller } = fetch();
+      abortOutdatedRequests();
+      controllerList[curRequestTime] = controller;
+      asyncResponse
+        .then(() => {
+          // Disregard aborted requests
+          /*
+          Note: the asyncResponse of an aborted request is not rejected due to the error handling
+          performed by useFetchTo.
+          */
+          if (!controller.signal.aborted) {
+            content.value = response.value;
+          }
+        })
+        .finally(() => {
+          // Disregard aborted requests
+          if (!controller.signal.aborted) {
+            loading.value = false;
+          }
+
+          /*
+          At this point the request has been either completed or aborted.
+          So it's time to remove its controller from the list
+          */
+          delete controllerList[curRequestTime];
+        });
+    }, props.debounce); // Debounce time
   }
 });
 </script>

frontend/src/composables/useFetch.ts

@@ -66,26 +66,36 @@ export const useFetchTo = (
     },
   };
 
-  const fetch = async () => {
+  const fetch = () => {
     loading.value = true;
     const actualURL = typeof url === "string" ? url : url.value;
-    try {
-      const result = await axios.request({
+    const controller = new AbortController();
+    const asyncResponse = axios
+      .request({
         ...parameters,
         url: actualURL,
         baseURL: backendUrl,
+        signal: controller.signal,
+      })
+      .then((result) => {
+        response.value = result;
+        data.value = result.data;
+      })
+      .catch((ex) => {
+        error.value = ex;
+        if (config.skipDefaultErrorHandling) {
+          return;
+        }
+        apiDataHandler.handleErrorDefault();
+      })
+      .finally(() => {
+        loading.value = false;
       });
-      response.value = result;
-      data.value = result.data;
-    } catch (ex) {
-      error.value = ex;
-      if (config.skipDefaultErrorHandling) {
-        return;
-      }
-      apiDataHandler.handleErrorDefault();
-    } finally {
-      loading.value = false;
-    }
+
+    return {
+      asyncResponse,
+      controller,
+    };
   };
 
   !config.skip && fetch();

frontend/tests/mocks/MockAbortController.ts

@@ -0,0 +1,10 @@
+import MockAbortSignal from "./MockAbortSignal";
+
+export default class MockAbortController {
+  signal = new MockAbortSignal();
+
+  abort(): void {
+    this.signal.dispatchEvent(new Event("abort"));
+    this.signal.aborted = true;
+  }
+}

frontend/tests/mocks/MockAbortSignal.ts

@@ -0,0 +1,3 @@
+export default class MockAbortSignal extends EventTarget {
+  aborted = false;
+}

frontend/tests/unittests/components/DataFetcher.spec.ts

@@ -4,38 +4,61 @@ import { nextTick, ref, type Ref } from "vue";
 import * as fetcher from "@/composables/useFetch";
 
 import DataFetcher from "@/components/DataFetcher.vue";
+import MockAbortController from "../../mocks/MockAbortController";
 
 vi.useFakeTimers();
 
-
-
 describe("DataFetcher", () => {
+  vi.spyOn(global, "AbortController").mockImplementation(
+    () => new MockAbortController() as AbortController,
+  );
+
   const mockedFetchTo = (url: Ref<string>, received: Ref<any>, config: any = {}) => ({
     response: ref({}),
     error: ref({}),
     data: received,
     loading: ref(false),
-    fetch: async () => {
-      received.value = { name: "Loaded" };
+    fetch: () => {
+      const controller = new AbortController();
+      const data = { name: "Loaded" };
+      received.value = data;
+      return {
+        asyncResponse: Promise.resolve(data),
+        controller,
+      };
     },
   });
 
   const mockedFetchToFunction =
     (
-      fetchData: (url: string) => Promise<any> = async () => ({
+      fetchData: (url: string, signal: AbortSignal) => Promise<any> = async () => ({
         name: "Loaded",
       }),
     ) =>
-    (url: Ref<string>, received: Ref<any>, config: any = {}) => ({
-      response: ref({}),
-      error: ref({}),
-      data: received,
-      loading: ref(false),
-      fetch: async () => {
-        received.value = await fetchData(url.value);
-        console.log(received.value);
-      },
-    });
+    (url: Ref<string>, received: Ref<any>, config: any = {}) => {
+      const error = ref({});
+      const response = ref({});
+      return {
+        response,
+        error,
+        data: received,
+        loading: ref(false),
+        fetch: () => {
+          const controller = new AbortController();
+          const asyncResponse = fetchData(url.value, controller.signal).catch((ex) => {
+            error.value = ex;
+          });
+          asyncResponse.then((data) => {
+            received.value = data;
+            response.value = { data };
+          });
+          return {
+            asyncResponse,
+            controller,
+          };
+        },
+      };
+    };
 
   const simpleFetchData = async (url: string) => {
     return new Promise((resolve) => {
@@ -47,27 +70,53 @@ describe("DataFetcher", () => {
 
   const mockFetchSimple = mockedFetchToFunction(simpleFetchData);
 
-  let lastResponse: any;
+  let lastResult: {
+    url: string;
+    response?: any;
+    error?: any;
+  };
+
+  const abortErrorMessage = "sample abort error message";
 
-  const fetchDataSleepByParam = async (url: string) => {
+  const fetchDataSleepByParam = (url: string, signal: AbortSignal) => {
     const regex = /.*\/(.+)/;
     const regexResult = regex.exec(url);
     const lastParam = regexResult[1];
     const time = parseInt(lastParam);
 
-    return new Promise((resolve) => {
-      setTimeout(() => {
+    return new Promise((resolve, reject) => {
+      const timeoutId = setTimeout(() => {
         const response = { name: url };
         resolve(response);
 
         // Just for checking on the tests.
-        lastResponse = response;
+        lastResult = {
+          url,
+          response,
+        };
       }, time);
+
+      signal.addEventListener("abort", () => {
+        clearTimeout(timeoutId);
+        const error = new Error(abortErrorMessage);
+        reject(error);
+
+        // Just for checking on the tests.
+        lastResult = {
+          url,
+          error,
+        };
+      });
     });
   };
 
   const mockFetchSleepByParam = mockedFetchToFunction(fetchDataSleepByParam);
 
+  beforeEach(() => {
+    // reset variable
+    lastResult = undefined;
+  });
+
   it("should render", () => {
     const wrapper = mount(DataFetcher, {
       props: {
@@ -314,10 +363,14 @@ describe("DataFetcher", () => {
         // More time, enough to get the response from /api/one/1000
         await vi.advanceTimersByTimeAsync(600);
 
-        // It was effectively responded
-        expect(lastResponse?.name).toEqual("/api/one/1000");
+        // It was effectively requested
+        expect(lastResult.url).toEqual("/api/one/1000");
+
+        // But it was aborted
+        expect(lastResult.error).toStrictEqual(new Error(abortErrorMessage));
+        expect(lastResult.response).toBeUndefined();
 
-        // It should remain empty regardless
+        // So it should remain empty
         expect(wrapper.find("div").text()).toBe("slot content is");
 
         // More time, enough to get the response from /api/two/1000
@@ -338,18 +391,24 @@ describe("DataFetcher", () => {
       });
 
       it("should discard the response from the first request", async () => {
+        await vi.waitFor(() => {
+          // api/one/1000 had been effectively requested
+          expect(lastResult.url).toEqual("/api/one/1000");
+        });
+
+        // But it has been aborted
+        expect(lastResult.error).toStrictEqual(new Error(abortErrorMessage));
+        expect(lastResult.response).toBeUndefined();
+
         // Enough to get only the response from /api/two/500
         await vi.advanceTimersByTimeAsync(600);
 
         expect(wrapper.find("div").text()).toBe("slot content is /api/two/500");
 
-        // More time, enough to get the response from /api/one/1000
+        // More time, enough to get the response from /api/one/1000, if it had not been aborted
         await vi.advanceTimersByTimeAsync(310);
 
-        // It was effectively responded
-        expect(lastResponse?.name).toEqual("/api/one/1000");
-
-        // But it should keep the current value
+        // So it should keep the current value
         expect(wrapper.find("div").text()).toBe("slot content is /api/two/500");
       });
     });