Skip to content

fix: display explicit 'retry' button(s) when a workspace fails #10720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Nov 22, 2023
Merged

fix: display explicit 'retry' button(s) when a workspace fails #10720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c28f1d8
refactor: remove workspace error enums
Parkreiner Nov 15, 2023
c249a40
refactor: improve co-location for serverVersion
Parkreiner Nov 15, 2023
718972f
fix: add in retry button for failed workspaces
Parkreiner Nov 15, 2023
6005cec
fix: make handleBuildRetry auto-detect debug permissions
Parkreiner Nov 15, 2023
0f91d44
chore: consolidate retry messaging
Parkreiner Nov 15, 2023
d635ecd
docs: clean up formatting for WorkspacePage.test comments
Parkreiner Nov 15, 2023
404e4c7
chore: update renderWorkspacePage to accept parameters
Parkreiner Nov 15, 2023
edecd14
fix: restore granularity to retry operations (debug vs normal)
Parkreiner Nov 16, 2023
b1f6660
chore: make workspace helpers take explicit workspace parameter
Parkreiner Nov 16, 2023
c3edba7
refactor: update how parameters for tests are defined
Parkreiner Nov 16, 2023
2c55fd8
wip: add tests for workspace retries (breaks 2 previous tests
Parkreiner Nov 16, 2023
8d7fe73
fix: update old tests to be correctly parameterized
Parkreiner Nov 17, 2023
91ce668
fix: make sure file formatting goes through
Parkreiner Nov 17, 2023
44f86d5
merge main
Parkreiner Nov 22, 2023
abc3ae6
fix: update variable name for clarity
Parkreiner Nov 22, 2023
fa64932
fix: add export for runtime buttonTypes array
Parkreiner Nov 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions site/src/pages/WorkspacePage/Workspace.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Meta, StoryObj } from "@storybook/react";
import { WatchAgentMetadataContext } from "components/Resources/AgentMetadata";
import { ProvisionerJobLog } from "api/typesGenerated";
import * as Mocks from "testHelpers/entities";
import { Workspace, WorkspaceErrors } from "./Workspace";
import { Workspace } from "./Workspace";
import { withReactContext } from "storybook-react-context";
import EventSource from "eventsourcemock";
import { ProxyContext, getPreferredProxy } from "contexts/ProxyContext";
Expand Down Expand Up @@ -133,7 +133,7 @@ export const Failed: Story = {
...Running.args,
workspace: Mocks.MockFailedWorkspace,
workspaceErrors: {
[WorkspaceErrors.BUILD_ERROR]: Mocks.mockApiError({
buildError: Mocks.mockApiError({
message: "A workspace build is already active.",
}),
},
Expand Down Expand Up @@ -172,7 +172,6 @@ export const FailedWithRetry: Story = {
},
},
},
canRetryDebugMode: true,
buildLogs: <WorkspaceBuildLogsSection logs={makeFailedBuildLogs()} />,
},
};
Expand Down Expand Up @@ -224,7 +223,7 @@ export const GetBuildsError: Story = {
args: {
...Running.args,
workspaceErrors: {
[WorkspaceErrors.GET_BUILDS_ERROR]: Mocks.mockApiError({
getBuildsError: Mocks.mockApiError({
message: "There is a problem fetching builds.",
}),
},
Expand All @@ -235,7 +234,7 @@ export const CancellationError: Story = {
args: {
...Failed.args,
workspaceErrors: {
[WorkspaceErrors.CANCELLATION_ERROR]: Mocks.mockApiError({
cancellationError: Mocks.mockApiError({
message: "Job could not be canceled.",
}),
},
Expand Down
93 changes: 45 additions & 48 deletions site/src/pages/WorkspacePage/Workspace.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,13 @@ import { BuildsTable } from "./BuildsTable";
import { WorkspaceDeletedBanner } from "./WorkspaceDeletedBanner";
import { WorkspaceStats } from "./WorkspaceStats";

export enum WorkspaceErrors {
GET_BUILDS_ERROR = "getBuildsError",
BUILD_ERROR = "buildError",
CANCELLATION_ERROR = "cancellationError",
}
export type WorkspaceError =
| "getBuildsError"
| "buildError"
| "cancellationError";

export type WorkspaceErrors = Partial<Record<WorkspaceError, unknown>>;

export interface WorkspaceProps {
scheduleProps: {
onDeadlinePlus: (hours: number) => void;
Expand All @@ -56,16 +58,17 @@ export interface WorkspaceProps {
resources?: TypesGen.WorkspaceResource[];
canUpdateWorkspace: boolean;
updateMessage?: string;
canRetryDebugMode: boolean;
canChangeVersions: boolean;
hideSSHButton?: boolean;
hideVSCodeDesktopButton?: boolean;
workspaceErrors: Partial<Record<WorkspaceErrors, unknown>>;
workspaceErrors: WorkspaceErrors;
buildInfo?: TypesGen.BuildInfoResponse;
sshPrefix?: string;
template?: TypesGen.Template;
quotaBudget?: number;
canRetryDebugMode: boolean;
handleBuildRetry: () => void;
handleBuildRetryDebug: () => void;
buildLogs?: React.ReactNode;
builds: TypesGen.WorkspaceBuild[] | undefined;
onLoadMoreBuilds: () => void;
Expand All @@ -87,55 +90,39 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
handleCancel,
handleSettings,
handleChangeVersion,
handleDormantActivate: handleDormantActivate,
handleDormantActivate,
workspace,
isUpdating,
isRestarting,
resources,
builds,
canUpdateWorkspace,
updateMessage,
canRetryDebugMode,
canChangeVersions,
workspaceErrors,
hideSSHButton,
hideVSCodeDesktopButton,
buildInfo,
sshPrefix,
template,
canRetryDebugMode,
handleBuildRetry,
handleBuildRetryDebug,
buildLogs,
onLoadMoreBuilds,
isLoadingMoreBuilds,
hasMoreBuilds,
canAutostart,
}) => {
const navigate = useNavigate();
const serverVersion = buildInfo?.version || "";
const { saveLocal, getLocal } = useLocalStorage();

const buildError = Boolean(workspaceErrors[WorkspaceErrors.BUILD_ERROR]) && (
Copy link
Member Author

@Parkreiner Parkreiner Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two error JSX elements were moved into the return value, to bring them closer to where they're actually used

<ErrorAlert
error={workspaceErrors[WorkspaceErrors.BUILD_ERROR]}
dismissible
/>
);

const cancellationError = Boolean(
workspaceErrors[WorkspaceErrors.CANCELLATION_ERROR],
) && (
<ErrorAlert
error={workspaceErrors[WorkspaceErrors.CANCELLATION_ERROR]}
dismissible
/>
);

let transitionStats: TypesGen.TransitionStats | undefined = undefined;
if (template !== undefined) {
transitionStats = ActiveTransition(template, workspace);
}

const [showAlertPendingInQueue, setShowAlertPendingInQueue] = useState(false);

// 2023-11-15 - MES - This effect will be called every single render because
// "now" will always change and invalidate the dependency array. Need to
// figure out if this effect really should run every render (possibly meaning
// no dependency array at all), or how to get the array stabilized (ideal)
Copy link
Member Author

@Parkreiner Parkreiner Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the effect re-run every single render isn't ideal, but I don't think it's breaking anything

During the sprint right before Christmas, I'm planning to do an audit for every single useEffect/useLayoutEffect call we have. I know we have a couple of other wonky calls, too. Just figured it'd be good to flag this for now, if someone has the bandwidth to fix it early while working on another issue

const now = dayjs();
useEffect(() => {
if (
Expand Down Expand Up @@ -174,6 +161,9 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
const autoStartFailing = workspace.autostart_schedule && !canAutostart;
const requiresManualUpdate = updateRequired && autoStartFailing;

const transitionStats =
template !== undefined ? ActiveTransition(template, workspace) : undefined;

return (
<>
<FullWidthPageHeader>
Expand Down Expand Up @@ -213,8 +203,11 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
handleUpdate={handleUpdate}
handleCancel={handleCancel}
handleSettings={handleSettings}
handleRetry={handleBuildRetry}
handleRetryDebug={handleBuildRetryDebug}
handleChangeVersion={handleChangeVersion}
handleDormantActivate={handleDormantActivate}
canRetryDebug={canRetryDebugMode}
canChangeVersions={canChangeVersions}
isUpdating={isUpdating}
isRestarting={isRestarting}
Expand Down Expand Up @@ -244,8 +237,15 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
{updateMessage && <AlertDetail>{updateMessage}</AlertDetail>}
</Alert>
))}
{buildError}
{cancellationError}

{Boolean(workspaceErrors.buildError) && (
<ErrorAlert error={workspaceErrors.buildError} dismissible />
)}

{Boolean(workspaceErrors.cancellationError) && (
<ErrorAlert error={workspaceErrors.cancellationError} dismissible />
)}

{workspace.latest_build.status === "running" &&
!workspace.health.healthy && (
<Alert
Expand Down Expand Up @@ -311,16 +311,15 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
<Alert
severity="error"
actions={
canRetryDebugMode && (
<Button
key={0}
onClick={handleBuildRetry}
variant="text"
size="small"
>
Try in debug mode
</Button>
)
<Button
onClick={
canRetryDebugMode ? handleBuildRetryDebug : handleBuildRetry
}
variant="text"
size="small"
>
Retry{canRetryDebugMode && " in debug mode"}
</Button>
}
>
<AlertTitle>Workspace build failed</AlertTitle>
Expand Down Expand Up @@ -357,17 +356,15 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
showBuiltinApps={canUpdateWorkspace}
hideSSHButton={hideSSHButton}
hideVSCodeDesktopButton={hideVSCodeDesktopButton}
serverVersion={serverVersion}
serverVersion={buildInfo?.version || ""}
onUpdateAgent={handleUpdate} // On updating the workspace the agent version is also updated
/>
)}
/>
)}

{workspaceErrors[WorkspaceErrors.GET_BUILDS_ERROR] ? (
<ErrorAlert
error={workspaceErrors[WorkspaceErrors.GET_BUILDS_ERROR]}
/>
{workspaceErrors.getBuildsError ? (
<ErrorAlert error={workspaceErrors.getBuildsError} />
) : (
<BuildsTable
builds={builds}
Expand Down
60 changes: 42 additions & 18 deletions site/src/pages/WorkspacePage/WorkspaceActions/Buttons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
import { type FC } from "react";
import type { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
import { BuildParametersPopover } from "./BuildParametersPopover";

import Tooltip from "@mui/material/Tooltip";
import Button from "@mui/material/Button";
import BlockIcon from "@mui/icons-material/Block";
import LoadingButton from "@mui/lab/LoadingButton";
import ButtonGroup from "@mui/material/ButtonGroup";
import CloudQueueIcon from "@mui/icons-material/CloudQueue";
import CropSquareIcon from "@mui/icons-material/CropSquare";
import PlayCircleOutlineIcon from "@mui/icons-material/PlayCircleOutline";
import ReplayIcon from "@mui/icons-material/Replay";
import { FC } from "react";
import BlockOutlined from "@mui/icons-material/BlockOutlined";
import ButtonGroup from "@mui/material/ButtonGroup";
import { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
import { BuildParametersPopover } from "./BuildParametersPopover";
import BlockIcon from "@mui/icons-material/Block";
import OutlinedBlockIcon from "@mui/icons-material/BlockOutlined";
import PowerSettingsNewIcon from "@mui/icons-material/PowerSettingsNew";
import LoadingButton from "@mui/lab/LoadingButton";
import Tooltip from "@mui/material/Tooltip";
import RetryIcon from "@mui/icons-material/BuildOutlined";
import RetryDebugIcon from "@mui/icons-material/BugReportOutlined";

interface WorkspaceAction {
interface WorkspaceActionProps {
loading?: boolean;
handleAction: () => void;
disabled?: boolean;
tooltipText?: string;
}

export const UpdateButton: FC<WorkspaceAction> = ({
export const UpdateButton: FC<WorkspaceActionProps> = ({
handleAction,
loading,
}) => {
Expand All @@ -37,7 +40,7 @@ export const UpdateButton: FC<WorkspaceAction> = ({
);
};

export const ActivateButton: FC<WorkspaceAction> = ({
export const ActivateButton: FC<WorkspaceActionProps> = ({
handleAction,
loading,
}) => {
Expand All @@ -54,7 +57,7 @@ export const ActivateButton: FC<WorkspaceAction> = ({
};

export const StartButton: FC<
Omit<WorkspaceAction, "handleAction"> & {
Omit<WorkspaceActionProps, "handleAction"> & {
workspace: Workspace;
handleAction: (buildParameters?: WorkspaceBuildParameter[]) => void;
}
Expand All @@ -63,7 +66,7 @@ export const StartButton: FC<
<ButtonGroup
variant="outlined"
sx={{
// Workaround to make the border transitions smmothly on button groups
// Workaround to make the border transitions smoothly on button groups
"& > button:hover + button": {
borderLeft: "1px solid #FFF",
},
Expand Down Expand Up @@ -94,7 +97,10 @@ export const StartButton: FC<
);
};

export const StopButton: FC<WorkspaceAction> = ({ handleAction, loading }) => {
export const StopButton: FC<WorkspaceActionProps> = ({
handleAction,
loading,
}) => {
return (
<LoadingButton
loading={loading}
Expand All @@ -109,7 +115,7 @@ export const StopButton: FC<WorkspaceAction> = ({ handleAction, loading }) => {
};

export const RestartButton: FC<
Omit<WorkspaceAction, "handleAction"> & {
Omit<WorkspaceActionProps, "handleAction"> & {
workspace: Workspace;
handleAction: (buildParameters?: WorkspaceBuildParameter[]) => void;
}
Expand All @@ -118,7 +124,7 @@ export const RestartButton: FC<
<ButtonGroup
variant="outlined"
sx={{
// Workaround to make the border transitions smmothly on button groups
// Workaround to make the border transitions smoothly on button groups
"& > button:hover + button": {
borderLeft: "1px solid #FFF",
},
Expand Down Expand Up @@ -150,7 +156,7 @@ export const RestartButton: FC<
);
};

export const CancelButton: FC<WorkspaceAction> = ({ handleAction }) => {
export const CancelButton: FC<WorkspaceActionProps> = ({ handleAction }) => {
return (
<Button startIcon={<BlockIcon />} onClick={handleAction}>
Cancel
Expand All @@ -164,7 +170,7 @@ interface DisabledProps {

export const DisabledButton: FC<DisabledProps> = ({ label }) => {
return (
<Button startIcon={<BlockOutlined />} disabled>
<Button startIcon={<OutlinedBlockIcon />} disabled>
{label}
</Button>
);
Expand All @@ -181,3 +187,21 @@ export const ActionLoadingButton: FC<LoadingProps> = ({ label }) => {
</LoadingButton>
);
};

type DebugButtonProps = Omit<WorkspaceActionProps, "loading"> & {
debug?: boolean;
};

export const RetryButton = ({
handleAction,
debug = false,
}: DebugButtonProps) => {
return (
<Button
startIcon={debug ? <RetryDebugIcon /> : <RetryIcon />}
onClick={handleAction}
>
Retry{debug && " (Debug)"}
</Button>
);
};
Loading