fix(ui): fix error handling

ui/src/Components/MainModal/index.stories.js

@@ -2,6 +2,8 @@ import React from "react";
 
 import { storiesOf } from "@storybook/react";
 
+import fetchMock from "fetch-mock";
+
 import { AlertStore } from "Stores/AlertStore";
 import { Settings } from "Stores/Settings";
 import { MainModalContent, TabNames } from "./MainModalContent";
@@ -20,6 +22,18 @@ storiesOf("MainModal", module)
     const alertStore = new AlertStore([]);
     const settingsStore = new Settings();
 
+    fetchMock.mock(
+      "begin:/label",
+      {
+        status: 200,
+        body: JSON.stringify([]),
+        headers: { "Content-Type": "application/json" },
+      },
+      {
+        overwriteRoutes: true,
+      }
+    );
+
     alertStore.info.authentication.enabled = true;
     alertStore.info.authentication.username = "me@example.com";
     return (

ui/src/Components/SilenceModal/Browser/index.js

@@ -63,7 +63,12 @@ const Browser = ({
 
   const debouncedSearchTerm = useDebounce(searchTerm, 500);
 
-  const { response, error, isLoading } = useFetchGet(
+  const {
+    response,
+    error,
+    isLoading,
+    isRetrying,
+  } = useFetchGet(
     FormatBackendURI(
       `silences.json?sortReverse=${sortReverse ? "1" : "0"}&showExpired=${
         showExpired ? "1" : "0"
@@ -130,12 +135,19 @@ const Browser = ({
           Sort order
         </button>
       </div>
-      {error ? (
-        <FetchError message={error} />
-      ) : response === null && isLoading ? (
+      {response === null && isLoading ? (
         <Placeholder
-          content={<FontAwesomeIcon icon={faSpinner} size="lg" spin />}
+          content={
+            <FontAwesomeIcon
+              icon={faSpinner}
+              size="lg"
+              spin
+              className={isRetrying ? "text-danger" : ""}
+            />
+          }
         />
+      ) : error ? (
+        <FetchError message={error} />
       ) : response.length === 0 ? (
         <Placeholder content="Nothing to show" />
       ) : (

ui/src/Components/SilenceModal/Browser/index.test.js

@@ -174,6 +174,18 @@ describe("<Browser />", () => {
     expect(toDiffableHtml(tree.html())).toMatch(/fa-spinner/);
   });
 
+  it("renders loading placeholder before fetch finishes", () => {
+    useFetchGet.mockReturnValue({
+      response: null,
+      error: false,
+      isLoading: true,
+      isRetrying: true,
+    });
+    const tree = MountedBrowser();
+    expect(tree.find("Placeholder")).toHaveLength(1);
+    expect(toDiffableHtml(tree.html())).toMatch(/fa-spinner .+ text-danger/);
+  });
+
   it("renders empty placeholder after fetch with zero results", () => {
     useFetchGet.mockReturnValue({
       response: [],

ui/src/Components/SilenceModal/index.stories.js

@@ -266,9 +266,17 @@ storiesOf("SilenceModal", module)
       clusters: { am: ["am1"] },
     };
 
-    fetchMock.mock("begin:/silences.json?", [], {
-      overwriteRoutes: true,
-    });
+    fetchMock.mock(
+      "begin:/silences.json?",
+      {
+        status: 200,
+        body: JSON.stringify([]),
+        headers: { "Content-Type": "application/json" },
+      },
+      {
+        overwriteRoutes: true,
+      }
+    );
 
     return (
       <Modal>

ui/src/Hooks/useFetchGet.js

@@ -39,17 +39,27 @@ const useFetchGet = (uri, { autorun = true, deps = [] } = {}) => {
             if (!isCanceled.current) {
               setIsRetrying(true);
               setRetryCount(number);
+              return retry(err);
             }
-            return retry(err);
           }),
         FetchRetryConfig
       );
-      const json = await res.json();
-      if (!isCanceled.current) {
-        setResponse(json);
-        setIsLoading(false);
-        setIsRetrying(false);
+
+      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);

ui/src/Hooks/useFetchGet.test.js

@@ -146,6 +146,64 @@ describe("useFetchGet", () => {
     expect(result.current.isRetrying).toBe(false);
   });
 
+  it("error is updated after 500 response with JSON body", async () => {
+    fetchMock.mock("http://localhost/500/json", {
+      status: 500,
+      headers: { "Content-Type": "application/json" },
+      body: JSON.stringify({ status: "error" }),
+    });
+
+    const { result, waitForNextUpdate } = renderHook(() =>
+      useFetchGet("http://localhost/500/json")
+    );
+
+    await waitForNextUpdate();
+
+    expect(result.current.response).toBe(null);
+    expect(result.current.error).toMatchObject({ status: "error" });
+    expect(result.current.isLoading).toBe(false);
+    expect(result.current.isRetrying).toBe(false);
+  });
+
+  it("error is updated after 500 response with plain body", async () => {
+    fetchMock.mock("http://localhost/500/text", {
+      status: 500,
+      body: "error",
+    });
+
+    const { result, waitForNextUpdate } = renderHook(() =>
+      useFetchGet("http://localhost/500/text")
+    );
+
+    await waitForNextUpdate();
+
+    expect(result.current.response).toBe(null);
+    expect(result.current.error).toBe("error");
+    expect(result.current.isLoading).toBe(false);
+    expect(result.current.isRetrying).toBe(false);
+  });
+
+  it("error is updated after failed fetch", async () => {
+    const { result, waitForNextUpdate } = renderHook(() =>
+      useFetchGet("http://localhost/error")
+    );
+
+    expect(result.current.response).toBe(null);
+    expect(result.current.error).toBe(null);
+    expect(result.current.isLoading).toBe(true);
+    expect(result.current.isRetrying).toBe(false);
+
+    for (let i = 0; i <= FetchRetryConfig.retries; i++) {
+      jest.runOnlyPendingTimers();
+      await waitForNextUpdate();
+    }
+
+    expect(result.current.response).toBe(null);
+    expect(result.current.error).toBe("failed to fetch");
+    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,