Repository navigation
Description
workshop refresh doubles the cannot refresh "<NAME>" prefix on every pre-change failure
Summary
When workshop refresh fails before any change is created, the CLI prints its
error prefix twice:
error: cannot refresh "dev": cannot refresh "dev": workshop not launched
The same underlying failure reported through workshop launch prints the prefix
once. The duplication affects every pre-change refresh failure, not one specific
error, so it is a formatting defect in the refresh command rather than a problem
with any individual message.
Steps to reproduce
In any project directory with a workshop definition that has not been launched:
$ workshop refresh <NAME>
error: cannot refresh "<NAME>": cannot refresh "<NAME>": workshop not launchedAny pre-change failure reproduces it. Three observed on Workshop 0.9.7:
$ workshop refresh nosuchws
error: cannot refresh "nosuchws": cannot refresh "nosuchws": workshop definition "/path/.workshop/nosuchws.yaml" not found
$ workshop refresh <NAME> # definition edited to change confinement
error: cannot refresh "<NAME>": cannot refresh "<NAME>": confinement changed from "virtual-machine" to "container"
$ workshop refresh <NAME> # SDK added to a virtual-machine workshop
error: cannot refresh "<NAME>": cannot refresh "<NAME>": SDKs are currently unavailable for virtual machinesExpected vs actual
Expected, matching what workshop launch already does for the same failures:
error: cannot refresh "nosuchws": workshop definition "/path/.workshop/nosuchws.yaml" not found
Actual:
error: cannot refresh "nosuchws": cannot refresh "nosuchws": workshop definition "/path/.workshop/nosuchws.yaml" not found
For contrast, the identical daemon-side error surfaced through launch:
$ workshop launch nosuchws
error: cannot launch "nosuchws": workshop definition "/path/.workshop/nosuchws.yaml" not foundCause
The prefix is applied on both sides of the client/daemon boundary.
Daemon side, internal/overlord/workshopstate/manifest.go:
launchOrRefreshManifests already prefixes every error it returns with
cannot <action> %q:, where action is refresh on the refresh path. That
happens at each of its return sites, for example lines 255, 270, 275, 281, 302,
311, 316, 321 and 328.
Client side, cmd/workshop/refresh.go:233:
default:
// The request failed before any change ran; surface the cause.
return fmt.Errorf("cannot refresh %s: %w", strutil.Quoted(av), err)This wraps the already-prefixed daemon error a second time.
cmd/workshop/launch.go does not have an equivalent wrapper on its
pre-change error path; it returns the daemon error unchanged, which is why
launch prints a single prefix. The two commands disagree about who owns the
prefix, and refresh ends up applying it twice.
Note that the daemon error carries its own workshop name, so the two prefixes
are not merely redundant formatting: when refreshing several workshops at once,
the outer prefix is strutil.Quoted(av) over the whole argument vector while the
inner one names the single workshop that actually failed, and the two can differ.
Suggested fix
Make refresh match launch: drop the outer wrap at cmd/workshop/refresh.go:233
and return the daemon error as-is, so the prefix is owned by whichever side
produces the message.
If the outer prefix is instead the one worth keeping (it is the only one that
reports the full argument vector), the daemon-side prefix in
launchOrRefreshManifests should be removed instead, but that changes the
launch output too and needs the CLI to gain the prefix that launch currently
does not add.
Either way the fix is a single choice about ownership, applied consistently
across both commands.
Impact
Cosmetic, but it reaches users on every failed refresh, including the two
refusals that a reader of the virtual machine documentation is most likely to
hit deliberately (changing confinement, and adding an SDK to a virtual machine
workshop). Documentation that reproduces real CLI output has to reproduce the
duplication.
Environment
- Workshop: 0.9.7 (snap
workshop, rev 643,latest/edge, build3db9848d) - LXD: 6.9 (snap
lxd, rev 40424,6/stable) - Ubuntu host, x86_64
Activity