- Notifications
You must be signed in to change notification settings - Fork1k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
agent/agentcontainers/api.go Outdated
| logger.Error(ctx,"inject subagent into container failed",slog.Error(err)) | ||
| } | ||
| dc.Error=err.Error() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 callsapi.RefreshContainers, which should then trigger this (unless I'm mistaken and misreading).
There was a problem hiding this comment.
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 fromdevcontainer up rather than injection.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this 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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this 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).
Uh oh!
There was an error while loading.Please reload this page.
4756080 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Fixescoder/internal#705
Surface errors on the UI when a devcontainer agent is unable to be injected.
https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=68627f4cdb34c1d90063b7a8
https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=68627f4cdb34c1d90063b7a9