diff --git a/ui/src/Hooks/useFetchAny.js b/ui/src/Hooks/useFetchAny.js index 841be7ecc..f9725b65f 100644 --- a/ui/src/Hooks/useFetchAny.js +++ b/ui/src/Hooks/useFetchAny.js @@ -4,7 +4,7 @@ import merge from "lodash.merge"; import { CommonOptions } from "Common/Fetch"; -const useFetchAny = (upstreams) => { +const useFetchAny = (upstreams, { fetcher = null } = {}) => { const [index, setIndex] = useState(0); const [response, setResponse] = useState({ response: null, @@ -37,7 +37,7 @@ const useFetchAny = (upstreams) => { inProgress: true, }); try { - const res = await fetch( + const res = await (fetcher || fetch)( uri, merge({}, { method: "GET" }, CommonOptions, options) ); @@ -51,23 +51,25 @@ const useFetchAny = (upstreams) => { body = await res.text(); } - if (res.ok) { - setResponse({ - response: body, - error: null, - responseURI: uri, - inProgress: false, - }); - } else { - if (upstreams.length > index + 1) { - setIndex(index + 1); - } else { + if (!isCancelled) { + if (res.ok) { setResponse({ - response: null, - error: body, - responseURI: null, + response: body, + error: null, + responseURI: uri, inProgress: false, }); + } else { + if (upstreams.length > index + 1) { + setIndex(index + 1); + } else { + setResponse({ + response: null, + error: body, + responseURI: null, + inProgress: false, + }); + } } } } @@ -96,7 +98,7 @@ const useFetchAny = (upstreams) => { return () => { isCancelled = true; }; - }, [upstreams, index, reset]); + }, [upstreams, index, reset, fetcher]); return { ...response, reset }; }; diff --git a/ui/src/Hooks/useFetchAny.test.js b/ui/src/Hooks/useFetchAny.test.js index 46925d40e..a4cd8d866 100644 --- a/ui/src/Hooks/useFetchAny.test.js +++ b/ui/src/Hooks/useFetchAny.test.js @@ -323,4 +323,35 @@ describe("useFetchAny", () => { expect(result.current.inProgress).toBe(false); expect(result.current.responseURI).toBe(null); }); + + it("doesn't update response after cleanup on slow body read", async () => { + let tree; + const fetcher = jest.fn(() => + Promise.resolve({ + headers: { + get: () => "text/plain", + }, + text: async () => { + tree.unmount(); + return "ok"; + }, + }) + ); + + const upstreams = [{ uri: "http://localhost/slow/body", options: {} }]; + const Component = () => { + const { response, error, inProgress } = useFetchAny(upstreams, { + fetcher: fetcher, + }); + return ( + + {response} + {error} + {inProgress} + + ); + }; + + tree = mount(); + }); }); diff --git a/ui/src/Hooks/useFetchGet.js b/ui/src/Hooks/useFetchGet.js index ed99a1c77..17c1c446d 100644 --- a/ui/src/Hooks/useFetchGet.js +++ b/ui/src/Hooks/useFetchGet.js @@ -6,7 +6,10 @@ import promiseRetry from "promise-retry"; import { CommonOptions, FetchRetryConfig } from "Common/Fetch"; -const useFetchGet = (uri, { autorun = true, deps = [] } = {}) => { +const useFetchGet = ( + uri, + { autorun = true, deps = [], fetcher = null } = {} +) => { const [response, setResponse] = useState(null); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(true); @@ -27,7 +30,7 @@ const useFetchGet = (uri, { autorun = true, deps = [] } = {}) => { setError(null); const res = await promiseRetry( (retry, number) => - fetch( + (fetcher || fetch)( uri, merge( {}, @@ -58,20 +61,22 @@ const useFetchGet = (uri, { autorun = true, deps = [] } = {}) => { body = await res.text(); } - if (res.ok) { - setResponse(body); - } else { - setError(body); + if (!isCanceled.current) { + if (res.ok) { + setResponse(body); + } else { + setError(body); + } + setIsLoading(false); + setIsRetrying(false); } - setIsLoading(false); - setIsRetrying(false); } } catch (error) { setError(error.message); setIsLoading(false); setIsRetrying(false); } - }, [uri]); + }, [uri, fetcher]); useEffect(() => { if (autorun) get(); diff --git a/ui/src/Hooks/useFetchGet.test.js b/ui/src/Hooks/useFetchGet.test.js index 8fc8f749f..17cc0ed6e 100644 --- a/ui/src/Hooks/useFetchGet.test.js +++ b/ui/src/Hooks/useFetchGet.test.js @@ -11,8 +11,6 @@ import { useFetchGet } from "./useFetchGet"; describe("useFetchGet", () => { beforeAll(() => { - jest.useFakeTimers(); - fetchMock.mock("http://localhost/ok", { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ status: "ok" }), @@ -23,6 +21,7 @@ describe("useFetchGet", () => { }); beforeEach(() => { + jest.useFakeTimers(); fetchMock.resetHistory(); }); @@ -116,7 +115,7 @@ describe("useFetchGet", () => { expect(result.current.retryCount).toBe(FetchRetryConfig.retries + 1); expect(fetchMock.calls()).toHaveLength(FetchRetryConfig.retries + 1); - expect(fetchMock.lastCall()[0]).toBe("http://localhost/error"); + expect(fetchMock.lastUrl()).toBe("http://localhost/error"); //verify headers for each request for (let i = 0; i <= FetchRetryConfig.retries; i++) { @@ -367,4 +366,39 @@ describe("useFetchGet", () => { jest.runOnlyPendingTimers(); await fetchMock.flush(true); }); + + it("doesn't update response after cleanup on slow body read", async () => { + FetchRetryConfig.retries = 0; + + let tree; + const fetcher = jest.fn(() => + Promise.resolve({ + headers: { + get: () => "text/plain", + }, + text: async () => { + tree.unmount(); + return "ok"; + }, + }) + ); + jest.useRealTimers(); + + const Component = () => { + const { + response, + error, + isLoading, + } = useFetchGet("http://localhost/slow/body", { fetcher: fetcher }); + return ( + + {response} + {error} + {isLoading} + + ); + }; + + tree = mount(); + }); });