From 9a4c946178cebf21b65b3bb46bc24fbfe44eb974 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 3 Jul 2024 10:17:54 -0400 Subject: [PATCH 1/3] fix: let workspace pages download partial logs for unhealthy workspaces (#13761) * fix: get basic fix in for preventing download logs from blowing up UI * fix: make sure blob units can't go out of bounds * fix: make sure timeout is cleared on component unmount * fix: reduce risk of shared cache state breaking useAgentLogs * fix: allow partial downloading of logs * fix: make sure useMemo cache is used properly * wip: commit current progress on updated logs functionality * docs: rewrite comment for clarity * refactor: clean up current code * fix: update styles for unavailable logs * fix: resolve linter violations * fix: update type signature of getErrorDetail * fix: revert log/enabled logic for useAgentLogs * fix: remove memoization from DownloadLogsDialog * fix: update name of timeout state * refactor: make log web sockets logic more clear * docs: reword comment for clarity * fix: commit current style update progress * fix: finish style updates (cherry picked from commit 940afa1ab1d273cddaef52c1a27616ce71d018bb) --- site/src/api/errors.ts | 7 +- .../resources/AgentLogs/useAgentLogs.ts | 31 +- .../WorkspaceActions/DownloadLogsDialog.tsx | 265 ++++++++++++++---- 3 files changed, 234 insertions(+), 69 deletions(-) diff --git a/site/src/api/errors.ts b/site/src/api/errors.ts index ada591d754fb2..621b19856601b 100644 --- a/site/src/api/errors.ts +++ b/site/src/api/errors.ts @@ -110,15 +110,18 @@ export const getValidationErrorMessage = (error: unknown): string => { return validationErrors.map((error) => error.detail).join("\n"); }; -export const getErrorDetail = (error: unknown): string | undefined | null => { +export const getErrorDetail = (error: unknown): string | undefined => { if (error instanceof Error) { return "Please check the developer console for more details."; } + if (isApiError(error)) { return error.response.data.detail; } + if (isApiErrorResponse(error)) { return error.detail; } - return null; + + return undefined; }; diff --git a/site/src/modules/resources/AgentLogs/useAgentLogs.ts b/site/src/modules/resources/AgentLogs/useAgentLogs.ts index e5d797a14e9c2..943dfcc194396 100644 --- a/site/src/modules/resources/AgentLogs/useAgentLogs.ts +++ b/site/src/modules/resources/AgentLogs/useAgentLogs.ts @@ -15,22 +15,35 @@ export type UseAgentLogsOptions = Readonly<{ enabled?: boolean; }>; +/** + * Defines a custom hook that gives you all workspace agent logs for a given + * workspace. + * + * Depending on the status of the workspace, all logs may or may not be + * available. + */ export function useAgentLogs( options: UseAgentLogsOptions, ): readonly WorkspaceAgentLog[] | undefined { const { workspaceId, agentId, agentLifeCycleState, enabled = true } = options; + const queryClient = useQueryClient(); const queryOptions = agentLogs(workspaceId, agentId); - const query = useQuery({ - ...queryOptions, - enabled, - }); - const logs = query.data; + const { data: logs, isFetched } = useQuery({ ...queryOptions, enabled }); + // Track the ID of the last log received when the initial logs response comes + // back. If the logs are not complete, the ID will mark the start point of the + // Web sockets response so that the remaining logs can be received over time const lastQueriedLogId = useRef(0); useEffect(() => { - if (logs && lastQueriedLogId.current === 0) { - lastQueriedLogId.current = logs[logs.length - 1].id; + const isAlreadyTracking = lastQueriedLogId.current !== 0; + if (isAlreadyTracking) { + return; + } + + const lastLog = logs?.at(-1); + if (lastLog !== undefined) { + lastQueriedLogId.current = lastLog.id; } }, [logs]); @@ -42,7 +55,7 @@ export function useAgentLogs( }); useEffect(() => { - if (agentLifeCycleState !== "starting" || !query.isFetched) { + if (agentLifeCycleState !== "starting" || !isFetched) { return; } @@ -69,7 +82,7 @@ export function useAgentLogs( return () => { socket.close(); }; - }, [addLogs, agentId, agentLifeCycleState, query.isFetched]); + }, [addLogs, agentId, agentLifeCycleState, isFetched]); return logs; } diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/DownloadLogsDialog.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/DownloadLogsDialog.tsx index aefd4d7d6b9e1..ab1ee817a9de7 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/DownloadLogsDialog.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/DownloadLogsDialog.tsx @@ -2,10 +2,11 @@ import { useTheme, type Interpolation, type Theme } from "@emotion/react"; import Skeleton from "@mui/material/Skeleton"; import { saveAs } from "file-saver"; import JSZip from "jszip"; -import { useMemo, useState, type FC } from "react"; +import { type FC, useMemo, useState, useRef, useEffect } from "react"; import { useQueries, useQuery } from "react-query"; import { agentLogs, buildLogs } from "api/queries/workspaces"; import type { Workspace, WorkspaceAgent } from "api/typesGenerated"; +import { Alert } from "components/Alert/Alert"; import { ConfirmDialog, type ConfirmDialogProps, @@ -28,70 +29,107 @@ type DownloadableFile = { export const DownloadLogsDialog: FC = ({ workspace, + open, + onClose, download = saveAs, - ...dialogProps }) => { const theme = useTheme(); - const agents = selectAgents(workspace); - const agentLogResults = useQueries({ - queries: agents.map((a) => ({ - ...agentLogs(workspace.id, a.id), - enabled: dialogProps.open, - })), - }); + const buildLogsQuery = useQuery({ ...buildLogs(workspace), - enabled: dialogProps.open, + enabled: open, }); - const downloadableFiles: DownloadableFile[] = useMemo(() => { - const files: DownloadableFile[] = [ - { - name: `${workspace.name}-build-logs.txt`, - blob: buildLogsQuery.data - ? new Blob([buildLogsQuery.data.map((l) => l.output).join("\n")], { - type: "text/plain", - }) - : undefined, - }, - ]; - - agents.forEach((a, i) => { + + const allUniqueAgents = useMemo(() => { + const allAgents = workspace.latest_build.resources.flatMap( + (resource) => resource.agents ?? [], + ); + + // Can't use the "new Set()" trick because we're not dealing with primitives + const uniqueAgents = new Map(allAgents.map((agent) => [agent.id, agent])); + const iterable = [...uniqueAgents.values()]; + return iterable; + }, [workspace.latest_build.resources]); + + const agentLogQueries = useQueries({ + queries: allUniqueAgents.map((agent) => ({ + ...agentLogs(workspace.id, agent.id), + enabled: open, + })), + }); + + // Note: trying to memoize this via useMemo got really clunky. Removing all + // memoization for now, but if we get to a point where performance matters, + // we should make it so that this state doesn't even begin to mount until the + // user decides to open the Logs dropdown + const allFiles: readonly DownloadableFile[] = (() => { + const files = allUniqueAgents.map((a, i) => { const name = `${a.name}-logs.txt`; - const logs = agentLogResults[i].data; - const txt = logs?.map((l) => l.output).join("\n"); + const txt = agentLogQueries[i]?.data?.map((l) => l.output).join("\n"); + let blob: Blob | undefined; if (txt) { blob = new Blob([txt], { type: "text/plain" }); } - files.push({ name, blob }); + + return { name, blob }; }); + const buildLogsFile = { + name: `${workspace.name}-build-logs.txt`, + blob: buildLogsQuery.data + ? new Blob([buildLogsQuery.data.map((l) => l.output).join("\n")], { + type: "text/plain", + }) + : undefined, + }; + + files.unshift(buildLogsFile); return files; - }, [agentLogResults, agents, buildLogsQuery.data, workspace.name]); - const isLoadingFiles = downloadableFiles.some((f) => f.blob === undefined); + })(); + const [isDownloading, setIsDownloading] = useState(false); + const isWorkspaceHealthy = workspace.health.healthy; + const isLoadingFiles = allFiles.some((f) => f.blob === undefined); + + const downloadTimeoutIdRef = useRef(undefined); + useEffect(() => { + const clearTimeoutOnUnmount = () => { + window.clearTimeout(downloadTimeoutIdRef.current); + }; + + return clearTimeoutOnUnmount; + }, []); return ( { + setIsDownloading(true); + const zip = new JSZip(); + allFiles.forEach((f) => { + if (f.blob) { + zip.file(f.name, f.blob); + } + }); + try { - setIsDownloading(true); - const zip = new JSZip(); - downloadableFiles.forEach((f) => { - if (f.blob) { - zip.file(f.name, f.blob); - } - }); const content = await zip.generateAsync({ type: "blob" }); download(content, `${workspace.name}-logs.zip`); - dialogProps.onClose(); - setTimeout(() => { + onClose(); + + downloadTimeoutIdRef.current = window.setTimeout(() => { setIsDownloading(false); }, theme.transitions.duration.leavingScreen); } catch (error) { @@ -106,18 +144,21 @@ export const DownloadLogsDialog: FC = ({ Downloading logs will create a zip file containing all logs from all jobs in this workspace. This may take a while.

+ + {!isWorkspaceHealthy && isLoadingFiles && ( + + Your workspace is unhealthy. Some logs may be unavailable for + download. + + )} +
    - {downloadableFiles.map((f) => ( -
  • - {f.name} - - {f.blob ? ( - humanBlobSize(f.blob.size) - ) : ( - - )} - -
    • + {allFiles.map((f) => ( + ))}
@@ -126,20 +167,98 @@ export const DownloadLogsDialog: FC = ({ ); }; +type DownloadingItemProps = Readonly<{ + // A value of undefined indicates that the component will wait forever + giveUpTimeMs?: number; + file: DownloadableFile; +}>; + +const DownloadingItem: FC = ({ file, giveUpTimeMs }) => { + const theme = useTheme(); + const [isWaiting, setIsWaiting] = useState(true); + + useEffect(() => { + if (giveUpTimeMs === undefined || file.blob !== undefined) { + setIsWaiting(true); + return; + } + + const timeoutId = window.setTimeout( + () => setIsWaiting(false), + giveUpTimeMs, + ); + + return () => window.clearTimeout(timeoutId); + }, [giveUpTimeMs, file]); + + const { baseName, fileExtension } = extractFileNameInfo(file.name); + + return ( +
  • + + {baseName} + .{fileExtension} + + + + {file.blob ? ( + humanBlobSize(file.blob.size) + ) : isWaiting ? ( + + ) : ( +

    Not available

    + )} +     +
    • + ); +}; + function humanBlobSize(size: number) { - const units = ["B", "KB", "MB", "GB", "TB"]; + const BLOB_SIZE_UNITS = ["B", "KB", "MB", "GB", "TB"] as const; let i = 0; - while (size > 1024 && i < units.length) { + while (size > 1024 && i < BLOB_SIZE_UNITS.length) { size /= 1024; i++; } - return `${size.toFixed(2)} ${units[i]}`; + + // The condition for the while loop above means that over time, we could break + // out of the loop because we accidentally shot past the array bounds and i + // is at index (BLOB_SIZE_UNITS.length). Adding a lot of redundant checks to + // make sure we always have a usable unit + const finalUnit = BLOB_SIZE_UNITS[i] ?? BLOB_SIZE_UNITS.at(-1) ?? "TB"; + return `${size.toFixed(2)} ${finalUnit}`; } -function selectAgents(workspace: Workspace): WorkspaceAgent[] { - return workspace.latest_build.resources - .flatMap((r) => r.agents) - .filter((a) => a !== undefined) as WorkspaceAgent[]; +type FileNameInfo = Readonly<{ + baseName: string; + fileExtension: string | undefined; +}>; + +function extractFileNameInfo(filename: string): FileNameInfo { + if (filename.length === 0) { + return { + baseName: "", + fileExtension: undefined, + }; + } + + const periodIndex = filename.lastIndexOf("."); + if (periodIndex === -1) { + return { + baseName: filename, + fileExtension: undefined, + }; + } + + return { + baseName: filename.slice(0, periodIndex), + fileExtension: filename.slice(periodIndex + 1), + }; } const styles = { @@ -151,16 +270,46 @@ const styles = { flexDirection: "column", gap: 8, }, + listItem: { + width: "100%", display: "flex", justifyContent: "space-between", alignItems: "center", + columnGap: "32px", }, + listItemPrimary: (theme) => ({ fontWeight: 500, color: theme.palette.text.primary, + display: "flex", + flexFlow: "row nowrap", + columnGap: 0, + overflow: "hidden", }), + + listItemPrimaryBaseName: { + minWidth: 0, + flexShrink: 1, + overflow: "hidden", + textOverflow: "ellipsis", + }, + + listItemPrimaryFileExtension: { + flexShrink: 0, + }, + listItemSecondary: { + flexShrink: 0, fontSize: 14, + whiteSpace: "nowrap", }, + + notAvailableText: (theme) => ({ + display: "flex", + flexFlow: "row nowrap", + alignItems: "center", + columnGap: "4px", + color: theme.palette.text.disabled, + }), } satisfies Record>; From 9757166c5d41cac1fb08839c0656dea8e75c8d10 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 9 Jul 2024 14:55:46 -0300 Subject: [PATCH 2/3] fix(site): enable dormant workspace to be deleted (#13850) (cherry picked from commit 01b30eaa324df69b82ad4c6570dc721f10751de6) --- .../WorkspaceActions/WorkspaceActions.stories.tsx | 12 ++++++++++++ .../WorkspacePage/WorkspaceActions/constants.ts | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx index b42dbd418a04f..c50f1ac8dfffe 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx @@ -206,6 +206,18 @@ export const OpenDownloadLogs: Story = { }, }; +export const CanDeleteDormantWorkspace: Story = { + args: { + workspace: Mocks.MockDormantWorkspace, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "More options" })); + const deleteButton = canvas.getByText("Delete…"); + await expect(deleteButton).toBeEnabled(); + }, +}; + function generateLogs(count: number) { return Array.from({ length: count }, (_, i) => ({ output: `log ${i + 1}`, diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts index f6d9f8f1cfa20..329a958ee12a8 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts +++ b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts @@ -48,7 +48,7 @@ export const abilitiesByWorkspaceStatus = ( return { actions: ["activate"], canCancel: false, - canAcceptJobs: false, + canAcceptJobs: true, }; } From 66006f2a3e19020c03d6451d1b3555f25bb2c513 Mon Sep 17 00:00:00 2001 From: Kayla Washburn-Love Date: Wed, 17 Jul 2024 09:53:40 -0600 Subject: [PATCH 3/3] chore: remove `organizationIds` from `AuthProvider` (#13917) (cherry picked from commit 80cbffe8435d56ceef8d8d75bf3df2f62f8cef91) --- site/src/contexts/auth/AuthProvider.tsx | 2 -- site/src/contexts/auth/RequireAuth.test.tsx | 1 - site/src/contexts/auth/RequireAuth.tsx | 6 +--- .../modules/dashboard/DashboardProvider.tsx | 35 ++++--------------- .../OrganizationSettingsLayout.tsx | 22 ++++++++---- .../OrganizationSettingsPage.tsx | 2 +- site/src/testHelpers/storybook.tsx | 1 - 7 files changed, 24 insertions(+), 45 deletions(-) diff --git a/site/src/contexts/auth/AuthProvider.tsx b/site/src/contexts/auth/AuthProvider.tsx index 2925ac095aadd..d71ece0ae4bc7 100644 --- a/site/src/contexts/auth/AuthProvider.tsx +++ b/site/src/contexts/auth/AuthProvider.tsx @@ -30,7 +30,6 @@ export type AuthContextValue = { isUpdatingProfile: boolean; user: User | undefined; permissions: Permissions | undefined; - organizationIds: readonly string[] | undefined; signInError: unknown; updateProfileError: unknown; signOut: () => void; @@ -119,7 +118,6 @@ export const AuthProvider: FC = ({ children }) => { permissions: permissionsQuery.data as Permissions | undefined, signInError: loginMutation.error, updateProfileError: updateProfileMutation.error, - organizationIds: userQuery.data?.organization_ids, }} > {children} diff --git a/site/src/contexts/auth/RequireAuth.test.tsx b/site/src/contexts/auth/RequireAuth.test.tsx index e1194cb601cbc..29cd9b6c54f96 100644 --- a/site/src/contexts/auth/RequireAuth.test.tsx +++ b/site/src/contexts/auth/RequireAuth.test.tsx @@ -95,7 +95,6 @@ describe("useAuthenticated", () => { wrapper: createAuthWrapper({ user: MockUser, permissions: MockPermissions, - organizationIds: [], }), }); }).not.toThrow(); diff --git a/site/src/contexts/auth/RequireAuth.tsx b/site/src/contexts/auth/RequireAuth.tsx index 6172ba8212ac5..d00b52dcd05d1 100644 --- a/site/src/contexts/auth/RequireAuth.tsx +++ b/site/src/contexts/auth/RequireAuth.tsx @@ -74,7 +74,7 @@ type RequireKeys = Omit & { // values are not undefined when authenticated type AuthenticatedAuthContextValue = RequireKeys< AuthContextValue, - "user" | "permissions" | "organizationIds" + "user" | "permissions" >; export const useAuthenticated = (): AuthenticatedAuthContextValue => { @@ -88,9 +88,5 @@ export const useAuthenticated = (): AuthenticatedAuthContextValue => { throw new Error("Permissions are not available."); } - if (!auth.organizationIds) { - throw new Error("Organization ID is not available."); - } - return auth as AuthenticatedAuthContextValue; }; diff --git a/site/src/modules/dashboard/DashboardProvider.tsx b/site/src/modules/dashboard/DashboardProvider.tsx index a44a162b994dd..2f3f16252887d 100644 --- a/site/src/modules/dashboard/DashboardProvider.tsx +++ b/site/src/modules/dashboard/DashboardProvider.tsx @@ -1,9 +1,4 @@ -import { - createContext, - type FC, - type PropsWithChildren, - useState, -} from "react"; +import { createContext, type FC, type PropsWithChildren } from "react"; import { useQuery } from "react-query"; import { appearance } from "api/queries/appearance"; import { entitlements } from "api/queries/entitlements"; @@ -15,12 +10,14 @@ import type { } from "api/typesGenerated"; import { Loader } from "components/Loader/Loader"; import { useAuthenticated } from "contexts/auth/RequireAuth"; -import { useEffectEvent } from "hooks/hookPolyfills"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; export interface DashboardValue { + /** + * @deprecated Do not add new usage of this value. It is being removed as part + * of the multi-org work. + */ organizationId: string; - setOrganizationId: (id: string) => void; entitlements: Entitlements; experiments: Experiments; appearance: AppearanceConfig; @@ -32,7 +29,7 @@ export const DashboardContext = createContext( export const DashboardProvider: FC = ({ children }) => { const { metadata } = useEmbeddedMetadata(); - const { user, organizationIds } = useAuthenticated(); + const { user } = useAuthenticated(); const entitlementsQuery = useQuery(entitlements(metadata.entitlements)); const experimentsQuery = useQuery(experiments(metadata.experiments)); const appearanceQuery = useQuery(appearance(metadata.appearance)); @@ -40,23 +37,6 @@ export const DashboardProvider: FC = ({ children }) => { const isLoading = !entitlementsQuery.data || !appearanceQuery.data || !experimentsQuery.data; - const lastUsedOrganizationId = localStorage.getItem( - `user:${user.id}.lastUsedOrganizationId`, - ); - const [activeOrganizationId, setActiveOrganizationId] = useState(() => - lastUsedOrganizationId && organizationIds.includes(lastUsedOrganizationId) - ? lastUsedOrganizationId - : organizationIds[0], - ); - - const setOrganizationId = useEffectEvent((id: string) => { - if (!organizationIds.includes(id)) { - throw new ReferenceError("Invalid organization ID"); - } - localStorage.setItem(`user:${user.id}.lastUsedOrganizationId`, id); - setActiveOrganizationId(id); - }); - if (isLoading) { return ; } @@ -64,8 +44,7 @@ export const DashboardProvider: FC = ({ children }) => { return ( { }; export const OrganizationSettingsLayout: FC = () => { - const { permissions, organizationIds } = useAuthenticated(); + const location = useLocation(); + const { permissions } = useAuthenticated(); const { experiments } = useDashboard(); const { organization } = useParams() as { organization: string }; const organizationsQuery = useQuery(myOrganizations()); const multiOrgExperimentEnabled = experiments.includes("multi-organization"); + const inOrganizationSettings = + location.pathname.startsWith("/organizations") && + location.pathname !== "/organizations/new"; + if (!multiOrgExperimentEnabled) { return ; } @@ -50,10 +55,13 @@ export const OrganizationSettingsLayout: FC = () => { {organizationsQuery.data ? ( org.name === organization, - )?.id ?? organizationIds[0], + currentOrganizationId: !inOrganizationSettings + ? undefined + : !organization + ? organizationsQuery.data[0]?.id + : organizationsQuery.data.find( + (org) => org.name === organization, + )?.id, organizations: organizationsQuery.data, }} > diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx index bc278b79c7e42..02ed8d1de9767 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationSettingsPage.tsx @@ -140,7 +140,7 @@ const OrganizationSettingsPage: FC = () => { css={styles.dangerButton} variant="contained" onClick={() => - deleteOrganizationMutation.mutate(currentOrganizationId) + deleteOrganizationMutation.mutate(currentOrganizationId!) } > Delete this organization diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index af1ba691bf826..d795af5f1818d 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -27,7 +27,6 @@ export const withDashboardProvider = ( {}, entitlements, experiments, appearance: MockAppearanceConfig,