From 54550bacf92bedd585e98ec386083981862c48da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 21 Dec 2019 18:33:10 +0000 Subject: [PATCH 1/2] feat(ui): try to detect if API requests are intercepted by an auth proxy --- ui/src/Common/Fetch.js | 22 +++++++++++++++++----- ui/src/Common/Fetch.test.js | 22 +++++++++++++++++++++- ui/src/Stores/AlertStore.js | 17 +++++++++++++++-- ui/src/Stores/AlertStore.test.js | 28 +++++++++++++++++++++++----- 4 files changed, 76 insertions(+), 13 deletions(-) diff --git a/ui/src/Common/Fetch.js b/ui/src/Common/Fetch.js index fb09d84c1..625016fce 100644 --- a/ui/src/Common/Fetch.js +++ b/ui/src/Common/Fetch.js @@ -8,17 +8,29 @@ const CommonOptions = { }; const FetchRetryConfig = { - retries: 5, - minTimeout: 1000, + retries: 10, + minTimeout: 2000, maxTimeout: 5000 }; const FetchGet = async (uri, options, retryOptions) => await promiseRetry( (retry, number) => - fetch(uri, merge({}, { method: "GET" }, CommonOptions, options)).catch( - retry - ), + fetch( + uri, + merge( + {}, + { + method: "GET", + mode: + number <= Math.round(FetchRetryConfig.retries * 0.8) + ? "cors" + : "no-cors" + }, + CommonOptions, + options + ) + ).catch(retry), FetchRetryConfig ); diff --git a/ui/src/Common/Fetch.test.js b/ui/src/Common/Fetch.test.js index d31126888..0d777b6d6 100644 --- a/ui/src/Common/Fetch.test.js +++ b/ui/src/Common/Fetch.test.js @@ -18,7 +18,7 @@ describe("Fetch", () => { }; const methodOptions = { - FetchGet: { method: "GET" }, + FetchGet: { method: "GET", mode: "cors" }, FetchPost: { method: "POST" }, FetchDelete: { method: "DELETE" } }; @@ -59,4 +59,24 @@ describe("Fetch", () => { ); }); } + + it("FetchGet switches to no-cors after 80% failures", async () => { + fetch.mockReject(new Error("Fetch error")); + + const request = FetchGet("http://example.com", {}); + await expect(request).rejects.toThrow("Fetch error"); + + expect(fetch).toHaveBeenCalledTimes(11); + expect(fetch).toHaveBeenCalledWith("http://example.com", { + method: "GET", + credentials: "include", + mode: "no-cors", + redirect: "follow" + }); + for (let i = 0; i <= 10; i++) { + expect(fetch.mock.calls[i][1]).toMatchObject({ + mode: i < 8 ? "cors" : "no-cors" + }); + } + }); }); diff --git a/ui/src/Stores/AlertStore.js b/ui/src/Stores/AlertStore.js index 8b57f8520..29d0eed10 100644 --- a/ui/src/Stores/AlertStore.js +++ b/ui/src/Stores/AlertStore.js @@ -174,9 +174,15 @@ class AlertStore { { totalAlerts: 0, version: "unknown", - upgradeNeeded: false + upgradeNeeded: false, + reloadNeeded: false, + setReloadNeeded() { + this.reloadNeeded = true; + } + }, + { + setReloadNeeded: action }, - {}, { name: "API response info" } ); @@ -270,6 +276,13 @@ class AlertStore { return FetchGet(alertsURI, {}) .then(result => { + // we're sending requests with mode=cors so the response should also be type=cors + // after a few failures in the retry loop we will switch to no-cors + // if that request comes back as type=opaque then we might be getting + // redirected by an auth proxy + if (result.type === "opaque") { + this.info.setReloadNeeded(); + } this.status.setProcessing(); return result.json(); }) diff --git a/ui/src/Stores/AlertStore.test.js b/ui/src/Stores/AlertStore.test.js index aa7792d0c..d1e2d3522 100644 --- a/ui/src/Stores/AlertStore.test.js +++ b/ui/src/Stores/AlertStore.test.js @@ -375,7 +375,7 @@ describe("AlertStore.fetch", () => { const store = new AlertStore([]); await expect(store.fetch()).resolves.toHaveProperty("error"); - expect(global.fetch).toHaveBeenCalledTimes(6); + expect(global.fetch).toHaveBeenCalledTimes(11); expect(store.status.value).toEqual(AlertStoreStatuses.Failure); expect(store.info.version).toBe("unknown"); // there should be a trace of the error @@ -388,7 +388,7 @@ describe("AlertStore.fetch", () => { fetch.mockReject(new Error("Fetch error")); await expect(store.fetch()).resolves.toHaveProperty("error"); - expect(global.fetch).toHaveBeenCalledTimes(6); + expect(global.fetch).toHaveBeenCalledTimes(11); }); it("fetch() retry counter is reset after successful fetch", async () => { @@ -397,16 +397,34 @@ describe("AlertStore.fetch", () => { fetch.mockReject(new Error("Fetch error")); await expect(store.fetch()).resolves.toHaveProperty("error"); - expect(global.fetch).toHaveBeenCalledTimes(6); + expect(global.fetch).toHaveBeenCalledTimes(11); const response = EmptyAPIResponse(); fetch.mockResponse(JSON.stringify(response)); await expect(store.fetch()).resolves.toBeUndefined(); - expect(global.fetch).toHaveBeenCalledTimes(7); + expect(global.fetch).toHaveBeenCalledTimes(12); fetch.mockReject(new Error("Fetch error")); await expect(store.fetch()).resolves.toHaveProperty("error"); - expect(global.fetch).toHaveBeenCalledTimes(13); + expect(global.fetch).toHaveBeenCalledTimes(23); + }); + + it("fetch() reloads the page after if auth middleware is detected", async () => { + jest.spyOn(console, "trace").mockImplementation(() => {}); + + const store = new AlertStore(["label=value"]); + + jest.spyOn(global, "fetch").mockImplementation(async () => + Promise.resolve({ + type: "opaque", + body: "auth needed", + json: jest.fn(() => EmptyAPIResponse()) + }) + ); + + await expect(store.fetch()).resolves.toBeUndefined(); + + expect(store.info.reloadNeeded).toBe(true); }); it("unapplied filters are marked as applied on fetch error", async () => { From 59470daffbeb89b9a12899f93048148651b71dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 21 Dec 2019 18:33:38 +0000 Subject: [PATCH 2/2] feat(ui): add a component to reload the page if needed --- .../__snapshots__/index.test.js.snap | 41 ++++++++++++++++ ui/src/Components/Grid/ReloadNeeded/index.js | 48 +++++++++++++++++++ .../Grid/ReloadNeeded/index.test.js | 43 +++++++++++++++++ .../__snapshots__/index.test.js.snap | 2 +- ui/src/Components/Grid/UpgradeNeeded/index.js | 2 +- ui/src/Components/Grid/index.js | 13 +++-- ui/src/Components/Grid/index.stories.js | 4 ++ ui/src/Components/Grid/index.test.js | 6 +++ .../NavBar/FilterInput/index.test.js | 2 +- .../SilenceModal/SilencePreview/index.test.js | 2 + 10 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 ui/src/Components/Grid/ReloadNeeded/__snapshots__/index.test.js.snap create mode 100644 ui/src/Components/Grid/ReloadNeeded/index.js create mode 100644 ui/src/Components/Grid/ReloadNeeded/index.test.js diff --git a/ui/src/Components/Grid/ReloadNeeded/__snapshots__/index.test.js.snap b/ui/src/Components/Grid/ReloadNeeded/__snapshots__/index.test.js.snap new file mode 100644 index 000000000..74e31e607 --- /dev/null +++ b/ui/src/Components/Grid/ReloadNeeded/__snapshots__/index.test.js.snap @@ -0,0 +1,41 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` matches snapshot 1`] = ` +" +

+
+ + + + +

+ + + + + All API connection attempts failed. This migth be caused by authentication middleware, will try to reload. +

+
+

+" +`; diff --git a/ui/src/Components/Grid/ReloadNeeded/index.js b/ui/src/Components/Grid/ReloadNeeded/index.js new file mode 100644 index 000000000..1b1539792 --- /dev/null +++ b/ui/src/Components/Grid/ReloadNeeded/index.js @@ -0,0 +1,48 @@ +import React, { Component } from "react"; +import PropTypes from "prop-types"; + +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; +import { faExclamationCircle } from "@fortawesome/free-solid-svg-icons/faExclamationCircle"; +import { faSpinner } from "@fortawesome/free-solid-svg-icons/faSpinner"; + +import { CenteredMessage } from "Components/CenteredMessage"; + +class ReloadNeeded extends Component { + static propTypes = { + reloadAfter: PropTypes.number.isRequired + }; + + reloadApp = () => { + window.location.reload(); + }; + + componentDidMount() { + const { reloadAfter } = this.props; + this.timer = setTimeout(this.reloadApp, reloadAfter); + } + + componentWillUnmount() { + clearTimeout(this.timer); + this.timer = null; + } + + render() { + return ( + +
+ +

+ + All API connection attempts failed. This migth be caused by + authentication middleware, will try to reload. +

+
+ + ); + } +} + +export { ReloadNeeded }; diff --git a/ui/src/Components/Grid/ReloadNeeded/index.test.js b/ui/src/Components/Grid/ReloadNeeded/index.test.js new file mode 100644 index 000000000..91fdc7c5a --- /dev/null +++ b/ui/src/Components/Grid/ReloadNeeded/index.test.js @@ -0,0 +1,43 @@ +import React from "react"; + +import { mount, shallow } from "enzyme"; + +import toDiffableHtml from "diffable-html"; + +import { ReloadNeeded } from "."; + +beforeEach(() => { + jest.useFakeTimers(); + jest.clearAllTimers(); +}); + +afterEach(() => { + jest.clearAllTimers(); + jest.restoreAllMocks(); +}); + +describe("", () => { + it("matches snapshot", () => { + const tree = shallow( + + ); + expect(toDiffableHtml(tree.html())).toMatchSnapshot(); + }); + + it("calls window.location.reload after timer is done", () => { + const reloadSpy = jest + .spyOn(global.window.location, "reload") + .mockImplementation(() => {}); + mount(); + jest.runOnlyPendingTimers(); + expect(reloadSpy).toBeCalled(); + }); + + it("timer is cleared on unmount", () => { + const tree = mount(); + const instance = tree.instance(); + + instance.componentWillUnmount(); + expect(instance.timer).toBeNull(); + }); +}); diff --git a/ui/src/Components/Grid/UpgradeNeeded/__snapshots__/index.test.js.snap b/ui/src/Components/Grid/UpgradeNeeded/__snapshots__/index.test.js.snap index 6a55d3cea..1948febd8 100644 --- a/ui/src/Components/Grid/UpgradeNeeded/__snapshots__/index.test.js.snap +++ b/ui/src/Components/Grid/UpgradeNeeded/__snapshots__/index.test.js.snap @@ -25,7 +25,7 @@ exports[` matches snapshot 1`] = ` focusable=\\"false\\" data-prefix=\\"fas\\" data-icon=\\"spinner\\" - class=\\"svg-inline--fa fa-spinner fa-w-16 fa-spin mr-1\\" + class=\\"svg-inline--fa fa-spinner fa-w-16 fa-spin mr-2\\" role=\\"img\\" xmlns=\\"http://www.w3.org/2000/svg\\" viewbox=\\"0 0 512 512\\" diff --git a/ui/src/Components/Grid/UpgradeNeeded/index.js b/ui/src/Components/Grid/UpgradeNeeded/index.js index f2ebe92a1..768b812e3 100644 --- a/ui/src/Components/Grid/UpgradeNeeded/index.js +++ b/ui/src/Components/Grid/UpgradeNeeded/index.js @@ -41,7 +41,7 @@ class UpgradeNeeded extends Component { />

- + Upgrading to a new version: {newVersion}

diff --git a/ui/src/Components/Grid/index.js b/ui/src/Components/Grid/index.js index 83818de40..77cf3c233 100644 --- a/ui/src/Components/Grid/index.js +++ b/ui/src/Components/Grid/index.js @@ -10,6 +10,7 @@ import { AlertGrid } from "./AlertGrid"; import { FatalError } from "./FatalError"; import { UpstreamError } from "./UpstreamError"; import { UpgradeNeeded } from "./UpgradeNeeded"; +import { ReloadNeeded } from "./ReloadNeeded"; import { EmptyGrid } from "./EmptyGrid"; const Grid = observer( @@ -23,10 +24,6 @@ const Grid = observer( render() { const { alertStore, settingsStore, silenceFormStore } = this.props; - if (alertStore.status.error) { - return ; - } - if (alertStore.info.upgradeNeeded) { return ( ; + } + + if (alertStore.status.error) { + return ; + } + if ( alertStore.data.upstreams.counters && alertStore.data.upstreams.counters.total === 1 && diff --git a/ui/src/Components/Grid/index.stories.js b/ui/src/Components/Grid/index.stories.js index d5ec644e1..e4f977ad7 100644 --- a/ui/src/Components/Grid/index.stories.js +++ b/ui/src/Components/Grid/index.stories.js @@ -8,6 +8,7 @@ import { Settings } from "Stores/Settings"; import { SilenceFormStore } from "Stores/SilenceFormStore"; import { FatalError } from "./FatalError"; import { UpgradeNeeded } from "./UpgradeNeeded"; +import { ReloadNeeded } from "./ReloadNeeded"; import { EmptyGrid } from "./EmptyGrid"; import { Grid } from "."; import { InternalError } from "../../ErrorBoundary"; @@ -32,6 +33,9 @@ storiesOf("Grid", module) .add("UpgradeNeeded", () => { return ; }) + .add("ReloadNeeded", () => { + return ; + }) .add("EmptyGrid", () => { return (
diff --git a/ui/src/Components/Grid/index.test.js b/ui/src/Components/Grid/index.test.js index 49503e643..722eb8545 100644 --- a/ui/src/Components/Grid/index.test.js +++ b/ui/src/Components/Grid/index.test.js @@ -94,6 +94,12 @@ describe("", () => { expect(tree.text()).toBe(""); }); + it("renders ReloadNeeded when alertStore.info.reloadNeeded=true", () => { + alertStore.info.reloadNeeded = true; + const tree = ShallowGrid(); + expect(tree.text()).toBe(""); + }); + it("renders AlertGrid before any fetch finished when totalAlerts is 0", () => { alertStore.info.version = "unknown"; alertStore.info.totalAlerts = 0; diff --git a/ui/src/Components/NavBar/FilterInput/index.test.js b/ui/src/Components/NavBar/FilterInput/index.test.js index d5a15c2ad..b8f831c9e 100644 --- a/ui/src/Components/NavBar/FilterInput/index.test.js +++ b/ui/src/Components/NavBar/FilterInput/index.test.js @@ -203,7 +203,7 @@ describe("", () => { tree.find("input").simulate("change", { target: { value: "bar" } }); await WaitForFetch(tree); - expect(fetch.mock.calls).toHaveLength(6); + expect(fetch.mock.calls).toHaveLength(11); expect(fetch.mock.calls[0]).toContain("./autocomplete.json?term=bar"); expect(instance.inputStore.suggestions).toHaveLength(0); }); diff --git a/ui/src/Components/SilenceModal/SilencePreview/index.test.js b/ui/src/Components/SilenceModal/SilencePreview/index.test.js index 18c123d9b..ef907878e 100644 --- a/ui/src/Components/SilenceModal/SilencePreview/index.test.js +++ b/ui/src/Components/SilenceModal/SilencePreview/index.test.js @@ -90,6 +90,7 @@ describe("", () => { { method: "GET", credentials: "include", + mode: "cors", redirect: "follow" } ); @@ -108,6 +109,7 @@ describe("", () => { { method: "GET", credentials: "include", + mode: "cors", redirect: "follow" } );