From 55939ea4142e0d3b7ba13ad3794d7888992afa39 Mon Sep 17 00:00:00 2001 From: Bradley Behnke Date: Wed, 11 Oct 2023 13:51:08 -0700 Subject: [PATCH] feat: error handling/display for all pages and pipeline load block (#1187) --- ui/src/App.tsx | 37 ++- .../common/ErrorIndicator/index.tsx | 43 ++++ .../common/ErrorIndicator/style.css | 4 + .../common/SlidingSidebar/index.tsx | 12 +- .../SlidingSidebar/partials/Errors/index.tsx | 83 ++++--- .../SlidingSidebar/partials/Errors/style.css | 6 +- .../partials/PipelineCreate/index.tsx | 9 + ui/src/components/pages/Cluster/index.tsx | 9 +- .../ClusterNamespaceListing/index.tsx | 12 +- ui/src/components/pages/Namespace/index.tsx | 6 +- .../NamespacePipelineListing/index.tsx | 12 +- .../Namespace/partials/PipelineCard/index.tsx | 66 +++-- ui/src/components/pages/Pipeline/index.tsx | 80 +----- .../pages/Pipeline/partials/Graph/index.tsx | 217 ++++++++-------- ui/src/types/declarations/app.d.ts | 10 +- ui/src/types/declarations/cluster.d.ts | 1 + ui/src/types/declarations/graph.d.ts | 1 - ui/src/types/declarations/namespace.d.ts | 1 + .../fetchWrappers/clusterSummaryFetch.ts | 233 ++---------------- ui/src/utils/fetchWrappers/fetch.ts | 1 + .../fetchWrappers/namespaceSummaryFetch.ts | 38 ++- ui/src/utils/fetchWrappers/pipelineFetch.ts | 46 ++-- .../fetchWrappers/pipelineUpdateFetch.ts | 5 +- .../fetcherHooks/pipelineViewFetch.test.ts | 6 +- .../utils/fetcherHooks/pipelineViewFetch.ts | 177 +++++-------- 25 files changed, 504 insertions(+), 611 deletions(-) create mode 100644 ui/src/components/common/ErrorIndicator/index.tsx create mode 100644 ui/src/components/common/ErrorIndicator/style.css diff --git a/ui/src/App.tsx b/ui/src/App.tsx index ec8fc152aa..7295194937 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -11,7 +11,7 @@ import Drawer from "@mui/material/Drawer"; import AppBar from "@mui/material/AppBar"; import Toolbar from "@mui/material/Toolbar"; import CircularProgress from "@mui/material/CircularProgress"; -import { Routes, Route } from "react-router-dom"; +import { Routes, Route, useLocation } from "react-router-dom"; import { Breadcrumbs } from "./components/common/Breadcrumbs"; import { Cluster } from "./components/pages/Cluster"; import { Namespaces } from "./components/pages/Namespace"; @@ -23,7 +23,7 @@ import { SlidingSidebarProps, } from "./components/common/SlidingSidebar"; import { ErrorDisplay } from "./components/common/ErrorDisplay"; -import { AppContextProps } from "./types/declarations/app"; +import { AppContextProps, AppError } from "./types/declarations/app"; import logo from "./images/icon.png"; import textLogo from "./images/text-icon.png"; @@ -33,8 +33,17 @@ import "react-toastify/dist/ReactToastify.css"; export const AppContext = React.createContext({ systemInfo: undefined, systemInfoError: undefined, + // eslint-disable-next-line @typescript-eslint/no-empty-function + setSidebarProps: () => {}, + errors: [], + // eslint-disable-next-line @typescript-eslint/no-empty-function + addError: () => {}, + // eslint-disable-next-line @typescript-eslint/no-empty-function + clearErrors: () => {}, }); +const MAX_ERRORS = 6; + function App() { // TODO remove, used for testing ns only installation // const { systemInfo, error: systemInfoError } = { @@ -52,7 +61,14 @@ function App() { const [sidebarCloseIndicator, setSidebarCloseIndicator] = useState< string | undefined >(); + const [errors, setErrors] = useState([]); const { systemInfo, error: systemInfoError, loading } = useSystemInfoFetch(); + const location = useLocation(); + + useEffect(() => { + // Route changed + setErrors([]); + }, [location]); // Resize observer to keep page width in state. To be used by other dependent components. useEffect(() => { @@ -87,6 +103,20 @@ function App() { setSidebarCloseIndicator("id" + Math.random().toString(16).slice(2)); }, []); + const handleAddError = useCallback((error: string) => { + setErrors((prev) => { + prev.unshift({ + message: error, + date: new Date(), + }); + return prev.slice(0, MAX_ERRORS); + }); + }, []); + + const handleClearErrors = useCallback(() => { + setErrors([]); + }, []); + const routes = useMemo(() => { if (loading) { // System info loading @@ -171,6 +201,9 @@ function App() { systemInfoError, sidebarProps, setSidebarProps, + errors, + addError: handleAddError, + clearErrors: handleClearErrors, }} > diff --git a/ui/src/components/common/ErrorIndicator/index.tsx b/ui/src/components/common/ErrorIndicator/index.tsx new file mode 100644 index 0000000000..9488b58c11 --- /dev/null +++ b/ui/src/components/common/ErrorIndicator/index.tsx @@ -0,0 +1,43 @@ +import React, { useContext, useCallback } from "react"; +import Box from "@mui/material/Box"; +import Paper from "@mui/material/Paper"; +import { AppContextProps } from "../../../types/declarations/app"; +import { AppContext } from "../../../App"; +import { SidebarType } from "../SlidingSidebar"; +import ErrorOutlineIcon from "@mui/icons-material/ErrorOutline"; +import ErrorIcon from "@mui/icons-material/Error"; + +import "./style.css"; + +export function ErrorIndicator() { + const { errors, setSidebarProps } = useContext(AppContext); + + const onErrorClick = useCallback(() => { + setSidebarProps({ + type: SidebarType.ERRORS, + slide: false, + }); + }, []); + + return ( + + + {errors && errors.length ? ( + + ) : ( + + )} + {errors.length ? ( + Error occurred + ) : undefined} + + + ); +} diff --git a/ui/src/components/common/ErrorIndicator/style.css b/ui/src/components/common/ErrorIndicator/style.css new file mode 100644 index 0000000000..d60b734029 --- /dev/null +++ b/ui/src/components/common/ErrorIndicator/style.css @@ -0,0 +1,4 @@ +.error-indicator-text { + margin-left: 0.5rem; + min-width: 6.5625rem; +} \ No newline at end of file diff --git a/ui/src/components/common/SlidingSidebar/index.tsx b/ui/src/components/common/SlidingSidebar/index.tsx index e521c36e09..b86ac7b38e 100644 --- a/ui/src/components/common/SlidingSidebar/index.tsx +++ b/ui/src/components/common/SlidingSidebar/index.tsx @@ -13,7 +13,7 @@ import { GeneratorDetails, GeneratorDetailsProps, } from "./partials/GeneratorDetails"; -import { Errors, ErrorsProps } from "./partials/Errors"; +import { Errors } from "./partials/Errors"; import { PiplineCreate } from "./partials/PipelineCreate"; import { PiplineUpdate } from "./partials/PipelineUpdate"; import { ISBCreate } from "./partials/ISBCreate"; @@ -79,7 +79,6 @@ export interface SlidingSidebarProps { vertexDetailsProps?: VertexDetailsProps; edgeDetailsProps?: EdgeDetailsProps; generatorDetailsProps?: GeneratorDetailsProps; - errorsProps?: ErrorsProps; specEditorProps?: SpecEditorSidebarProps; parentCloseIndicator?: string; } @@ -92,13 +91,12 @@ export function SlidingSidebar({ vertexDetailsProps, edgeDetailsProps, generatorDetailsProps, - errorsProps, specEditorProps, parentCloseIndicator, }: SlidingSidebarProps) { const { setSidebarProps } = useContext(AppContext); const [width, setWidth] = useState( - errorsProps + type === SidebarType.ERRORS ? MIN_WIDTH_BY_TYPE[SidebarType.ERRORS] : (pageWidth * 0.75) ); @@ -243,10 +241,7 @@ export function SlidingSidebar({ } return ; case SidebarType.ERRORS: - if (!errorsProps) { - break; - } - return ; + return ; default: break; } @@ -258,7 +253,6 @@ export function SlidingSidebar({ vertexDetailsProps, edgeDetailsProps, generatorDetailsProps, - errorsProps, ]); return ( diff --git a/ui/src/components/common/SlidingSidebar/partials/Errors/index.tsx b/ui/src/components/common/SlidingSidebar/partials/Errors/index.tsx index 4f1fa88b06..6d6f25f99e 100644 --- a/ui/src/components/common/SlidingSidebar/partials/Errors/index.tsx +++ b/ui/src/components/common/SlidingSidebar/partials/Errors/index.tsx @@ -1,28 +1,60 @@ -import React, { useMemo } from "react"; +import React, { useCallback, useContext, useMemo } from "react"; import Box from "@mui/material/Box"; -import { Slide, ToastContainer } from "react-toastify"; +import Grid from "@mui/material/Grid"; +import Paper from "@mui/material/Paper"; +import Button from "@mui/material/Button"; +import { AppContextProps } from "../../../../../types/declarations/app"; +import { AppContext } from "../../../../../App"; import "./style.css"; -export interface ErrorsProps { - errors: boolean; -} +export function Errors() { + const { errors, clearErrors } = useContext(AppContext); + + const handleClear = useCallback(() => { + clearErrors(); + }, [clearErrors]); -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export function Errors({ errors }: ErrorsProps) { - const header = useMemo(() => { - const headerContainerStyle = { + const content = useMemo(() => { + const paperStyle = { display: "flex", - flexDirection: "row", + flexDirection: "column", + padding: "1rem", }; - const textClass = "vertex-details-header-text"; - return ( - - Errors - + + {!errors.length && ( + + + No errors + + + )} + {errors.map((error) => ( + + + {error.message} + {error.date.toLocaleTimeString()} + + + ))} + {!!errors.length && ( + + )} + ); - }, []); + }, [errors]); return ( - - {header} - - + Errors + {content} ); } diff --git a/ui/src/components/common/SlidingSidebar/partials/Errors/style.css b/ui/src/components/common/SlidingSidebar/partials/Errors/style.css index b46961c0c0..ea02a61995 100644 --- a/ui/src/components/common/SlidingSidebar/partials/Errors/style.css +++ b/ui/src/components/common/SlidingSidebar/partials/Errors/style.css @@ -1,5 +1,9 @@ -.vertex-details-header-text { +.errors-header-text { font-size: 1.25rem; font-style: normal; font-weight: 500; } + +.errors-message-text { + color: #D52B1E; +} diff --git a/ui/src/components/common/SlidingSidebar/partials/PipelineCreate/index.tsx b/ui/src/components/common/SlidingSidebar/partials/PipelineCreate/index.tsx index 637413a6be..2ef30788ea 100644 --- a/ui/src/components/common/SlidingSidebar/partials/PipelineCreate/index.tsx +++ b/ui/src/components/common/SlidingSidebar/partials/PipelineCreate/index.tsx @@ -62,6 +62,15 @@ export function PiplineCreate({ active: !!createdPipelineId, }); + // Call update complete on dismount if pipeline was created + useEffect(() => { + return () => { + if (createdPipelineId) { + onUpdateComplete && onUpdateComplete(); + } + }; + }, [createdPipelineId, onUpdateComplete]); + // Track creation process and close on completion useEffect(() => { if (!createdPipelineId) { diff --git a/ui/src/components/pages/Cluster/index.tsx b/ui/src/components/pages/Cluster/index.tsx index bce4afce72..9a92ad71ee 100644 --- a/ui/src/components/pages/Cluster/index.tsx +++ b/ui/src/components/pages/Cluster/index.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from "react"; +import React, { useContext, useMemo } from "react"; import Box from "@mui/material/Box"; import CircularProgress from "@mui/material/CircularProgress"; import { @@ -9,12 +9,16 @@ import { import { ClusterNamespaceListing } from "./partials/ClusterNamespaceListing"; import { ErrorDisplay } from "../../common/ErrorDisplay"; import { useClusterSummaryFetch } from "../../../utils/fetchWrappers/clusterSummaryFetch"; +import { AppContextProps } from "../../../types/declarations/app"; +import { AppContext } from "../../../App"; import "./style.css"; export function Cluster() { + const { addError } = useContext(AppContext); const { data, loading, error } = useClusterSummaryFetch({ loadOnRefresh: false, + addError, }); const summarySections: SummarySection[] = useMemo(() => { @@ -22,7 +26,7 @@ export function Cluster() { return [ { type: SummarySectionType.CUSTOM, - customComponent: , + customComponent: , }, ]; } @@ -32,6 +36,7 @@ export function Cluster() { type: SummarySectionType.CUSTOM, customComponent: ( diff --git a/ui/src/components/pages/Cluster/partials/ClusterNamespaceListing/index.tsx b/ui/src/components/pages/Cluster/partials/ClusterNamespaceListing/index.tsx index 16817f58be..a54159e255 100644 --- a/ui/src/components/pages/Cluster/partials/ClusterNamespaceListing/index.tsx +++ b/ui/src/components/pages/Cluster/partials/ClusterNamespaceListing/index.tsx @@ -4,6 +4,7 @@ import Pagination from "@mui/material/Pagination"; import Grid from "@mui/material/Grid"; import { DebouncedSearchInput } from "../../../../common/DebouncedSearchInput"; import { NamespaceCard } from "../NamespaceCard"; +import { ErrorIndicator } from "../../../../common/ErrorIndicator"; import { ClusterNamespaceListingProps, ClusterNamespaceSummary, @@ -113,11 +114,20 @@ export function ClusterNamespaceListing({ - + + + + Namespaces diff --git a/ui/src/components/pages/Namespace/index.tsx b/ui/src/components/pages/Namespace/index.tsx index 56e582e59b..f2712a1dab 100644 --- a/ui/src/components/pages/Namespace/index.tsx +++ b/ui/src/components/pages/Namespace/index.tsx @@ -18,11 +18,12 @@ import "./style.css"; export function Namespaces() { const { namespaceId } = useParams(); - const { setSidebarProps } = useContext(AppContext); + const { setSidebarProps, addError } = useContext(AppContext); const { data, pipelineRawData, isbRawData, loading, error, refresh } = useNamespaceSummaryFetch({ namespace: namespaceId || "", loadOnRefresh: false, + addError, }); const handleK8sEventsClick = useCallback(() => { @@ -40,7 +41,7 @@ export function Namespaces() { return [ { type: SummarySectionType.CUSTOM, - customComponent: , + customComponent: , }, ]; } @@ -50,6 +51,7 @@ export function Namespaces() { type: SummarySectionType.CUSTOM, customComponent: ( diff --git a/ui/src/components/pages/Namespace/partials/NamespacePipelineListing/index.tsx b/ui/src/components/pages/Namespace/partials/NamespacePipelineListing/index.tsx index 33b2cf9555..4d2ffe45c3 100644 --- a/ui/src/components/pages/Namespace/partials/NamespacePipelineListing/index.tsx +++ b/ui/src/components/pages/Namespace/partials/NamespacePipelineListing/index.tsx @@ -20,10 +20,8 @@ import { AppContext } from "../../../../../App"; import { SidebarType } from "../../../../common/SlidingSidebar"; import { ViewType } from "../../../../common/SpecEditor"; import { Button, MenuItem, Select } from "@mui/material"; - -import "./style.css"; +import { ErrorIndicator } from "../../../../common/ErrorIndicator"; import { - ACTIVE, ALL, ALPHABETICAL_SORT, ASC, @@ -38,6 +36,8 @@ import { WARNING, } from "../../../../../utils"; +import "./style.css"; + const MAX_PAGE_SIZE = 4; const sortOptions = [ @@ -323,7 +323,7 @@ export function NamespacePipelineListing({ padding: "0 2.625rem", }} > - + @@ -406,6 +407,9 @@ export function NamespacePipelineListing({ + + + (AppContext); - const [editOption] = useState("view"); + const [editOption] = useState("edit"); const [deleteOption] = useState("delete"); const [deleteProps, setDeleteProps] = useState(); const [statusPayload, setStatusPayload] = useState(undefined); @@ -58,6 +59,19 @@ export function PipelineCard({ ); const [timerDateStamp, setTimerDateStamp] = useState(undefined); const [timer, setTimer] = useState(undefined); + const [pipelineAbleToLoad, setPipelineAbleToLoad] = useState(false); + const { pipelineAvailable } = usePipelineUpdateFetch({ + namespaceId: namespace, + pipelineId: data?.name, + active: !pipelineAbleToLoad, + refreshInterval: 5000, // 5 seconds + }); + + useEffect(() => { + if (pipelineAvailable) { + setPipelineAbleToLoad(true); + } + }, [pipelineAvailable]); const handleUpdateComplete = useCallback(() => { refresh(); @@ -136,7 +150,8 @@ export function PipelineCard({ }, 1000); setTimer(pauseTimer); }, []); - const handlePlayClick = useCallback((e) => { + + const handlePlayClick = useCallback(() => { handleTimer(); setStatusPayload({ spec: { @@ -147,7 +162,7 @@ export function PipelineCard({ }); }, []); - const handlePauseClick = useCallback((e) => { + const handlePauseClick = useCallback(() => { handleTimer(); setStatusPayload({ spec: { @@ -301,19 +316,23 @@ export function PipelineCard({ - - + + + {" "} + + + Pipeline Pausing... + + + {data?.pipeline?.status?.lastUpdated + ? timeAgo(data?.pipeline?.status?.lastUpdated) + : ""} + + + + ) : ( + "" + )} + + + + + - - {/**/} - {/* {showSpec && }*/} - {/* {edgeOpen && }*/} - {/* {nodeOpen && (*/} - {/* */} - {/* )}*/} - {/**/} ); } diff --git a/ui/src/types/declarations/app.d.ts b/ui/src/types/declarations/app.d.ts index 61a8e08606..b198e20b4e 100644 --- a/ui/src/types/declarations/app.d.ts +++ b/ui/src/types/declarations/app.d.ts @@ -3,9 +3,17 @@ export interface SystemInfo { managedNamespace: string | undefined; } +export interface AppError { + message: string; + date: Date; +} + export interface AppContextProps { systemInfo: SystemInfo | undefined; systemInfoError: any | undefined; sidebarProps?: SlidingSideBarProps; - setSidebarProps?: (props: SlidingSideBarProps | undefined) => void; + setSidebarProps: (props: SlidingSideBarProps | undefined) => void; + errors: AppError[]; + addError: (error: string) => void; + clearErrors: () => void; } \ No newline at end of file diff --git a/ui/src/types/declarations/cluster.d.ts b/ui/src/types/declarations/cluster.d.ts index 77e02033f8..e5b91d766a 100644 --- a/ui/src/types/declarations/cluster.d.ts +++ b/ui/src/types/declarations/cluster.d.ts @@ -39,6 +39,7 @@ export interface ClusterSummaryFetchResult { export interface ClusterSummaryFetchProps { loadOnRefresh?: boolean; + addError: (error: string) => void; } export interface ClusterNamespaceListingProps { diff --git a/ui/src/types/declarations/graph.d.ts b/ui/src/types/declarations/graph.d.ts index 35ecdc1b53..cb7d279997 100644 --- a/ui/src/types/declarations/graph.d.ts +++ b/ui/src/types/declarations/graph.d.ts @@ -31,7 +31,6 @@ export interface FlowProps { handleNodeClick: (e: Element | EventType, node: Node) => void; handleEdgeClick: (e: Element | EventType, edge: Edge) => void; handlePaneClick: () => void; - setSidebarProps: Dispatch>; } export interface HighlightContextProps { diff --git a/ui/src/types/declarations/namespace.d.ts b/ui/src/types/declarations/namespace.d.ts index b52c3833b1..275b41e841 100644 --- a/ui/src/types/declarations/namespace.d.ts +++ b/ui/src/types/declarations/namespace.d.ts @@ -36,6 +36,7 @@ export interface NamespaceSummaryFetchResult { export interface NamespaceSummaryFetchProps { namespace: string; loadOnRefresh?: boolean; + addError: (error: string) => void; } export interface PipelineCardProps { namespace: string; diff --git a/ui/src/utils/fetchWrappers/clusterSummaryFetch.ts b/ui/src/utils/fetchWrappers/clusterSummaryFetch.ts index 9026535400..d1d5d3a4aa 100644 --- a/ui/src/utils/fetchWrappers/clusterSummaryFetch.ts +++ b/ui/src/utils/fetchWrappers/clusterSummaryFetch.ts @@ -7,199 +7,6 @@ import { ClusterSummaryFetchResult, } from "../../types/declarations/cluster"; -// const MOCK_DATA = [ -// { -// namespace: "my-ns", -// pipelineSummary: { -// active: { -// Healthy: 1, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 2, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 2, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 2, -// }, -// }, -// { -// namespace: "my-other-ns", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns1", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns2", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns3", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns4", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns5", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns6", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns7", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// { -// namespace: "my-other-ns8", -// pipelineSummary: { -// active: { -// Healthy: 5, -// Warning: 1, -// Critical: 1, -// }, -// inactive: 1, -// }, -// isbServiceSummary: { -// active: { -// Healthy: 5, -// Warning: 2, -// Critical: 2, -// }, -// inactive: 1, -// }, -// }, -// ]; - const rawDataToClusterSummary = ( rawData: any[] ): ClusterSummaryData | undefined => { @@ -289,10 +96,11 @@ const rawDataToClusterSummary = ( }; const DEFAULT_NS_NAME = "default"; -const DATA_REFRESH_INTERVAL = 60000; // ms +const DATA_REFRESH_INTERVAL = 30000; // ms export const useClusterSummaryFetch = ({ loadOnRefresh = false, + addError, }: ClusterSummaryFetchProps) => { const [results, setResults] = useState({ data: undefined, @@ -331,23 +139,34 @@ export const useClusterSummaryFetch = ({ return; } if (fetchError) { - setResults({ - data: undefined, - loading: false, - error: fetchError, - }); + if (options?.requestKey === "") { + // Failed on first load, return error + setResults({ + data: undefined, + loading: false, + error: fetchError, + }); + } else { + // Failed on refresh, add error to app context + addError(fetchError); + } return; } - if (fetchData && fetchData.errMsg) { - setResults({ - data: undefined, - loading: false, - error: fetchData.errMsg, - }); + if (fetchData?.errMsg) { + if (options?.requestKey === "") { + // Failed on first load, return error + setResults({ + data: undefined, + loading: false, + error: fetchData.errMsg, + }); + } else { + // Failed on refresh, add error to app context + addError(fetchData.errMsg); + } return; } if (fetchData) { - // const clusterSummary = rawDataToClusterSummary(MOCK_DATA); // TODO REMOVE MOCK const clusterSummary = rawDataToClusterSummary(fetchData.data); setResults({ data: clusterSummary, @@ -356,7 +175,7 @@ export const useClusterSummaryFetch = ({ }); return; } - }, [fetchData, fetchLoading, fetchError, loadOnRefresh, options]); + }, [fetchData, fetchLoading, fetchError, loadOnRefresh, options, addError]); return results; }; diff --git a/ui/src/utils/fetchWrappers/fetch.ts b/ui/src/utils/fetchWrappers/fetch.ts index 21226f94c8..ea373b28aa 100644 --- a/ui/src/utils/fetchWrappers/fetch.ts +++ b/ui/src/utils/fetchWrappers/fetch.ts @@ -31,6 +31,7 @@ export const useFetch = ( setLoading(false); } else { const data = await response.json(); + setError(undefined); setData(data); setLoading(false); } diff --git a/ui/src/utils/fetchWrappers/namespaceSummaryFetch.ts b/ui/src/utils/fetchWrappers/namespaceSummaryFetch.ts index 2c33c174be..ced3963eac 100644 --- a/ui/src/utils/fetchWrappers/namespaceSummaryFetch.ts +++ b/ui/src/utils/fetchWrappers/namespaceSummaryFetch.ts @@ -102,6 +102,7 @@ const DATA_REFRESH_INTERVAL = 15000; // ms export const useNamespaceSummaryFetch = ({ namespace, loadOnRefresh = false, + addError, }: NamespaceSummaryFetchProps) => { const [options, setOptions] = useState({ skip: false, @@ -161,21 +162,33 @@ export const useNamespaceSummaryFetch = ({ return; } if (pipelineError || isbError) { - setResults({ - data: undefined, - loading: false, - error: pipelineError || isbError, - refresh, - }); + if (options?.requestKey === "") { + // Failed on first load, return error + setResults({ + data: undefined, + loading: false, + error: pipelineError || isbError, + refresh, + }); + } else { + // Failed on refresh, add error to app context + addError(pipelineError || isbError); + } return; } if (pipelineData?.errMsg || isbData?.errMsg) { - setResults({ - data: undefined, - loading: false, - error: pipelineData?.errMsg || isbData?.errMsg, - refresh, - }); + if (options?.requestKey === "") { + // Failed on first load, return error + setResults({ + data: undefined, + loading: false, + error: pipelineData?.errMsg || isbData?.errMsg, + refresh, + }); + } else { + // Failed on refresh, add error to app context + addError(pipelineData?.errMsg || isbData?.errMsg); + } return; } if (pipelineData && isbData) { @@ -211,6 +224,7 @@ export const useNamespaceSummaryFetch = ({ loadOnRefresh, options, refresh, + addError, ]); return results; diff --git a/ui/src/utils/fetchWrappers/pipelineFetch.ts b/ui/src/utils/fetchWrappers/pipelineFetch.ts index 9234aaba48..4e3a1cdfef 100644 --- a/ui/src/utils/fetchWrappers/pipelineFetch.ts +++ b/ui/src/utils/fetchWrappers/pipelineFetch.ts @@ -1,12 +1,15 @@ import { useEffect, useState, useCallback } from "react"; import { Options, useFetch } from "./fetch"; -import {PipelineSummaryFetchResult} from "../../types/declarations/pipeline"; - +import { PipelineSummaryFetchResult } from "../../types/declarations/pipeline"; const DATA_REFRESH_INTERVAL = 15000; // ms // fetch pipeline summary and ISB summary -export const usePipelineSummaryFetch = ({ namespaceId, pipelineId }: any) => { +export const usePipelineSummaryFetch = ({ + namespaceId, + pipelineId, + addError, +}: any) => { const [isb, setIsb] = useState(null); const [options, setOptions] = useState({ skip: false, @@ -69,21 +72,33 @@ export const usePipelineSummaryFetch = ({ namespaceId, pipelineId }: any) => { return; } if (pipelineError || isbError) { - setResults({ - data: undefined, - loading: false, - error: pipelineError || isbError, - refresh, - }); + if (options?.requestKey === "") { + // Failed on first load, return error + setResults({ + data: undefined, + loading: false, + error: pipelineError || isbError, + refresh, + }); + } else { + // Failed on refresh, add error to app context + addError(pipelineError || isbError); + } return; } if (pipelineData?.errMsg || isbData?.errMsg) { - setResults({ - data: undefined, - loading: false, - error: pipelineData?.errMsg || isbData?.errMsg, - refresh, - }); + if (options?.requestKey === "") { + // Failed on first load, return error + setResults({ + data: undefined, + loading: false, + error: pipelineData?.errMsg || isbData?.errMsg, + refresh, + }); + } else { + // Failed on refresh, add error to app context + addError(pipelineData?.errMsg || isbData?.errMsg); + } return; } if (pipelineData) { @@ -117,6 +132,7 @@ export const usePipelineSummaryFetch = ({ namespaceId, pipelineId }: any) => { isbError, options, refresh, + addError, ]); return results; }; diff --git a/ui/src/utils/fetchWrappers/pipelineUpdateFetch.ts b/ui/src/utils/fetchWrappers/pipelineUpdateFetch.ts index 2a9e8ab86f..0bee44167c 100644 --- a/ui/src/utils/fetchWrappers/pipelineUpdateFetch.ts +++ b/ui/src/utils/fetchWrappers/pipelineUpdateFetch.ts @@ -9,6 +9,7 @@ export const usePipelineUpdateFetch = ({ namespaceId, pipelineId, active, + refreshInterval = DATA_REFRESH_INTERVAL, }: any) => { const [options, setOptions] = useState({ skip: !active, @@ -44,7 +45,7 @@ export const usePipelineUpdateFetch = ({ skip: false, requestKey: "id" + Math.random().toString(16).slice(2), }); - }, DATA_REFRESH_INTERVAL); + }, refreshInterval); // Clear any existing interval running and store new one setIntervalId((prev: any) => { if (prev) { @@ -56,7 +57,7 @@ export const usePipelineUpdateFetch = ({ // Clear interval on unmount clearInterval(id); }; - }, [active]); + }, [active, refreshInterval]); useEffect(() => { if (loading) { diff --git a/ui/src/utils/fetcherHooks/pipelineViewFetch.test.ts b/ui/src/utils/fetcherHooks/pipelineViewFetch.test.ts index 428b4a2274..2a937806c0 100644 --- a/ui/src/utils/fetcherHooks/pipelineViewFetch.test.ts +++ b/ui/src/utils/fetcherHooks/pipelineViewFetch.test.ts @@ -201,7 +201,7 @@ describe("Custom Pipeline hook", () => { (global as any).fetch = mockedFetch; await act(async () => { const { result } = renderHook(() => - usePipelineViewFetch("default", "simple-pipeline") + usePipelineViewFetch("default", "simple-pipeline", () => { return;}) ); }); expect(mockedFetch).toBeCalledTimes(9); @@ -224,7 +224,9 @@ describe("Custom Pipeline hook", () => { (global as any).fetch = mockedFetch; await act(async () => { const { result } = renderHook(() => - usePipelineViewFetch("default", "simple-pipeline") + usePipelineViewFetch("default", "simple-pipeline", () => { + return; + }) ); }); expect(mockedFetch).toBeCalledTimes(2); diff --git a/ui/src/utils/fetcherHooks/pipelineViewFetch.ts b/ui/src/utils/fetcherHooks/pipelineViewFetch.ts index d344fa18b8..a75ee6e660 100644 --- a/ui/src/utils/fetcherHooks/pipelineViewFetch.ts +++ b/ui/src/utils/fetcherHooks/pipelineViewFetch.ts @@ -11,9 +11,10 @@ import { export const usePipelineViewFetch = ( namespaceId: string | undefined, - pipelineId: string | undefined + pipelineId: string | undefined, + addError: (error: string) => void ) => { - const [requestKey, setRequestKey] = useState(`${Date.now()}`); + const [requestKey, setRequestKey] = useState(""); const [pipeline, setPipeline] = useState(undefined); const [ns_pl, setNS_PL] = useState(""); const [spec, setSpec] = useState(undefined); @@ -37,13 +38,8 @@ export const usePipelineViewFetch = ( const [nodeOutDegree, setNodeOutDegree] = useState>( new Map() ); - const [pipelineErr, setPipelineErr] = useState(undefined); - const [buffersErr, setBuffersErr] = useState(undefined); - const [podsErr, setPodsErr] = useState(undefined); - const [metricsErr, setMetricsErr] = useState(undefined); - const [watermarkErr, setWatermarkErr] = useState( - undefined - ); + const [pipelineErr, setPipelineErr] = useState(undefined); + const [buffersErr, setBuffersErr] = useState(undefined); const [loading, setLoading] = useState(true); const BASE_API = `/api/v1/namespaces/${namespaceId}/pipelines/${pipelineId}`; @@ -65,37 +61,35 @@ export const usePipelineViewFetch = ( // Update spec state if it is not equal to the spec from the response if (!isEqual(spec, json.data?.pipeline?.spec)) setSpec(json.data?.pipeline?.spec); + setPipelineErr(undefined); } else if (json?.errMsg) { // pipeline API call returns an error message - setPipelineErr([ - { - error: json.errMsg, - options: { toastId: "pipeline-fetch-error", autoClose: 5000 }, - }, - ]); + if (requestKey === "") { + setPipelineErr(json.errMsg); + } else { + addError(json.errMsg); + } } } else { // Handle the case when the response is not OK - setPipelineErr([ - { - error: "Failed to fetch the pipeline details", - options: { toastId: "pipeline-fetch", autoClose: 5000 }, - }, - ]); + if (requestKey === "") { + setPipelineErr(`Failed with code: ${response.status}`); + } else { + addError(`Failed with code: ${response.status}`); + } } - } catch { + } catch (e: any) { // Handle any errors that occur during the fetch request - setPipelineErr([ - { - error: "Failed to fetch the pipeline details", - options: { toastId: "pipeline-fetch", autoClose: 5000 }, - }, - ]); + if (requestKey === "") { + setPipelineErr(e.message); + } else { + addError(e.message); + } } }; fetchPipeline(); - }, [requestKey]); + }, [requestKey, addError]); // Call to get buffers useEffect(() => { @@ -109,37 +103,35 @@ export const usePipelineViewFetch = ( if (json?.data) { // Update buffers state with data from the response setBuffers(json.data); + setBuffersErr(undefined); } else if (json?.errMsg) { // Buffer API call returns an error message - setBuffersErr([ - { - error: json.errMsg, - options: { toastId: "isb-fetch-error", autoClose: 5000 }, - }, - ]); + if (requestKey === "") { + setBuffersErr(json.errMsg); + } else { + addError(json.errMsg); + } } } else { // Handle the case when the response is not OK - setBuffersErr([ - { - error: "Failed to fetch the pipeline buffers", - options: { toastId: "isb-fetch", autoClose: 5000 }, - }, - ]); + if (requestKey === "") { + setBuffersErr(`Failed with code: ${response.status}`); + } else { + addError(`Failed with code: ${response.status}`); + } } - } catch { + } catch (e: any) { // Handle any errors that occur during the fetch request - setBuffersErr([ - { - error: "Failed to fetch the pipeline buffers", - options: { toastId: "isb-fetch", autoClose: 5000 }, - }, - ]); + if (requestKey === "") { + setBuffersErr(e.message); + } else { + addError(e.message); + } } }; fetchBuffers(); - }, [requestKey]); + }, [requestKey, addError]); // Refresh pipeline and buffer info every 30 sec useEffect(() => { @@ -152,7 +144,6 @@ export const usePipelineViewFetch = ( // This useEffect is used to obtain all the pods for a given vertex in a pipeline. useEffect(() => { const vertexToPodsMap = new Map(); - const podsErr: any[] = []; if (spec?.vertices) { // Fetch pods count for each vertex in parallel @@ -172,13 +163,7 @@ export const usePipelineViewFetch = ( vertexToPodsMap.set(vertex.name, json.data.length); } else if (json?.errMsg) { // Pods API call returns an error message - podsErr.push({ - error: json.errMsg, - options: { - toastId: `${vertex.name}-pods-fetch-error`, - autoClose: 5000, - }, - }); + addError(json.errMsg); } }); }) @@ -187,19 +172,9 @@ export const usePipelineViewFetch = ( results.forEach((result) => { if (result && result?.status === "rejected") { // Handle rejected promises and add error messages to podsErr - podsErr.push({ - error: `${result.reason.response.status}: Failed to get pods count for ${result.reason.vertex} vertex`, - options: { - toastId: `${result.reason.vertex}-pods-fetch`, - autoClose: 5000, - }, - }); + addError(`Failed to get pods: ${result.reason.response.status}`); } }); - if (podsErr.length > 0) { - // Update podsErr state if there are any errors - setPodsErr(podsErr); - } }) .then(() => { if (!isEqual(vertexPods, vertexToPodsMap)) { @@ -207,13 +182,14 @@ export const usePipelineViewFetch = ( setVertexPods(vertexToPodsMap); } }) - .catch(console.error); + .catch((e: any) => { + addError(`Error: ${e.message}`); + }); } - }, [spec, requestKey]); + }, [spec, requestKey, addError]); const getVertexMetrics = useCallback(() => { const vertexToMetricsMap = new Map(); - const metricsErr: any[] = []; if (spec?.vertices && vertexPods.size > 0) { // Fetch metrics for all vertices together @@ -260,13 +236,9 @@ export const usePipelineViewFetch = ( ) { // Handle case when processingRates are not available for a vertex vertexMetrics.error = true; - metricsErr.push({ - error: `Failed to get metrics for ${vertexName} vertex`, - options: { - toastId: `${vertexName}-metrics`, - autoClose: 5000, - }, - }); + addError( + `Failed to get metrics for ${vertexName} vertex` + ); } } }); @@ -283,13 +255,7 @@ export const usePipelineViewFetch = ( }); } else if (json?.errMsg) { // Metrics API call returns an error message - metricsErr.push({ - error: json.errMsg, - options: { - toastId: "vertex-metrics-fetch-error", - autoClose: 5000, - }, - }); + addError(json.errMsg); } }), ]) @@ -297,21 +263,18 @@ export const usePipelineViewFetch = ( results.forEach((result) => { if (result && result?.status === "rejected") { // Handle rejected promises and add error messages to metricsErr - metricsErr.push({ - error: `${result.reason.response.status}: Failed to get metrics for some vertices`, - options: { toastId: `vertex-metrics-fetch`, autoClose: 5000 }, - }); + addError( + `Failed to get metrics: ${result.reason.response.status}` + ); } }); - if (metricsErr.length > 0) { - // Update metricsErr state if there are any errors - setMetricsErr(metricsErr); - } }) .then(() => setVertexMetrics(vertexToMetricsMap)) - .catch(console.error); + .catch((e: any) => { + addError(`Error: ${e.message}`); + }); } - }, [spec, vertexPods]); + }, [spec, vertexPods, addError]); // This useEffect is used to obtain metrics for a given vertex in a pipeline and refreshes every 1 minute useEffect(() => { @@ -325,7 +288,6 @@ export const usePipelineViewFetch = ( // This is used to obtain the watermark of a given pipeline const getPipelineWatermarks = useCallback(() => { const edgeToWatermarkMap = new Map(); - const watermarkErr: any[] = []; if (spec?.edges) { if (spec?.watermark?.disabled === true) { @@ -353,13 +315,7 @@ export const usePipelineViewFetch = ( }); } else if (json?.errMsg) { // Watermarks API call returns an error message - watermarkErr.push({ - error: json.errMsg, - options: { - toastId: "watermarks-fetch-error", - autoClose: 5000, - }, - }); + addError(json.errMsg); } }), ]) @@ -367,19 +323,17 @@ export const usePipelineViewFetch = ( results.forEach((result) => { if (result && result?.status === "rejected") { // Handle rejected promises and add error messages to watermarkErr - watermarkErr.push({ - error: `${result.reason.status}: Failed to get watermarks for some vertices`, - options: { toastId: "watermarks-fetch", autoClose: 5000 }, - }); + addError(`Failed to get watermarks: ${result.reason.status}`); } }); - if (watermarkErr.length > 0) setWatermarkErr(watermarkErr); }) .then(() => setEdgeWatermark(edgeToWatermarkMap)) - .catch(console.error); + .catch((e: any) => { + addError(`Error: ${e.message}`); + }); } } - }, [spec]); + }, [spec, addError]); // This useEffect is used to obtain watermark for a given vertex in a pipeline and refreshes every 1 minute useEffect(() => { @@ -693,9 +647,6 @@ export const usePipelineViewFetch = ( edges, pipelineErr, buffersErr, - podsErr, - metricsErr, - watermarkErr, loading, }; };