From 80160baaf7878b39116765bd475f472f0de439a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= <l.mierzwa@gmail.com>
Date: Mon, 18 May 2020 11:03:01 +0100
Subject: [PATCH] fix(ui): fix cleanup in useFetchGet hook

Don't try to set the response if request was canceled
---
 ui/src/Hooks/useFetchGet.js      | 34 +++++++-------
 ui/src/Hooks/useFetchGet.test.js | 79 ++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 17 deletions(-)

diff --git a/ui/src/Hooks/useFetchGet.js b/ui/src/Hooks/useFetchGet.js
index f39f98119..71cd8856c 100644
--- a/ui/src/Hooks/useFetchGet.js
+++ b/ui/src/Hooks/useFetchGet.js
@@ -45,27 +45,27 @@ const useFetchGet = (uri, { autorun = true, deps = [] } = {}) => {
         FetchRetryConfig
       );
 
-      let body;
-      const contentType = res.headers.get("content-type");
-      if (contentType && contentType.indexOf("application/json") !== -1) {
-        body = await res.json();
-      } else {
-        body = await res.text();
-      }
-
-      if (res.ok) {
-        setResponse(body);
-      } else {
-        setError(body);
-      }
-      setIsLoading(false);
-      setIsRetrying(false);
-    } catch (error) {
       if (!isCanceled.current) {
-        setError(error.message);
+        let body;
+        const contentType = res.headers.get("content-type");
+        if (contentType && contentType.indexOf("application/json") !== -1) {
+          body = await res.json();
+        } else {
+          body = await res.text();
+        }
+
+        if (res.ok) {
+          setResponse(body);
+        } else {
+          setError(body);
+        }
         setIsLoading(false);
         setIsRetrying(false);
       }
+    } catch (error) {
+      setError(error.message);
+      setIsLoading(false);
+      setIsRetrying(false);
     }
   }, [uri]);
 
diff --git a/ui/src/Hooks/useFetchGet.test.js b/ui/src/Hooks/useFetchGet.test.js
index 195285f02..8fc8f749f 100644
--- a/ui/src/Hooks/useFetchGet.test.js
+++ b/ui/src/Hooks/useFetchGet.test.js
@@ -204,6 +204,32 @@ describe("useFetchGet", () => {
     expect(result.current.isRetrying).toBe(false);
   });
 
+  it("error is updated on uparsable JSON", async () => {
+    fetchMock.mock("http://localhost/json/invalid", {
+      headers: { "Content-Type": "application/json" },
+      body: "this is not a valid JSON body",
+    });
+
+    const { result, waitForNextUpdate } = renderHook(() =>
+      useFetchGet("http://localhost/json/invalid")
+    );
+
+    expect(result.current.response).toBe(null);
+    expect(result.current.error).toBe(null);
+    expect(result.current.isLoading).toBe(true);
+    expect(result.current.isRetrying).toBe(false);
+
+    jest.runOnlyPendingTimers();
+    await waitForNextUpdate();
+
+    expect(result.current.response).toBe(null);
+    expect(result.current.error).toBe(
+      "invalid json response body at http://localhost/json/invalid reason: Unexpected token h in JSON at position 1"
+    );
+    expect(result.current.isLoading).toBe(false);
+    expect(result.current.isRetrying).toBe(false);
+  });
+
   it("doesn't update response on 200 response after cleanup", async () => {
     fetchMock.mock("http://localhost/slow/ok", {
       delay: 1000,
@@ -288,4 +314,57 @@ describe("useFetchGet", () => {
       await fetchMock.flush(true);
     }
   });
+
+  it("doesn't update error on unparsable JSON after cleanup", async () => {
+    fetchMock.mock("http://localhost/slow/json/invalid", {
+      delay: 1000,
+      headers: { "Content-Type": "application/json" },
+      body: "this is not a valid JSON body",
+    });
+
+    const Component = () => {
+      const { response, error, isLoading } = useFetchGet(
+        "http://localhost/slow/json/invalid"
+      );
+      return (
+        <span>
+          <span>{response}</span>
+          <span>{error}</span>
+          <span>{isLoading}</span>
+        </span>
+      );
+    };
+
+    const tree = mount(<Component />);
+    tree.unmount();
+
+    jest.runOnlyPendingTimers();
+    await fetchMock.flush(true);
+  });
+
+  it("doesn't update response after cleanup", async () => {
+    fetchMock.mock("http://localhost/slow/text", {
+      delay: 1000,
+      body: "ok",
+    });
+
+    const Component = () => {
+      const { response, error, isLoading } = useFetchGet(
+        "http://localhost/slow/text"
+      );
+      return (
+        <span>
+          <span>{response}</span>
+          <span>{error}</span>
+          <span>{isLoading}</span>
+        </span>
+      );
+    };
+
+    const tree = mount(<Component />);
+    tree.unmount();
+
+    jest.runOnlyPendingTimers();
+    await fetchMock.flush(true);
+  });
 });