From 4756080eb2d6edb26b63f7a5e314c7e6df9f7bd3 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 30 Jun 2025 21:34:29 +0100 Subject: [PATCH] feat(site): display devcontainer start error (#18637) Fixes https://github.com/coder/internal/issues/705 Surface errors on the UI when a devcontainer agent is unable to be injected. --- agent/agentcontainers/api.go | 5 + agent/agentcontainers/api_test.go | 219 ++++++++++++++++++ coderd/apidoc/docs.go | 3 + coderd/apidoc/swagger.json | 3 + codersdk/workspaceagents.go | 2 + docs/reference/api/agents.md | 1 + docs/reference/api/schemas.md | 3 + site/src/api/typesGenerated.ts | 1 + .../AgentDevcontainerCard.stories.tsx | 10 + .../resources/AgentDevcontainerCard.tsx | 21 +- site/src/modules/resources/AgentRow.tsx | 8 + site/src/modules/resources/AgentStatus.tsx | 54 ++++- 12 files changed, 325 insertions(+), 5 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index ddf5ce438c..808c7345fc 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -717,6 +717,9 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code err := api.maybeInjectSubAgentIntoContainerLocked(ctx, dc) if err != nil { logger.Error(ctx, "inject subagent into container failed", slog.Error(err)) + dc.Error = err.Error() + } else { + dc.Error = "" } } @@ -1032,6 +1035,7 @@ func (api *API) CreateDevcontainer(workspaceFolder, configPath string, opts ...D api.mu.Lock() dc = api.knownDevcontainers[dc.WorkspaceFolder] dc.Status = codersdk.WorkspaceAgentDevcontainerStatusError + dc.Error = err.Error() api.knownDevcontainers[dc.WorkspaceFolder] = dc api.recreateErrorTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "errorTimes") api.mu.Unlock() @@ -1055,6 +1059,7 @@ func (api *API) CreateDevcontainer(workspaceFolder, configPath string, opts ...D } } dc.Dirty = false + dc.Error = "" api.recreateSuccessTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "successTimes") api.knownDevcontainers[dc.WorkspaceFolder] = dc api.mu.Unlock() diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 2a742e3be8..468e5ef938 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1649,6 +1649,225 @@ func TestAPI(t *testing.T) { assert.Empty(t, fakeSAC.agents) }) + t.Run("Error", func(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("Dev Container tests are not supported on Windows (this test uses mocks but fails due to Windows paths)") + } + + t.Run("DuringUp", func(t *testing.T) { + t.Parallel() + + var ( + ctx = testutil.Context(t, testutil.WaitMedium) + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + mClock = quartz.NewMock(t) + fCCLI = &fakeContainerCLI{arch: ""} + fDCCLI = &fakeDevcontainerCLI{ + upErrC: make(chan error, 1), + } + fSAC = &fakeSubAgentClient{ + logger: logger.Named("fakeSubAgentClient"), + } + + testDevcontainer = codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "test-devcontainer", + WorkspaceFolder: "/workspaces/project", + ConfigPath: "/workspaces/project/.devcontainer/devcontainer.json", + Status: codersdk.WorkspaceAgentDevcontainerStatusStopped, + } + ) + + mClock.Set(time.Now()).MustWait(ctx) + tickerTrap := mClock.Trap().TickerFunc("updaterLoop") + nowRecreateErrorTrap := mClock.Trap().Now("recreate", "errorTimes") + nowRecreateSuccessTrap := mClock.Trap().Now("recreate", "successTimes") + + api := agentcontainers.NewAPI(logger, + agentcontainers.WithClock(mClock), + agentcontainers.WithContainerCLI(fCCLI), + agentcontainers.WithDevcontainerCLI(fDCCLI), + agentcontainers.WithDevcontainers( + []codersdk.WorkspaceAgentDevcontainer{testDevcontainer}, + []codersdk.WorkspaceAgentScript{{ID: testDevcontainer.ID, LogSourceID: uuid.New()}}, + ), + agentcontainers.WithSubAgentClient(fSAC), + agentcontainers.WithSubAgentURL("test-subagent-url"), + agentcontainers.WithWatcher(watcher.NewNoop()), + ) + api.Start() + defer func() { + close(fDCCLI.upErrC) + api.Close() + }() + + r := chi.NewRouter() + r.Mount("/", api.Routes()) + + tickerTrap.MustWait(ctx).MustRelease(ctx) + tickerTrap.Close() + + // Given: We send a 'recreate' request. + req := httptest.NewRequest(http.MethodPost, "/devcontainers/"+testDevcontainer.ID.String()+"/recreate", nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusAccepted, rec.Code) + + // Given: We simulate an error running `devcontainer up` + simulatedError := xerrors.New("simulated error") + testutil.RequireSend(ctx, t, fDCCLI.upErrC, simulatedError) + + nowRecreateErrorTrap.MustWait(ctx).MustRelease(ctx) + nowRecreateErrorTrap.Close() + + req = httptest.NewRequest(http.MethodGet, "/", nil) + rec = httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var response codersdk.WorkspaceAgentListContainersResponse + err := json.NewDecoder(rec.Body).Decode(&response) + require.NoError(t, err) + + // Then: We expect that there will be an error associated with the devcontainer. + require.Len(t, response.Devcontainers, 1) + require.Equal(t, "simulated error", response.Devcontainers[0].Error) + + // Given: We send another 'recreate' request. + req = httptest.NewRequest(http.MethodPost, "/devcontainers/"+testDevcontainer.ID.String()+"/recreate", nil) + rec = httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusAccepted, rec.Code) + + // Given: We allow `devcontainer up` to succeed. + testutil.RequireSend(ctx, t, fDCCLI.upErrC, nil) + + nowRecreateSuccessTrap.MustWait(ctx).MustRelease(ctx) + nowRecreateSuccessTrap.Close() + + req = httptest.NewRequest(http.MethodGet, "/", nil) + rec = httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + response = codersdk.WorkspaceAgentListContainersResponse{} + err = json.NewDecoder(rec.Body).Decode(&response) + require.NoError(t, err) + + // Then: We expect that there will be no error associated with the devcontainer. + require.Len(t, response.Devcontainers, 1) + require.Equal(t, "", response.Devcontainers[0].Error) + }) + + t.Run("DuringInjection", func(t *testing.T) { + t.Parallel() + + var ( + ctx = testutil.Context(t, testutil.WaitMedium) + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + mClock = quartz.NewMock(t) + mCCLI = acmock.NewMockContainerCLI(gomock.NewController(t)) + fDCCLI = &fakeDevcontainerCLI{} + fSAC = &fakeSubAgentClient{ + logger: logger.Named("fakeSubAgentClient"), + createErrC: make(chan error, 1), + } + + containerCreatedAt = time.Now() + testContainer = codersdk.WorkspaceAgentContainer{ + ID: "test-container-id", + FriendlyName: "test-container", + Image: "test-image", + Running: true, + CreatedAt: containerCreatedAt, + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspaces", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/.devcontainer/devcontainer.json", + }, + } + ) + + coderBin, err := os.Executable() + require.NoError(t, err) + + // Mock the `List` function to always return the test container. + mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{testContainer}, + }, nil).AnyTimes() + + // We're going to force the container CLI to fail, which will allow us to test the + // error handling. + simulatedError := xerrors.New("simulated error") + mCCLI.EXPECT().DetectArchitecture(gomock.Any(), testContainer.ID).Return("", simulatedError).Times(1) + + mClock.Set(containerCreatedAt).MustWait(ctx) + tickerTrap := mClock.Trap().TickerFunc("updaterLoop") + + api := agentcontainers.NewAPI(logger, + agentcontainers.WithClock(mClock), + agentcontainers.WithContainerCLI(mCCLI), + agentcontainers.WithDevcontainerCLI(fDCCLI), + agentcontainers.WithSubAgentClient(fSAC), + agentcontainers.WithSubAgentURL("test-subagent-url"), + agentcontainers.WithWatcher(watcher.NewNoop()), + ) + api.Start() + defer func() { + close(fSAC.createErrC) + api.Close() + }() + + r := chi.NewRouter() + r.Mount("/", api.Routes()) + + // Given: We allow an attempt at creation to occur. + tickerTrap.MustWait(ctx).MustRelease(ctx) + tickerTrap.Close() + + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var response codersdk.WorkspaceAgentListContainersResponse + err = json.NewDecoder(rec.Body).Decode(&response) + require.NoError(t, err) + + // Then: We expect that there will be an error associated with the devcontainer. + require.Len(t, response.Devcontainers, 1) + require.Equal(t, "detect architecture: simulated error", response.Devcontainers[0].Error) + + gomock.InOrder( + mCCLI.EXPECT().DetectArchitecture(gomock.Any(), testContainer.ID).Return(runtime.GOARCH, nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil), + mCCLI.EXPECT().Copy(gomock.Any(), testContainer.ID, coderBin, "/.coder-agent/coder").Return(nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), testContainer.ID, "root", "/bin/sh", "-c", "chown $(id -u):$(id -g) /.coder-agent/coder").Return(nil, nil), + ) + + // Given: We allow creation to succeed. + testutil.RequireSend(ctx, t, fSAC.createErrC, nil) + + _, aw := mClock.AdvanceNext() + aw.MustWait(ctx) + + req = httptest.NewRequest(http.MethodGet, "/", nil) + rec = httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + response = codersdk.WorkspaceAgentListContainersResponse{} + err = json.NewDecoder(rec.Body).Decode(&response) + require.NoError(t, err) + + // Then: We expect that the error will be gone + require.Len(t, response.Devcontainers, 1) + require.Equal(t, "", response.Devcontainers[0].Error) + }) + }) + t.Run("Create", func(t *testing.T) { t.Parallel() diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 522ba671a9..745ea3e3ff 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -16997,6 +16997,9 @@ const docTemplate = `{ "dirty": { "type": "boolean" }, + "error": { + "type": "string" + }, "id": { "type": "string", "format": "uuid" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index abcae550a4..d782946aa4 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -15527,6 +15527,9 @@ "dirty": { "type": "boolean" }, + "error": { + "type": "string" + }, "id": { "type": "string", "format": "uuid" diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index 77cc0aff9a..2bfae8aac3 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -417,6 +417,8 @@ type WorkspaceAgentDevcontainer struct { Dirty bool `json:"dirty"` Container *WorkspaceAgentContainer `json:"container,omitempty"` Agent *WorkspaceAgentDevcontainerAgent `json:"agent,omitempty"` + + Error string `json:"error,omitempty"` } // WorkspaceAgentDevcontainerAgent represents the sub agent for a diff --git a/docs/reference/api/agents.md b/docs/reference/api/agents.md index c32e8202fa..cff5fef6f3 100644 --- a/docs/reference/api/agents.md +++ b/docs/reference/api/agents.md @@ -833,6 +833,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con } }, "dirty": true, + "error": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 79c6f817bc..389c7ec9c3 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -8554,6 +8554,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| } }, "dirty": true, + "error": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", @@ -8569,6 +8570,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `config_path` | string | false | | | | `container` | [codersdk.WorkspaceAgentContainer](#codersdkworkspaceagentcontainer) | false | | | | `dirty` | boolean | false | | | +| `error` | string | false | | | | `id` | string | false | | | | `name` | string | false | | | | `status` | [codersdk.WorkspaceAgentDevcontainerStatus](#codersdkworkspaceagentdevcontainerstatus) | false | | Additional runtime fields. | @@ -8710,6 +8712,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| } }, "dirty": true, + "error": "string", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 0e6a481406..6552c0f0d5 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3329,6 +3329,7 @@ export interface WorkspaceAgentDevcontainer { readonly dirty: boolean; readonly container?: WorkspaceAgentContainer; readonly agent?: WorkspaceAgentDevcontainerAgent; + readonly error?: string; } // From codersdk/workspaceagents.go diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index 1f798b7540..75c53d8b65 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -46,6 +46,16 @@ const meta: Meta = { export default meta; type Story = StoryObj; +export const HasError: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + error: "unable to inject devcontainer with agent", + agent: undefined, + }, + }, +}; + export const NoPorts: Story = {}; export const WithPorts: Story = { diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 47ef6388a3..77118ad26d 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -6,6 +6,7 @@ import type { WorkspaceAgentDevcontainer, WorkspaceAgentListContainersResponse, } from "api/typesGenerated"; + import { Button } from "components/Button/Button"; import { displayError } from "components/GlobalSnackbar/utils"; import { Spinner } from "components/Spinner/Spinner"; @@ -23,11 +24,12 @@ import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; import type { FC } from "react"; import { useEffect } from "react"; import { useMutation, useQueryClient } from "react-query"; +import { cn } from "utils/cn"; import { portForwardURL } from "utils/portForward"; import { AgentApps, organizeAgentApps } from "./AgentApps/AgentApps"; import { AgentButton } from "./AgentButton"; import { AgentLatency } from "./AgentLatency"; -import { SubAgentStatus } from "./AgentStatus"; +import { DevcontainerStatus } from "./AgentStatus"; import { PortForwardButton } from "./PortForwardButton"; import { AgentSSHButton } from "./SSHButton/SSHButton"; import { SubAgentOutdatedTooltip } from "./SubAgentOutdatedTooltip"; @@ -190,7 +192,10 @@ export const AgentDevcontainerCard: FC = ({ key={devcontainer.id} direction="column" spacing={0} - className="relative py-4 border border-dashed border-border rounded" + className={cn( + "relative py-4 border border-dashed border-border rounded", + devcontainer.error && "border-content-destructive border-solid", + )} >
- + + {devcontainer.error} +
+ )} + {(showSubAgentApps || showSubAgentAppsPlaceholders) && (
{subAgent && diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index 54ffe229b2..3d0888f787 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -156,6 +156,9 @@ export const AgentRow: FC = ({ shouldDisplayAppsSection = false; } + // Check if any devcontainers have errors to gray out agent border + const hasDevcontainerErrors = devcontainers?.some((dc) => dc.error); + return ( = ({ styles.agentRow, styles[`agentRow-${agent.status}`], styles[`agentRow-lifecycle-${agent.lifecycle_state}`], + hasDevcontainerErrors && styles.agentRowWithErrors, ]} >
@@ -537,4 +541,8 @@ const styles = { position: "relative", }, }), + + agentRowWithErrors: (theme) => ({ + borderColor: theme.palette.divider, + }), } satisfies Record>; diff --git a/site/src/modules/resources/AgentStatus.tsx b/site/src/modules/resources/AgentStatus.tsx index eeae1d8fcd..7eb165d19f 100644 --- a/site/src/modules/resources/AgentStatus.tsx +++ b/site/src/modules/resources/AgentStatus.tsx @@ -1,7 +1,10 @@ import type { Interpolation, Theme } from "@emotion/react"; import Link from "@mui/material/Link"; import Tooltip from "@mui/material/Tooltip"; -import type { WorkspaceAgent } from "api/typesGenerated"; +import type { + WorkspaceAgent, + WorkspaceAgentDevcontainer, +} from "api/typesGenerated"; import { ChooseOne, Cond } from "components/Conditionals/ChooseOne"; import { HelpTooltip, @@ -50,6 +53,12 @@ interface SubAgentStatusProps { agent?: WorkspaceAgent; } +interface DevcontainerStatusProps { + devcontainer: WorkspaceAgentDevcontainer; + parentAgent: WorkspaceAgent; + agent?: WorkspaceAgent; +} + const StartTimeoutLifecycle: FC = ({ agent }) => { return ( @@ -274,7 +283,7 @@ export const AgentStatus: FC = ({ agent }) => { ); }; -export const SubAgentStatus: FC = ({ agent }) => { +const SubAgentStatus: FC = ({ agent }) => { if (!agent) { return ; } @@ -296,6 +305,47 @@ export const SubAgentStatus: FC = ({ agent }) => { ); }; +const DevcontainerStartError: FC = ({ agent }) => { + return ( + + + + + + + Error starting the devcontainer agent + + + Something went wrong during the devcontainer agent startup.{" "} + + Troubleshoot + + . + + + + ); +}; + +export const DevcontainerStatus: FC = ({ + devcontainer, + parentAgent, + agent, +}) => { + if (devcontainer.error) { + // When a dev container has an 'error' associated with it, + // then we won't have an agent associated with it. This is + // why we use the parent agent instead of the sub agent. + return ; + } + + return ; +}; + const styles = { status: { width: 6,