[dashboard, server] Fix streaming of image build logs (#20095)

* [json-rpc] Fix encoding of watchImageBuildLogs data to number[] Because json-rpc can't handle complex objects like UInt8Array properly. Conversion is done using "Array.from(UInt8Array)" and "new UInt8Array(data)" * [dashboard] Fix "workspaceId is required" errors * Review suggestions Co-authored-by: Filip Troníček <[email protected]> --------- Co-authored-by: Filip Troníček <[email protected]>
gitpod-io · Aug 8, 2024 · b43c97e · b43c97e
1 parent 4f6e87c
commit b43c97e
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 49 deletions.
diff --git a/components/dashboard/src/components/PrebuildLogs.tsx b/components/dashboard/src/components/PrebuildLogs.tsx
@@ -100,10 +100,11 @@ export default function PrebuildLogs(props: PrebuildLogsProps) {
                         info: WorkspaceImageBuild.StateInfo,
                         content?: WorkspaceImageBuild.LogContent,
                     ) => {
-                        if (!content) {
+                        if (!content?.data) {
                             return;
                         }
-                        logsEmitter.emit("logs", content.data);
+                        const uintArray = new Uint8Array(content.data);
+                        logsEmitter.emit("logs", uintArray);
                     },
                 }),
             );

diff --git a/components/dashboard/src/data/workspaces/default-workspace-image-query.ts b/components/dashboard/src/data/workspaces/default-workspace-image-query.ts
@@ -8,12 +8,16 @@ import { useQuery } from "@tanstack/react-query";
 import { GetWorkspaceDefaultImageResponse } from "@gitpod/public-api/lib/gitpod/v1/workspace_pb";
 import { workspaceClient } from "../../service/public-api";
 
-export const useWorkspaceDefaultImageQuery = (workspaceId: string) => {
-    return useQuery<GetWorkspaceDefaultImageResponse>({
-        queryKey: ["default-workspace-image-v2", { workspaceId }],
+export const useWorkspaceDefaultImageQuery = (workspaceId?: string) => {
+    return useQuery<GetWorkspaceDefaultImageResponse | null, Error, GetWorkspaceDefaultImageResponse | undefined>({
+        queryKey: ["default-workspace-image-v2", { workspaceId: workspaceId || "undefined" }],
         staleTime: 1000 * 60 * 10, // 10 minute
         queryFn: async () => {
+            if (!workspaceId) {
+                return null; // no workspaceId, no image. Using null because "undefined" is not persisted by react-query
+            }
             return await workspaceClient.getWorkspaceDefaultImage({ workspaceId });
         },
+        select: (data) => data || undefined,
     });
 };
diff --git a/components/dashboard/src/start/StartPage.tsx b/components/dashboard/src/start/StartPage.tsx
@@ -132,7 +132,7 @@ function StartError(props: { error: StartWorkspaceError }) {
 }
 
 function WarningView(props: { workspaceId?: string; showLatestIdeWarning?: boolean; error?: StartWorkspaceError }) {
-    const { data: imageInfo } = useWorkspaceDefaultImageQuery(props.workspaceId ?? "");
+    const { data: imageInfo } = useWorkspaceDefaultImageQuery(props.workspaceId);
     let useWarning: "latestIde" | "orgImage" | undefined = props.showLatestIdeWarning ? "latestIde" : undefined;
     if (
         props.error &&

diff --git a/components/dashboard/src/start/StartWorkspace.tsx b/components/dashboard/src/start/StartWorkspace.tsx
@@ -322,8 +322,8 @@ export default class StartWorkspace extends React.Component<StartWorkspaceProps,
         this.setState({ ideOptions });
     }
 
-    private async onWorkspaceUpdate(workspace: Workspace) {
-        if (!workspace.status?.instanceId) {
+    private async onWorkspaceUpdate(workspace?: Workspace) {
+        if (!workspace?.status?.instanceId || !workspace.id) {
             return;
         }
         // Here we filter out updates to instances we haven't started to avoid issues with updates coming in out-of-order
@@ -791,10 +791,11 @@ function ImageBuildView(props: ImageBuildViewProps) {
                 info: WorkspaceImageBuild.StateInfo,
                 content?: WorkspaceImageBuild.LogContent,
             ) => {
-                if (!content) {
+                if (!content?.data) {
                     return;
                 }
-                logsEmitter.emit("logs", content.data);
+                const chunk = new Uint8Array(content.data);
+                logsEmitter.emit("logs", chunk);
             },
         });
 

diff --git a/components/gitpod-protocol/src/protocol.ts b/components/gitpod-protocol/src/protocol.ts
@@ -974,7 +974,7 @@ export namespace WorkspaceImageBuild {
         maxSteps?: number;
     }
     export interface LogContent {
-        data: Uint8Array;
+        data: number[]; // encode with "Array.from(UInt8Array)"", decode with "new UInt8Array(data)"
     }
     export type LogCallback = (info: StateInfo, content: LogContent | undefined) => void;
     export namespace LogLine {

diff --git a/components/server/src/workspace/gitpod-server-impl.ts b/components/server/src/workspace/gitpod-server-impl.ts
@@ -100,6 +100,7 @@ import {
     ProjectEnvVar,
     UserEnvVar,
     UserFeatureSettings,
+    WorkspaceImageBuild,
     WorkspaceTimeoutSetting,
 } from "@gitpod/gitpod-protocol/lib/protocol";
 import { ListUsageRequest, ListUsageResponse } from "@gitpod/gitpod-protocol/lib/usage";
@@ -1096,7 +1097,12 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         const teamMembers = await this.organizationService.listMembers(user.id, workspace.organizationId);
         await this.guardAccess({ kind: "workspaceLog", subject: workspace, teamMembers }, "get");
 
-        await this.workspaceService.watchWorkspaceImageBuildLogs(user.id, workspaceId, client);
+        const receiver = async (chunk: Uint8Array) => {
+            client.onWorkspaceImageBuildLogs(undefined as any as WorkspaceImageBuild.StateInfo, {
+                data: Array.from(chunk), // json-rpc can't handle objects, so we convert back-and-forth here
+            });
+        };
+        await this.workspaceService.watchWorkspaceImageBuildLogs(user.id, workspaceId, receiver);
     }
 
     async getHeadlessLog(ctx: TraceContext, instanceId: string): Promise<HeadlessLogUrls> {

diff --git a/components/server/src/workspace/headless-log-controller.ts b/components/server/src/workspace/headless-log-controller.ts
@@ -12,7 +12,6 @@ import {
     TeamMemberInfo,
     User,
     Workspace,
-    WorkspaceImageBuild,
     WorkspaceInstance,
 } from "@gitpod/gitpod-protocol";
 import { log, LogContext } from "@gitpod/gitpod-protocol/lib/util/logging";
@@ -183,20 +182,14 @@ export class HeadlessLogController {
                         res,
                         "image-build",
                     );
-                    const client = {
-                        onWorkspaceImageBuildLogs: async (
-                            _info: WorkspaceImageBuild.StateInfo,
-                            content?: WorkspaceImageBuild.LogContent,
-                        ) => {
-                            if (!content) return;
-
-                            await writeToResponse(content.data);
-                        },
-                    };
 
                     try {
                         await runWithSubSignal(abortController, async () => {
-                            await this.workspaceService.watchWorkspaceImageBuildLogs(user.id, workspaceId, client);
+                            await this.workspaceService.watchWorkspaceImageBuildLogs(
+                                user.id,
+                                workspaceId,
+                                writeToResponse,
+                            );
                         });
 
                         // Wait until we finished writing all chunks in our queue

diff --git a/components/server/src/workspace/workspace-service.spec.db.ts b/components/server/src/workspace/workspace-service.spec.db.ts
@@ -12,7 +12,6 @@ import {
     Project,
     User,
     WorkspaceConfig,
-    WorkspaceImageBuild,
     WorkspaceInstancePort,
 } from "@gitpod/gitpod-protocol";
 import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
@@ -489,43 +488,33 @@ describe("WorkspaceService", async () => {
     it("should watchWorkspaceImageBuildLogs", async () => {
         const svc = container.get(WorkspaceService);
         const ws = await createTestWorkspace(svc, org, owner, project);
-        const client = {
-            onWorkspaceImageBuildLogs: (
-                info: WorkspaceImageBuild.StateInfo,
-                content: WorkspaceImageBuild.LogContent | undefined,
-            ) => {},
-        };
+        const receiver = async (chunk: Uint8Array) => {};
 
-        await svc.watchWorkspaceImageBuildLogs(owner.id, ws.id, client); // returns without error in case of non-running workspace
+        await svc.watchWorkspaceImageBuildLogs(owner.id, ws.id, receiver); // returns without error in case of non-running workspace
 
         await expectError(
             ErrorCodes.PERMISSION_DENIED,
-            svc.watchWorkspaceImageBuildLogs(member.id, ws.id, client),
+            svc.watchWorkspaceImageBuildLogs(member.id, ws.id, receiver),
             "should fail for member on not-shared workspace",
         );
 
         await expectError(
             ErrorCodes.NOT_FOUND,
-            svc.watchWorkspaceImageBuildLogs(stranger.id, ws.id, client),
+            svc.watchWorkspaceImageBuildLogs(stranger.id, ws.id, receiver),
             "should fail for stranger on not-shared workspace",
         );
     });
 
     it("should watchWorkspaceImageBuildLogs - shared", async () => {
         const svc = container.get(WorkspaceService);
         const ws = await createTestWorkspace(svc, org, owner, project);
-        const client = {
-            onWorkspaceImageBuildLogs: (
-                info: WorkspaceImageBuild.StateInfo,
-                content: WorkspaceImageBuild.LogContent | undefined,
-            ) => {},
-        };
+        const receiver = async (chunk: Uint8Array) => {};
 
         await svc.controlAdmission(owner.id, ws.id, "everyone");
 
-        await svc.watchWorkspaceImageBuildLogs(owner.id, ws.id, client); // returns without error in case of non-running workspace
-        await svc.watchWorkspaceImageBuildLogs(member.id, ws.id, client);
-        await svc.watchWorkspaceImageBuildLogs(stranger.id, ws.id, client);
+        await svc.watchWorkspaceImageBuildLogs(owner.id, ws.id, receiver); // returns without error in case of non-running workspace
+        await svc.watchWorkspaceImageBuildLogs(member.id, ws.id, receiver);
+        await svc.watchWorkspaceImageBuildLogs(stranger.id, ws.id, receiver);
     });
 
     it("should sendHeartBeat", async () => {

diff --git a/components/server/src/workspace/workspace-service.ts b/components/server/src/workspace/workspace-service.ts
@@ -10,7 +10,6 @@ import { RedisPublisher, WorkspaceDB } from "@gitpod/gitpod-db/lib";
 import {
     CommitContext,
     GetWorkspaceTimeoutResult,
-    GitpodClient,
     GitpodServer,
     HeadlessLogUrls,
     PortProtocol,
@@ -1116,7 +1115,7 @@ export class WorkspaceService {
     public async watchWorkspaceImageBuildLogs(
         userId: string,
         workspaceId: string,
-        client: Pick<GitpodClient, "onWorkspaceImageBuildLogs">,
+        receiver: (chunk: Uint8Array) => Promise<void>,
     ): Promise<void> {
         // check access
         await this.getWorkspace(userId, workspaceId);
@@ -1167,10 +1166,7 @@ export class WorkspaceService {
                 }
 
                 try {
-                    // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-                    client.onWorkspaceImageBuildLogs(undefined as any, {
-                        data: chunk,
-                    });
+                    await receiver(chunk);
                 } catch (err) {
                     log.error("error while streaming imagebuild logs", err);
                     aborted.resolve(true);