[server, dashboard] improve single prebuild view (gitpod-io#19445)

* [server] proper handle prebuild log stream response * Write error message to body * Check read_prebuild permission on organization level when get workspace * fixup * Update regex and add unit tests * Add persist error toast for prebuild errors * 💄 * Fix stopped workspace log * fix rerun * Ensure fga enabled like papi * Update components/dashboard/src/prebuilds/detail/PrebuildDetailPage.tsx Co-authored-by: Filip Troníček <[email protected]> * Address feedback --------- Co-authored-by: Filip Troníček <[email protected]>
ZDSRKJ · Feb 22, 2024 · bcd2e93 · bcd2e93
1 parent c13409e
commit bcd2e93
Show file tree

Hide file tree

Showing 9 changed files with 224 additions and 58 deletions.
diff --git a/components/dashboard/src/components/WorkspaceLogs.tsx b/components/dashboard/src/components/WorkspaceLogs.tsx
@@ -12,6 +12,7 @@ import { FitAddon } from "xterm-addon-fit";
 import "xterm/css/xterm.css";
 import { ThemeContext } from "../theme-context";
 import { cn } from "@podkit/lib/cn";
+import { LoadingState } from "@podkit/loading/LoadingState";
 
 const darkTheme: ITheme = {
     // What written on DevTool dark:bg-gray-800 is
@@ -28,6 +29,7 @@ export interface WorkspaceLogsProps {
     errorMessage?: string;
     classes?: string;
     xtermClasses?: string;
+    isLoading?: boolean;
 }
 
 export default function WorkspaceLogs(props: WorkspaceLogsProps) {
@@ -104,6 +106,11 @@ export default function WorkspaceLogs(props: WorkspaceLogsProps) {
                 "bg-gray-100 dark:bg-gray-800 relative",
             )}
         >
+            {props.isLoading && (
+                <div className="absolute top-2 right-2">
+                    <LoadingState delay={false} size={16} />
+                </div>
+            )}
             <div
                 className={cn(props.xtermClasses || "absolute top-0 left-0 bottom-0 right-0 m-6")}
                 ref={xTermParentRef}

diff --git a/components/dashboard/src/data/prebuilds/prebuild-logs-emitter.ts b/components/dashboard/src/data/prebuilds/prebuild-logs-emitter.ts
@@ -7,10 +7,16 @@
 import EventEmitter from "events";
 import { prebuildClient } from "../../service/public-api";
 import { useEffect, useState } from "react";
-import { onDownloadPrebuildLogsUrl } from "@gitpod/public-api-common/lib/prebuild-utils";
+import { matchPrebuildError, onDownloadPrebuildLogsUrl } from "@gitpod/public-api-common/lib/prebuild-utils";
 
 export function usePrebuildLogsEmitter(prebuildId: string) {
     const [emitter] = useState(new EventEmitter());
+    const [isLoading, setIsLoading] = useState(true);
+
+    useEffect(() => {
+        setIsLoading(true);
+    }, [prebuildId]);
+
     useEffect(() => {
         const controller = new AbortController();
         const watch = async () => {
@@ -25,11 +31,19 @@ export function usePrebuildLogsEmitter(prebuildId: string) {
             dispose = onDownloadPrebuildLogsUrl(
                 prebuild.prebuild.status.logUrl,
                 (msg) => {
-                    emitter.emit("logs", msg);
+                    const error = matchPrebuildError(msg);
+                    if (!error) {
+                        emitter.emit("logs", msg);
+                    } else {
+                        emitter.emit("logs-error", error);
+                    }
                 },
                 {
                     includeCredentials: true,
                     maxBackoffTimes: 3,
+                    onEnd: () => {
+                        setIsLoading(false);
+                    },
                 },
             );
         };
@@ -42,5 +56,5 @@ export function usePrebuildLogsEmitter(prebuildId: string) {
             controller.abort();
         };
     }, [emitter, prebuildId]);
-    return { emitter };
+    return { emitter, isLoading };
 }
diff --git a/components/dashboard/src/prebuilds/detail/PrebuildDetailPage.tsx b/components/dashboard/src/prebuilds/detail/PrebuildDetailPage.tsx
@@ -22,6 +22,7 @@ import Alert from "../../components/Alert";
 import { prebuildDisplayProps, prebuildStatusIconComponent } from "../../projects/prebuild-utils";
 import { LoadingButton } from "@podkit/buttons/LoadingButton";
 import { useConfiguration } from "../../data/configurations/configuration-queries";
+import { ApplicationError } from "@gitpod/gitpod-protocol/lib/messaging/error";
 
 const WorkspaceLogs = React.lazy(() => import("../../components/WorkspaceLogs"));
 
@@ -54,7 +55,7 @@ export const PrebuildDetailPage: FC = () => {
     const { toast } = useToast();
     const [currentPrebuild, setCurrentPrebuild] = useState<Prebuild | undefined>();
 
-    const { emitter: logEmitter } = usePrebuildLogsEmitter(prebuildId);
+    const { emitter: logEmitter, isLoading: isStreamingLogs } = usePrebuildLogsEmitter(prebuildId);
     const {
         isFetching: isTriggeringPrebuild,
         refetch: triggerPrebuild,
@@ -82,6 +83,9 @@ export const PrebuildDetailPage: FC = () => {
                 toast("Failed to fetch logs: " + err.message);
             }
         });
+        logEmitter.on("logs-error", (err: ApplicationError) => {
+            toast("Fetching logs failed: " + err.message, { autoHide: false });
+        });
     }, [logEmitter, toast]);
 
     useEffect(() => {
@@ -209,6 +213,7 @@ export const PrebuildDetailPage: FC = () => {
                                         classes="h-full w-full"
                                         xtermClasses="absolute top-0 left-0 bottom-0 right-0 mx-6 my-0"
                                         logsEmitter={logEmitter}
+                                        isLoading={isStreamingLogs}
                                     />
                                 </Suspense>
                             </div>

diff --git a/components/public-api/typescript-common/src/prebuild-utils.spec.ts b/components/public-api/typescript-common/src/prebuild-utils.spec.ts
@@ -0,0 +1,74 @@
+/**
+ * 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 { expect } from "chai";
+import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
+import { getPrebuildErrorMessage, matchPrebuildError } from "./prebuild-utils";
+
+describe("PrebuildUtils", () => {
+    describe("PrebuildErrorMessage", () => {
+        it("Application Error", async () => {
+            const errorList = [
+                new ApplicationError(ErrorCodes.UNIMPLEMENTED, "not implemented"),
+                new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "not implemented#"),
+                new ApplicationError(ErrorCodes.NOT_FOUND, "#not implemented#"),
+                new ApplicationError(ErrorCodes.NOT_FOUND, "#not#implemented#"),
+                new ApplicationError(ErrorCodes.NOT_FOUND, "12312#not#implemented#"),
+            ];
+            for (const err of errorList) {
+                const msg = getPrebuildErrorMessage(err);
+                const result = matchPrebuildError(msg);
+                expect(result).to.not.undefined;
+                expect(result?.code).to.equal(err.code);
+                expect(result?.message).to.equal(err.message);
+            }
+        });
+
+        it("Error", async () => {
+            const errorList = [
+                new Error("not implemented"),
+                new Error("not implemented#"),
+                new Error("#not implemented#"),
+                new Error("#not#implemented#"),
+                new Error("12312#not#implemented#"),
+            ];
+            for (const err of errorList) {
+                const msg = getPrebuildErrorMessage(err);
+                const result = matchPrebuildError(msg);
+                expect(result).to.not.undefined;
+                expect(result?.code).to.equal(ErrorCodes.INTERNAL_SERVER_ERROR);
+                expect(result?.message).to.equal("unexpected error");
+            }
+        });
+
+        it("others", async () => {
+            const errorList = [{}, [], Symbol("hello")];
+            for (const err of errorList) {
+                const msg = getPrebuildErrorMessage(err);
+                const result = matchPrebuildError(msg);
+                expect(result).to.not.undefined;
+                expect(result?.code).to.equal(ErrorCodes.INTERNAL_SERVER_ERROR);
+                expect(result?.message).to.equal("unknown error");
+            }
+        });
+    });
+
+    describe("matchPrebuildError", () => {
+        it("happy path", async () => {
+            const result = matchPrebuildError(
+                "X-Prebuild-Error#404#Headless logs for 07f877b5-631c-47c8-82aa-09de2f11fff3 not found#X-Prebuild-Error",
+            );
+            expect(result).to.not.undefined;
+            expect(result?.code).to.be.equal(404);
+            expect(result?.message).to.be.equal("Headless logs for 07f877b5-631c-47c8-82aa-09de2f11fff3 not found");
+        });
+
+        it("not match", async () => {
+            const result = matchPrebuildError("echo hello");
+            expect(result).to.be.undefined;
+        });
+    });
+});
diff --git a/components/public-api/typescript-common/src/prebuild-utils.ts b/components/public-api/typescript-common/src/prebuild-utils.ts
@@ -5,7 +5,7 @@
  */
 
 import { Disposable, DisposableCollection, HEADLESS_LOG_STREAM_STATUS_CODE_REGEX } from "@gitpod/gitpod-protocol";
-import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
+import { ApplicationError, ErrorCode, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";
 
 /**
  * new entry for the stream prebuild logs, contains logs of imageBuild (if it has) and prebuild tasks(first task only for now) logs
@@ -17,10 +17,35 @@ export function getPrebuildLogPath(prebuildId: string): string {
     return PREBUILD_LOGS_PATH_PREFIX + "/" + prebuildId;
 }
 
+/** cmp. @const HEADLESS_LOG_STREAM_ERROR_REGEX */
+const PREBUILD_LOG_STREAM_ERROR = "X-Prebuild-Error";
+const PREBUILD_LOG_STREAM_ERROR_REGEX = /X-Prebuild-Error#(?<code>[0-9]+)#(?<message>.*?)#X-Prebuild-Error/;
+
+export function matchPrebuildError(msg: string): undefined | ApplicationError {
+    const result = PREBUILD_LOG_STREAM_ERROR_REGEX.exec(msg);
+    if (!result || !result.groups) {
+        return;
+    }
+    return new ApplicationError(Number(result.groups.code) as ErrorCode, result.groups.message);
+}
+
+export function getPrebuildErrorMessage(err: any) {
+    let code: ErrorCode = ErrorCodes.INTERNAL_SERVER_ERROR;
+    let message = "unknown error";
+    if (err instanceof ApplicationError) {
+        code = err.code;
+        message = err.message;
+    } else if (err instanceof Error) {
+        message = "unexpected error";
+    }
+    return `${PREBUILD_LOG_STREAM_ERROR}#${code}#${message}#${PREBUILD_LOG_STREAM_ERROR}`;
+}
+
 const defaultBackoffTimes = 3;
 interface Options {
     includeCredentials: boolean;
     maxBackoffTimes?: number;
+    onEnd?: () => void;
 }
 
 /**
@@ -117,6 +142,9 @@ export function onDownloadPrebuildLogsUrl(
             await retryBackoff("error while listening to stream", err);
         } finally {
             reader?.cancel().catch(console.debug);
+            if (options.onEnd) {
+                options.onEnd();
+            }
         }
     };
     startWatchingLogs().catch(console.error);

diff --git a/components/server/src/prebuilds/prebuild-manager.ts b/components/server/src/prebuilds/prebuild-manager.ts
@@ -631,13 +631,14 @@ export class PrebuildManager {
     }
 
     public async watchPrebuildLogs(userId: string, prebuildId: string, onLog: (message: string) => void) {
-        const workspaceId = await this.waitUntilPrebuildWorkspaceCreated(userId, prebuildId);
-        if (!workspaceId) {
+        const { workspaceId, organizationId } = await this.waitUntilPrebuildWorkspaceCreated(userId, prebuildId);
+        if (!workspaceId || !organizationId) {
             throw new ApplicationError(ErrorCodes.PRECONDITION_FAILED, "prebuild workspace not found");
         }
-
+        await this.auth.checkPermissionOnOrganization(userId, "read_prebuild", organizationId);
         const workspaceStatusIt = this.workspaceService.getAndWatchWorkspaceStatus(userId, workspaceId, {
             signal: ctxSignal(),
+            skipPermissionCheck: true,
         });
         let hasImageBuild = false;
         for await (const itWsInfo of workspaceStatusIt) {
@@ -716,6 +717,9 @@ export class PrebuildManager {
                                         {
                                             includeCredentials: false,
                                             maxBackoffTimes: 3,
+                                            onEnd: () => {
+                                                sink.stop();
+                                            },
                                         },
                                     );
                                     return () => {
@@ -752,20 +756,22 @@ export class PrebuildManager {
     }
 
     private async waitUntilPrebuildWorkspaceCreated(userId: string, prebuildId: string) {
-        let prebuildWorkspaceId: string | undefined;
+        let workspaceId: string | undefined;
+        let organizationId: string | undefined;
         const prebuildIt = this.getAndWatchPrebuildStatus(userId, { prebuildId }, { signal: ctxSignal() });
 
         for await (const pb of prebuildIt) {
-            prebuildWorkspaceId = pb.info.buildWorkspaceId;
-            if (prebuildWorkspaceId) {
+            workspaceId = pb.info.buildWorkspaceId;
+            organizationId = pb.info.teamId;
+            if (workspaceId) {
                 break;
             }
             if (pb.status === "aborted" || pb.status === "failed" || pb.status === "timeout") {
                 break;
             }
         }
         await prebuildIt.return();
-        return prebuildWorkspaceId;
+        return { workspaceId, organizationId };
     }
 
     private parsePrebuildLogUrl(url: string) {

diff --git a/components/server/src/util/request-context.ts b/components/server/src/util/request-context.ts
@@ -203,6 +203,17 @@ export function runWithSubjectId<T>(subjectId: SubjectId | undefined, fun: () =>
     return runWithContext({ ...parent, subjectId }, fun);
 }
 
+export function runWithSubSignal<T>(abortController: AbortController, fun: () => T): T {
+    const parent = ctxTryGet();
+    if (!parent) {
+        throw new Error("runWithChildContext: No parent context available");
+    }
+    ctxOnAbort(() => {
+        abortController.abort("runWithSubSignal: Parent context signal aborted");
+    });
+    return runWithContext({ ...parent, signal: abortController.signal }, fun);
+}
+
 function runWithContext<C extends RequestContext, T>(context: C, fun: () => T): T {
     return asyncLocalStorage.run(context, fun);
 }