diff --git a/ui/src/Components/MainModal/index.stories.js b/ui/src/Components/MainModal/index.stories.js index 41736744a..24905927b 100644 --- a/ui/src/Components/MainModal/index.stories.js +++ b/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 ( diff --git a/ui/src/Components/SilenceModal/Browser/index.js b/ui/src/Components/SilenceModal/Browser/index.js index e933dd78b..316f5cf74 100644 --- a/ui/src/Components/SilenceModal/Browser/index.js +++ b/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 - {error ? ( - - ) : response === null && isLoading ? ( + {response === null && isLoading ? ( } + content={ + + } /> + ) : error ? ( + ) : response.length === 0 ? ( ) : ( diff --git a/ui/src/Components/SilenceModal/Browser/index.test.js b/ui/src/Components/SilenceModal/Browser/index.test.js index 24c9d84f5..2a39f88d9 100644 --- a/ui/src/Components/SilenceModal/Browser/index.test.js +++ b/ui/src/Components/SilenceModal/Browser/index.test.js @@ -174,6 +174,18 @@ describe("", () => { 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: [], diff --git a/ui/src/Components/SilenceModal/index.stories.js b/ui/src/Components/SilenceModal/index.stories.js index 1de35e029..7cc920602 100644 --- a/ui/src/Components/SilenceModal/index.stories.js +++ b/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 ( diff --git a/ui/src/Hooks/useFetchGet.js b/ui/src/Hooks/useFetchGet.js index 80bf66c87..f39f98119 100644 --- a/ui/src/Hooks/useFetchGet.js +++ b/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); diff --git a/ui/src/Hooks/useFetchGet.test.js b/ui/src/Hooks/useFetchGet.test.js index d2e8b0de8..195285f02 100644 --- a/ui/src/Hooks/useFetchGet.test.js +++ b/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,