Cleanup watching prebuilds (#19969)

filiptronicek · web-flow · commit e6f07ee95060 · 2024-06-28T12:36:23.000+02:00
* Fix dropped errors on available prebuilds

* Watch prebuild cleanup

* Remove extra file change from merge resolve

* Make not found errors task-specific

* Remove suspense where it's not needed

* Fix doubled logs
diff --git a/components/dashboard/src/data/prebuilds/prebuild-logs-emitter.ts b/components/dashboard/src/data/prebuilds/prebuild-logs-emitter.ts
@@ -5,7 +5,7 @@
  */
 
 import { prebuildClient } from "../../service/public-api";
-import { useEffect, useState } from "react";
+import { useEffect, useMemo, useState } from "react";
 import { matchPrebuildError, onDownloadPrebuildLogsUrl } from "@gitpod/public-api-common/lib/prebuild-utils";
 import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
 import { ReplayableEventEmitter } from "../../utils";
@@ -23,7 +23,12 @@ type LogEventTypes = {
  * @param taskId ID of the task to watch.
  */
 export function usePrebuildLogsEmitter(prebuildId: string, taskId: string) {
-    const [emitter] = useState(new ReplayableEventEmitter<LogEventTypes>());
+    const emitter = useMemo(
+        () => new ReplayableEventEmitter<LogEventTypes>(),
+        // We would like to re-create the emitter when the prebuildId or taskId changes, so that logs of old tasks / prebuilds are not mixed with the new ones.
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+        [prebuildId, taskId],
+    );
     const [disposable, setDisposable] = useState<Disposable | undefined>();
 
     useEffect(() => {
diff --git a/components/dashboard/src/prebuilds/detail/PrebuildDetailPage.tsx b/components/dashboard/src/prebuilds/detail/PrebuildDetailPage.tsx
@@ -6,9 +6,8 @@
 
 import { Prebuild, PrebuildPhase_Phase, TaskLog } from "@gitpod/public-api/lib/gitpod/v1/prebuild_pb";
 import { BreadcrumbNav } from "@podkit/breadcrumbs/BreadcrumbNav";
-import { Text } from "@podkit/typography/Text";
 import { Button } from "@podkit/buttons/Button";
-import { FC, Suspense, useCallback, useEffect, useMemo, useState } from "react";
+import { FC, useCallback, useEffect, useMemo, useState } from "react";
 import { Redirect, useHistory, useParams } from "react-router";
 import dayjs from "dayjs";
 import { useToast } from "../../components/toasts/Toasts";
@@ -26,9 +25,10 @@ import Alert from "../../components/Alert";
 import { PrebuildStatus } from "../../projects/prebuild-utils";
 import { LoadingButton } from "@podkit/buttons/LoadingButton";
 import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
-import { Tabs, TabsContent, TabsList, TabsTrigger } from "@podkit/tabs/Tabs";
+import { Tabs, TabsList, TabsTrigger } from "@podkit/tabs/Tabs";
 import { PrebuildTaskTab } from "./PrebuildTaskTab";
 import type { PlainMessage } from "@bufbuild/protobuf";
+import { PrebuildTaskErrorTab } from "./PrebuildTaskErrorTab";
 
 /**
  * Formats a date. For today, it returns the time. For this year, it returns the month and day and time. Otherwise, it returns the full date and time.
@@ -52,29 +52,28 @@ interface Props {
 export const PrebuildDetailPage: FC = () => {
     const { prebuildId } = useParams<Props>();
 
-    const { data: prebuild, isLoading: isInfoLoading, error, refetch } = usePrebuildQuery(prebuildId);
+    const { data: initialPrebuild, isLoading: isInfoLoading, error, refetch } = usePrebuildQuery(prebuildId);
+    const [currentPrebuild, setCurrentPrebuild] = useState<Prebuild | undefined>();
+    const prebuild = currentPrebuild ?? initialPrebuild;
 
     const history = useHistory();
     const { toast, dismissToast } = useToast();
-    const [currentPrebuild, setCurrentPrebuild] = useState<Prebuild | undefined>();
-    const [logNotFound, setLogNotFound] = useState(false);
     const [selectedTaskId, actuallySetSelectedTaskId] = useState<string | undefined>(
         window.location.hash.slice(1) || undefined,
     );
 
     const isImageBuild =
-        currentPrebuild?.status?.phase?.name === PrebuildPhase_Phase.QUEUED &&
-        !!currentPrebuild.status.imageBuildLogUrl;
+        prebuild?.status?.phase?.name === PrebuildPhase_Phase.QUEUED && !!prebuild.status.imageBuildLogUrl;
     const taskId = useMemo(() => {
-        if (!currentPrebuild) {
+        if (!prebuild) {
             return undefined;
         }
         if (isImageBuild) {
             return "image-build";
         }
 
-        return selectedTaskId ?? currentPrebuild?.status?.taskLogs.filter((f) => f.logUrl)[0]?.taskId ?? undefined;
-    }, [currentPrebuild, isImageBuild, selectedTaskId]);
+        return selectedTaskId ?? prebuild?.status?.taskLogs.filter((f) => f.logUrl)[0]?.taskId ?? undefined;
+    }, [isImageBuild, prebuild, selectedTaskId]);
 
     const {
         isFetching: isTriggeringPrebuild,
@@ -101,11 +100,7 @@ export const PrebuildDetailPage: FC = () => {
     );
 
     useEffect(() => {
-        setLogNotFound(false);
         const disposable = watchPrebuild(prebuildId, (prebuild) => {
-            if (currentPrebuild?.status?.phase?.name === PrebuildPhase_Phase.ABORTED) {
-                return true;
-            }
             setCurrentPrebuild(prebuild);
 
             return isPrebuildDone(prebuild);
@@ -114,21 +109,21 @@ export const PrebuildDetailPage: FC = () => {
         return () => {
             disposable.dispose();
         };
-    }, [prebuildId, currentPrebuild?.status?.phase?.name]);
+    }, [prebuildId]);
 
     const prebuildTasks = useMemo(() => {
         const validTasks: Omit<PlainMessage<TaskLog>, "taskJson">[] =
-            currentPrebuild?.status?.taskLogs.filter((t) => t.logUrl) ?? [];
+            prebuild?.status?.taskLogs.filter((t) => t.logUrl) ?? [];
         if (isImageBuild) {
             validTasks.unshift({
                 taskId: "image-build",
                 taskLabel: "Image Build",
-                logUrl: currentPrebuild?.status?.imageBuildLogUrl!, // we know this is defined because we're in the isImageBuild branch
+                logUrl: prebuild?.status?.imageBuildLogUrl!, // we know this is defined because we're in the isImageBuild branch
             });
         }
 
         return validTasks;
-    }, [currentPrebuild?.status, isImageBuild]);
+    }, [isImageBuild, prebuild?.status?.imageBuildLogUrl, prebuild?.status?.taskLogs]);
 
     useEffect(() => {
         history.listen(() => {
@@ -157,9 +152,8 @@ export const PrebuildDetailPage: FC = () => {
         }
     }, [prebuild, cancelPrebuildMutation]);
 
-    const isError = logNotFound || prebuildTasks.length === 0;
-
-    if (newPrebuildID) {
+    // For some reason, we sometimes hit a case where the newPrebuildID is actually set without us triggering the query.
+    if (newPrebuildID && prebuild?.id !== newPrebuildID) {
         return <Redirect to={repositoriesRoutes.PrebuildDetail(newPrebuildID)} />;
     }
 
@@ -201,8 +195,7 @@ export const PrebuildDetailPage: FC = () => {
                         )}
                     </div>
                 ) : (
-                    prebuild &&
-                    currentPrebuild && (
+                    prebuild && (
                         <div className={"border border-pk-border-base rounded-xl py-6 divide-y"}>
                             <div className="px-6 pb-4">
                                 <div className="flex flex-col gap-2">
@@ -236,10 +229,10 @@ export const PrebuildDetailPage: FC = () => {
                             </div>
                             <div className="flex flex-col gap-1 border-pk-border-base">
                                 <div className="py-4 px-6 flex flex-col gap-1">
-                                    <PrebuildStatus prebuild={currentPrebuild} />
-                                    {currentPrebuild?.status?.message && (
+                                    <PrebuildStatus prebuild={prebuild} />
+                                    {prebuild?.status?.message && (
                                         <div className="text-pk-content-secondary truncate">
-                                            {currentPrebuild?.status.message}
+                                            {prebuild?.status.message}
                                         </div>
                                     )}
                                 </div>
@@ -257,54 +250,20 @@ export const PrebuildDetailPage: FC = () => {
                                             </TabsTrigger>
                                         ))}
                                     </TabsList>
-                                    {!isError ? (
+                                    {prebuildTasks.length !== 0 ? (
                                         prebuildTasks.map(({ taskId }) => (
-                                            <PrebuildTaskTab
-                                                key={taskId}
-                                                taskId={taskId}
-                                                prebuild={currentPrebuild}
-                                                onLogNotFound={() => setLogNotFound(true)}
-                                            />
+                                            <PrebuildTaskTab key={taskId} taskId={taskId} prebuild={prebuild} />
                                         ))
                                     ) : (
-                                        <TabsContent
-                                            value={taskId ?? "empty-tab"}
-                                            className="h-112 mt-0 border-pk-border-base"
-                                        >
-                                            <Suspense fallback={<div />}>
-                                                <div className="px-6 py-4 h-full w-full bg-pk-surface-primary text-base flex items-center justify-center">
-                                                    <Text className="w-80 text-center">
-                                                        {logNotFound ? (
-                                                            <>
-                                                                Logs of this prebuild are inaccessible. Use{" "}
-                                                                <code>gp validate --prebuild --headless</code> in a
-                                                                workspace to see logs and debug prebuild issues.{" "}
-                                                                <a
-                                                                    href="https://www.gitpod.io/docs/configure/workspaces#validate-your-gitpod-configuration"
-                                                                    target="_blank"
-                                                                    rel="noreferrer noopener"
-                                                                    className="gp-link"
-                                                                >
-                                                                    Learn more
-                                                                </a>
-                                                                .
-                                                            </>
-                                                        ) : (
-                                                            <>
-                                                                No prebuild tasks defined in <code>.gitpod.yml</code>{" "}
-                                                                for this prebuild
-                                                            </>
-                                                        )}
-                                                    </Text>
-                                                </div>
-                                            </Suspense>
-                                        </TabsContent>
+                                        <PrebuildTaskErrorTab>
+                                            No prebuild tasks defined in <code>.gitpod.yml</code> for this prebuild
+                                        </PrebuildTaskErrorTab>
                                     )}
                                 </Tabs>
                             </div>
                             <div className="px-6 pt-6 flex justify-between border-pk-border-base">
                                 {[PrebuildPhase_Phase.BUILDING, PrebuildPhase_Phase.QUEUED].includes(
-                                    currentPrebuild?.status?.phase?.name ?? PrebuildPhase_Phase.UNSPECIFIED,
+                                    prebuild?.status?.phase?.name ?? PrebuildPhase_Phase.UNSPECIFIED,
                                 ) ? (
                                     <LoadingButton
                                         loading={cancelPrebuildMutation.isLoading}
diff --git a/components/dashboard/src/prebuilds/detail/PrebuildTaskErrorTab.tsx b/components/dashboard/src/prebuilds/detail/PrebuildTaskErrorTab.tsx
@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) 2024 Gitpod GmbH. All rights reserved.
+ * Licensed under the GNU Affero General Public License (AGPL).
+ * See License.AGPL.txt in the project root for license information.
+ */
+
+import { TabsContent } from "@podkit/tabs/Tabs";
+import { PropsWithChildren } from "react";
+import { Text } from "@podkit/typography/Text";
+
+type Props = {
+    taskId?: string;
+};
+export const PrebuildTaskErrorTab = ({ taskId, children }: PropsWithChildren<Props>) => {
+    return (
+        <TabsContent value={taskId ?? "empty-tab"} className="h-112 mt-0 border-pk-border-base">
+            <div className="px-6 py-4 h-full w-full bg-pk-surface-primary text-base flex items-center justify-center">
+                <Text className="w-80 text-center">{children}</Text>
+            </div>
+        </TabsContent>
+    );
+};
diff --git a/components/dashboard/src/prebuilds/detail/PrebuildTaskTab.tsx b/components/dashboard/src/prebuilds/detail/PrebuildTaskTab.tsx
@@ -12,16 +12,17 @@ import { useToast } from "../../components/toasts/Toasts";
 import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
 import { TabsContent } from "@podkit/tabs/Tabs";
 import { PersistedToastID } from "./PrebuildDetailPage";
+import { PrebuildTaskErrorTab } from "./PrebuildTaskErrorTab";
 
 const WorkspaceLogs = React.lazy(() => import("../../components/WorkspaceLogs"));
 
 type Props = {
     taskId: string;
     prebuild: Prebuild;
-    onLogNotFound: () => void;
 };
-export const PrebuildTaskTab = ({ taskId, prebuild, onLogNotFound }: Props) => {
+export const PrebuildTaskTab = ({ taskId, prebuild }: Props) => {
     const { emitter: logEmitter, disposable: disposeStreamingLogs } = usePrebuildLogsEmitter(prebuild.id, taskId);
+    const [error, setError] = React.useState<ApplicationError | undefined>();
     const { toast } = useToast();
 
     useEffect(() => {
@@ -36,7 +37,7 @@ export const PrebuildTaskTab = ({ taskId, prebuild, onLogNotFound }: Props) => {
                 return;
             }
             if (err instanceof ApplicationError && err.code === ErrorCodes.NOT_FOUND) {
-                // We don't want to show a toast for this error, because it's handled by `notFoundError` in `PrebuildDetailPage`
+                // We don't want to show a toast for this error, we handle it in the UI
                 return;
             }
             if (err?.message) {
@@ -46,7 +47,7 @@ export const PrebuildTaskTab = ({ taskId, prebuild, onLogNotFound }: Props) => {
 
         const logErrorListener = (err: ApplicationError) => {
             if (err.code === ErrorCodes.NOT_FOUND) {
-                onLogNotFound();
+                setError(err);
                 return;
             }
 
@@ -59,8 +60,27 @@ export const PrebuildTaskTab = ({ taskId, prebuild, onLogNotFound }: Props) => {
         return () => {
             logEmitter.removeListener("error", errorListener);
             logEmitter.removeListener("logs-error", logErrorListener);
+            setError(undefined);
         };
-    }, [logEmitter, onLogNotFound, taskId, toast]);
+    }, [logEmitter, taskId, toast]);
+
+    if (error) {
+        return (
+            <PrebuildTaskErrorTab taskId={taskId}>
+                Logs of this prebuild task are inaccessible. Use <code>gp validate --prebuild --headless</code> in a
+                workspace to see logs and debug prebuild issues.{" "}
+                <a
+                    href="https://www.gitpod.io/docs/configure/workspaces#validate-your-gitpod-configuration"
+                    target="_blank"
+                    rel="noreferrer noopener"
+                    className="gp-link"
+                >
+                    Learn more
+                </a>
+                .
+            </PrebuildTaskErrorTab>
+        );
+    }
 
     return (
         <TabsContent value={taskId} className="h-112 mt-0 border-pk-border-base">
