Skip to content

feat(site): display devcontainer start error #18637

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.

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 11 commits into from
Jun 30, 2025

Conversation

DanielleMaywood
Copy link
Contributor

@DanielleMaywood DanielleMaywood commented Jun 27, 2025

@@ -685,6 +685,8 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code
if err != nil {
logger.Error(ctx, "inject subagent into container failed", slog.Error(err))
}

dc.Error = err.Error()
Copy link
Member

Choose a reason for hiding this comment

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

We might want to reset this in some other scenarios as well, like recreate. And assign on recreate error. There’s also the devcontainer status but I don’t think we need to touch it necessarily in this case since it’s an injection error but the devcontainer itself is probably fine.

Copy link
Member

Choose a reason for hiding this comment

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

@DanielleMaywood did you look into adding this? At present I don't think we surface the error from recreate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current implementation of recreate calls api.RefreshContainers, which should then trigger this (unless I'm mistaken and misreading).

Copy link
Member

Choose a reason for hiding this comment

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

Not exactly. I’m referring to the error from devcontainer up rather than injection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see now, I'll take a look at fixing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mafredri I've made that change 👍 Was nice and easy

@DanielleMaywood DanielleMaywood changed the title feat(site): surface devcontainer start error feat(site): display devcontainer start error Jun 30, 2025
@DanielleMaywood DanielleMaywood marked this pull request as ready for review June 30, 2025 13:19
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Some suggestions below, but I don't need to review again.

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

A small FE suggestion but otherwise nothing blocking.

I think the recreate error is potentially worth considering as well (see where we assign devcontainer status to error).

@DanielleMaywood DanielleMaywood merged commit 4756080 into main Jun 30, 2025
39 checks passed
@DanielleMaywood DanielleMaywood deleted the danielle/surface-devcontainer-errors branch June 30, 2025 20:34
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Devcontainers: Surface errors from sub agent injection and dev container recreation visibly in the UI
3 participants