fix(api): cancelled requests don't emit user facing errors (#1472)

splunk · Nov 21, 2024 · 0970441 · 0970441
1 parent 59c6e32
commit 0970441
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 16 deletions.
diff --git a/ui/src/components/MultiInputComponent/MultiInputComponent.tsx b/ui/src/components/MultiInputComponent/MultiInputComponent.tsx
@@ -78,7 +78,7 @@ function MultiInputComponent(props: MultiInputComponentProps) {
             return;
         }
 
-        let current = true;
+        let mounted = true;
         const abortController = new AbortController();
 
         const url = referenceName
@@ -102,7 +102,7 @@ function MultiInputComponent(props: MultiInputComponentProps) {
             setLoading(true);
             getRequest<{ entry: FilterResponseParams }>(apiCallOptions)
                 .then((data) => {
-                    if (current) {
+                    if (mounted) {
                         setOptions(
                             generateOptions(
                                 filterResponse(
@@ -117,15 +117,15 @@ function MultiInputComponent(props: MultiInputComponentProps) {
                     }
                 })
                 .finally(() => {
-                    if (current) {
+                    if (mounted) {
                         setLoading(false);
                     }
                 });
         }
         // eslint-disable-next-line consistent-return
         return () => {
-            abortController.abort('Operation canceled.');
-            current = false;
+            mounted = false;
+            abortController.abort();
         };
         // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [dependencyValues]);

diff --git a/ui/src/components/SingleInputComponent/SingleInputComponent.tsx b/ui/src/components/SingleInputComponent/SingleInputComponent.tsx
@@ -128,7 +128,7 @@ function SingleInputComponent(props: SingleInputComponentProps) {
             return;
         }
 
-        let current = true;
+        let mounted = true;
         const abortController = new AbortController();
 
         const backendCallOptions = {
@@ -145,7 +145,7 @@ function SingleInputComponent(props: SingleInputComponentProps) {
         setLoading(true);
         getRequest<{ entry: FilterResponseParams }>(backendCallOptions)
             .then((data) => {
-                if (current) {
+                if (mounted) {
                     setOptions(
                         generateOptions(
                             filterResponse(data.entry, labelField, valueField, allowList, denyList)
@@ -155,16 +155,16 @@ function SingleInputComponent(props: SingleInputComponentProps) {
                 }
             })
             .catch(() => {
-                if (current) {
+                if (mounted) {
                     setLoading(false);
+                    setOptions([]);
                 }
-                setOptions([]);
             });
 
         // eslint-disable-next-line consistent-return
         return () => {
-            abortController.abort('Operation canceled.');
-            current = false;
+            mounted = false;
+            abortController.abort();
         };
         // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [dependencyValues]);

diff --git a/ui/src/util/api.test.ts → ui/src/util/api.test.tsx b/ui/src/util/api.test.ts → ui/src/util/api.test.tsx
@@ -1,9 +1,15 @@
-import { http, HttpResponse } from 'msw';
+import { delay, http, HttpResponse } from 'msw';
 import { generateEndPointUrl, getRequest } from './api';
 import { getGlobalConfigMock } from '../mocks/globalConfigMock';
 import { setUnifiedConfig } from './util';
 import { server } from '../mocks/server';
 
+const mockGenerateToastFn = jest.fn();
+jest.mock('./util', () => ({
+    ...jest.requireActual('./util'),
+    generateToast: () => mockGenerateToastFn(),
+}));
+
 describe('generateEndPointUrl', () => {
     it('should return the correct endpoint URL', () => {
         const mockConfig = getGlobalConfigMock();
@@ -23,7 +29,7 @@ describe('generateEndPointUrl', () => {
 });
 
 describe('getRequest', () => {
-    beforeEach(() => {
+    function setup() {
         const mockConfig = getGlobalConfigMock();
         setUnifiedConfig({
             ...mockConfig,
@@ -32,9 +38,12 @@ describe('getRequest', () => {
                 restRoot: 'testing_name',
             },
         });
-        server.use(http.get('*', () => HttpResponse.json({}, { status: 500 })));
-    });
+    }
+
     it('should call callbackOnError if handleError is true', async () => {
+        setup();
+        server.use(http.get('*', () => HttpResponse.json({}, { status: 500 })));
+
         const callbackOnError = jest.fn();
 
         await expect(() =>
@@ -45,9 +54,12 @@ describe('getRequest', () => {
             })
         ).rejects.toThrow();
 
+        expect(mockGenerateToastFn).toHaveBeenCalledTimes(1);
         expect(callbackOnError).toHaveBeenCalled();
     });
     it('should not call callbackOnError if handleError is false', async () => {
+        setup();
+        server.use(http.get('*', () => HttpResponse.json({}, { status: 500 })));
         const callbackOnError = jest.fn();
 
         await expect(() =>
@@ -58,6 +70,30 @@ describe('getRequest', () => {
             })
         ).rejects.toThrow();
 
+        expect(mockGenerateToastFn).not.toHaveBeenCalled();
         expect(callbackOnError).not.toHaveBeenCalled();
     });
+
+    it('should not show error if request is cancelled', async () => {
+        setup();
+        server.use(
+            http.get('*', async () => {
+                await delay('infinite');
+
+                return HttpResponse.json();
+            })
+        );
+        const abortController = new AbortController();
+
+        const request = getRequest({
+            endpointUrl: 'testing_endpoint',
+            handleError: true,
+            signal: abortController.signal,
+        });
+
+        abortController.abort();
+
+        await expect(request).rejects.toThrow();
+        expect(mockGenerateToastFn).not.toHaveBeenCalled();
+    });
 });
diff --git a/ui/src/util/api.ts b/ui/src/util/api.ts
@@ -58,7 +58,8 @@ async function fetchWithErrorHandling<TData>(
         }
         return await response.json();
     } catch (error) {
-        if (handleError) {
+        const isAborted = error instanceof DOMException && error.name === 'AbortError';
+        if (handleError && !isAborted) {
             const errorMsg = parseErrorMsg(error);
             generateToast(errorMsg, 'error');
             if (callbackOnError) {