From 5fea050fed5231633ec1a0b4d51927715da27c88 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Thu, 7 Mar 2024 21:00:43 +0100 Subject: [PATCH] Refactor API fetching code, handle all possible errors correctly Signed-off-by: Julius Volz --- web/ui/mantine-ui/src/api/api.ts | 122 +++++++++--------- .../src/api/responseTypes/config.ts | 3 + web/ui/mantine-ui/src/pages/AlertsPage.tsx | 7 +- web/ui/mantine-ui/src/pages/ConfigPage.tsx | 11 +- web/ui/mantine-ui/src/pages/FlagsPage.tsx | 4 +- web/ui/mantine-ui/src/pages/RulesPage.tsx | 2 +- web/ui/mantine-ui/src/pages/StatusPage.tsx | 10 +- .../mantine-ui/src/pages/TSDBStatusPage.tsx | 2 +- .../mantine-ui/src/pages/query/DataTable.tsx | 5 - .../src/pages/query/ExpressionInput.tsx | 9 +- 10 files changed, 85 insertions(+), 90 deletions(-) create mode 100644 web/ui/mantine-ui/src/api/responseTypes/config.ts diff --git a/web/ui/mantine-ui/src/api/api.ts b/web/ui/mantine-ui/src/api/api.ts index 5561db918a..b1db71f73d 100644 --- a/web/ui/mantine-ui/src/api/api.ts +++ b/web/ui/mantine-ui/src/api/api.ts @@ -16,79 +16,79 @@ export type ErrorAPIResponse = { export type APIResponse = SuccessAPIResponse | ErrorAPIResponse; -export const useAPIQuery = ({ - key, - path, - params, - enabled, -}: { +const createQueryFn = + ({ path, params }: { path: string; params?: Record }) => + async ({ signal }: { signal: AbortSignal }) => { + const queryString = params + ? `?${new URLSearchParams(params).toString()}` + : ""; + + try { + const res = await fetch(`/${API_PATH}/${path}${queryString}`, { + cache: "no-store", + credentials: "same-origin", + signal, + }); + + if ( + !res.ok && + !res.headers.get("content-type")?.startsWith("application/json") + ) { + // For example, Prometheus may send a 503 Service Unavailable response + // with a "text/plain" content type when it's starting up. But the API + // may also respond with a JSON error message and the same error code. + throw new Error(res.statusText); + } + + const apiRes = (await res.json()) as APIResponse; + + if (apiRes.status === "error") { + throw new Error( + apiRes.error !== undefined + ? apiRes.error + : 'missing "error" field in response JSON' + ); + } + + return apiRes as SuccessAPIResponse; + } catch (error) { + if (!(error instanceof Error)) { + throw new Error("Unknown error"); + } + + switch (error.name) { + case "TypeError": + throw new Error("Network error or unable to reach the server"); + case "SyntaxError": + throw new Error("Invalid JSON response"); + default: + throw error; + } + } + }; + +type QueryOptions = { key?: string; path: string; params?: Record; enabled?: boolean; -}) => - useQuery>({ - queryKey: [key || path], +}; + +export const useAPIQuery = ({ key, path, params, enabled }: QueryOptions) => + useQuery>({ + queryKey: key ? [key] : [path, params], retry: false, refetchOnWindowFocus: false, gcTime: 0, enabled, - queryFn: async ({ signal }) => { - const queryString = params - ? `?${new URLSearchParams(params).toString()}` - : ""; - return ( - fetch(`/${API_PATH}/${path}${queryString}`, { - cache: "no-store", - credentials: "same-origin", - signal, - }) - // TODO: think about how to check API errors here, if this code remains in use. - .then((res) => { - if (!res.ok) { - throw new Error(res.statusText); - } - return res; - }) - .then((res) => res.json() as Promise>) - ); - }, + queryFn: createQueryFn({ path, params }), }); -export const useSuspenseAPIQuery = ( - path: string, - params?: Record -) => +export const useSuspenseAPIQuery = ({ key, path, params }: QueryOptions) => useSuspenseQuery>({ - queryKey: [path], + queryKey: key ? [key] : [path, params], retry: false, refetchOnWindowFocus: false, gcTime: 0, - queryFn: ({ signal }) => { - const queryString = params - ? `?${new URLSearchParams(params).toString()}` - : ""; - return ( - fetch(`/${API_PATH}/${path}${queryString}`, { - cache: "no-store", - credentials: "same-origin", - signal, - }) - // Introduce 3 seconds delay to simulate slow network. - // .then( - // (res) => - // new Promise((resolve) => - // setTimeout(() => resolve(res), 2000) - // ) - // ) - // TODO: think about how to check API errors here, if this code remains in use. - .then((res) => { - if (!res.ok) { - throw new Error(res.statusText); - } - return res; - }) - .then((res) => res.json() as Promise>) - ); - }, + queryFn: createQueryFn({ path, params }), }); diff --git a/web/ui/mantine-ui/src/api/responseTypes/config.ts b/web/ui/mantine-ui/src/api/responseTypes/config.ts new file mode 100644 index 0000000000..f9a72f3b60 --- /dev/null +++ b/web/ui/mantine-ui/src/api/responseTypes/config.ts @@ -0,0 +1,3 @@ +export default interface ConfigResult { + yaml: string; +} diff --git a/web/ui/mantine-ui/src/pages/AlertsPage.tsx b/web/ui/mantine-ui/src/pages/AlertsPage.tsx index dba18f5d53..b1762004c4 100644 --- a/web/ui/mantine-ui/src/pages/AlertsPage.tsx +++ b/web/ui/mantine-ui/src/pages/AlertsPage.tsx @@ -17,8 +17,11 @@ import { formatRelative, now } from "../lib/formatTime"; import { Fragment, useState } from "react"; export default function AlertsPage() { - const { data } = useSuspenseAPIQuery(`/rules`, { - type: "alert", + const { data } = useSuspenseAPIQuery({ + path: `/rules`, + params: { + type: "alert", + }, }); const [showAnnotations, setShowAnnotations] = useState(false); diff --git a/web/ui/mantine-ui/src/pages/ConfigPage.tsx b/web/ui/mantine-ui/src/pages/ConfigPage.tsx index 4d3c27cdb2..644ed5d886 100644 --- a/web/ui/mantine-ui/src/pages/ConfigPage.tsx +++ b/web/ui/mantine-ui/src/pages/ConfigPage.tsx @@ -1,17 +1,14 @@ import { CodeHighlight } from "@mantine/code-highlight"; -import { useSuspenseQuery } from "@tanstack/react-query"; +import { useSuspenseAPIQuery } from "../api/api"; +import ConfigResult from "../api/responseTypes/config"; export default function ConfigPage() { const { data: { data: { yaml }, }, - } = useSuspenseQuery<{ data: { yaml: string } }>({ - queryKey: ["config"], - queryFn: () => { - return fetch("/api/v1/status/config").then((res) => res.json()); - }, - }); + } = useSuspenseAPIQuery({ path: `/status/config` }); + return ( >(`/status/flags`); + const { data } = useSuspenseAPIQuery>({ + path: `/status/flags`, + }); const flags = Object.entries(data.data).map(([flag, value]) => ({ flag, diff --git a/web/ui/mantine-ui/src/pages/RulesPage.tsx b/web/ui/mantine-ui/src/pages/RulesPage.tsx index 7b36dcaefb..56578d8bca 100644 --- a/web/ui/mantine-ui/src/pages/RulesPage.tsx +++ b/web/ui/mantine-ui/src/pages/RulesPage.tsx @@ -28,7 +28,7 @@ const healthBadgeClass = (state: string) => { }; export default function RulesPage() { - const { data } = useSuspenseAPIQuery(`/rules`); + const { data } = useSuspenseAPIQuery({ path: `/rules` }); return ( <> diff --git a/web/ui/mantine-ui/src/pages/StatusPage.tsx b/web/ui/mantine-ui/src/pages/StatusPage.tsx index 34e7392697..43aef6dca4 100644 --- a/web/ui/mantine-ui/src/pages/StatusPage.tsx +++ b/web/ui/mantine-ui/src/pages/StatusPage.tsx @@ -28,10 +28,12 @@ const statusConfig: Record< }; export default function StatusPage() { - const { data: buildinfo } = - useSuspenseAPIQuery>(`/status/buildinfo`); - const { data: runtimeinfo } = - useSuspenseAPIQuery>(`/status/runtimeinfo`); + const { data: buildinfo } = useSuspenseAPIQuery>({ + path: `/status/buildinfo`, + }); + const { data: runtimeinfo } = useSuspenseAPIQuery>({ + path: `/status/runtimeinfo`, + }); return ( diff --git a/web/ui/mantine-ui/src/pages/TSDBStatusPage.tsx b/web/ui/mantine-ui/src/pages/TSDBStatusPage.tsx index abc207c746..0b3aaae240 100644 --- a/web/ui/mantine-ui/src/pages/TSDBStatusPage.tsx +++ b/web/ui/mantine-ui/src/pages/TSDBStatusPage.tsx @@ -13,7 +13,7 @@ export default function TSDBStatusPage() { seriesCountByLabelValuePair, }, }, - } = useSuspenseAPIQuery(`/status/tsdb`); + } = useSuspenseAPIQuery({ path: `/status/tsdb` }); const unixToTime = (unix: number): string => { try { diff --git a/web/ui/mantine-ui/src/pages/query/DataTable.tsx b/web/ui/mantine-ui/src/pages/query/DataTable.tsx index 27c071467d..1f5ab0d482 100644 --- a/web/ui/mantine-ui/src/pages/query/DataTable.tsx +++ b/web/ui/mantine-ui/src/pages/query/DataTable.tsx @@ -81,11 +81,6 @@ const DataTable: FC = ({ expr, evalTime, retriggerIdx }) => { return No data queried yet; } - if (data.status !== "success") { - // TODO: Remove this case and handle it in useAPIQuery instead! - return null; - } - const { result, resultType } = data.data; if (result.length === 0) { diff --git a/web/ui/mantine-ui/src/pages/query/ExpressionInput.tsx b/web/ui/mantine-ui/src/pages/query/ExpressionInput.tsx index 099f5c1be2..2b1f69765d 100644 --- a/web/ui/mantine-ui/src/pages/query/ExpressionInput.tsx +++ b/web/ui/mantine-ui/src/pages/query/ExpressionInput.tsx @@ -148,10 +148,6 @@ const ExpressionInput: FC = ({ } if (formatResult) { - if (formatResult.status !== "success") { - // TODO: Remove this case and handle it in useAPIQuery instead! - return; - } setExpr(formatResult.data); notifications.show({ color: "green", @@ -222,10 +218,7 @@ const ExpressionInput: FC = ({ } onClick={() => formatQuery()} disabled={ - isFormatting || - expr === "" || - (formatResult?.status === "success" && - expr === formatResult.data) + isFormatting || expr === "" || expr === formatResult?.data } > Format expression