Skip to content

Commit

Permalink
osbuild-worker: do not use error in clienterror.Error.Details
Browse files Browse the repository at this point in the history
This is an alternative/complementary fix for PR#4137. It is very
simple so should be uncontroverisal.

It fixes an issue that @schuellerf discovered, i.e. that when an error
interface is passed into clienterrors.Error.Details the details get
lost because the json.Marshaler will not know how to handler an
error interface.

To find the problematic uses of `error` a custom vet checker was
build in https://github.com/mvo5/osbuild-cvet. With that the
result is:
```
$ go run github.com/mvo5/osbuild-cvet@latest ./...
/home/mvogt/devel/osbuild/osbuild-composer/cmd/osbuild-worker/jobimpl-depsolve.go:93:26: do not pass 'error' to WorkerClientError() details, use error.Error() instead
/home/mvogt/devel/osbuild/osbuild-composer/cmd/osbuild-worker/jobimpl-osbuild.go:404:31: do not pass 'error' to WorkerClientError() details, use error.Error() instead
/home/mvogt/devel/osbuild/osbuild-composer/cmd/osbuild-worker/jobimpl-osbuild.go:519:31: do not pass 'error' to WorkerClientError() details, use error.Error() instead
/home/mvogt/devel/osbuild/osbuild-composer/cmd/osbuild-worker/jobimpl-osbuild.go:556:31: do not pass '[]error' to WorkerClientError() details, use []string instead
```
and once this commit is in no more errors.

Just like PR#4137 this is not perfect because it will not do a
recursive check for the passed argument.
  • Loading branch information
mvo5 committed Jun 6, 2024
1 parent 8ebefbd commit 61bf0c3
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/osbuild-worker/jobimpl-depsolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (impl *DepsolveJobImpl) Run(job worker.Job) error {
for _, baseurlstr := range repo.BaseURLs {
match, err := impl.RepositoryMTLSConfig.CompareBaseURL(baseurlstr)
if err != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidRepositoryURL, "Repository URL is malformed", err)
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidRepositoryURL, "Repository URL is malformed", err.Error())
return err
}
if match {
Expand Down
10 changes: 5 additions & 5 deletions cmd/osbuild-worker/jobimpl-osbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {

osbuildVersion, err := osbuild.OSBuildVersion()
if err != nil {
osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "Error getting osbuild binary version", err)
osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "Error getting osbuild binary version", err.Error())
return err
}
osbuildJobResult.OSBuildVersion = osbuildVersion
Expand Down Expand Up @@ -516,7 +516,7 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {
osbuildJobResult.OSBuildOutput, err = executor.RunOSBuild(jobArgs.Manifest, impl.Store, outputDirectory, exports, exportPaths, nil, extraEnv, true, os.Stderr)
// First handle the case when "running" osbuild failed
if err != nil {
osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed", err)
osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed", err.Error())
return err
}

Expand Down Expand Up @@ -544,13 +544,13 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {

// Second handle the case when the build failed, but osbuild finished successfully
if !osbuildJobResult.OSBuildOutput.Success {
var osbErrors []error
var osbErrors []string
if osbuildJobResult.OSBuildOutput.Error != nil {
osbErrors = append(osbErrors, fmt.Errorf("osbuild error: %s", string(osbuildJobResult.OSBuildOutput.Error)))
osbErrors = append(osbErrors, fmt.Sprintf("osbuild error: %s", string(osbuildJobResult.OSBuildOutput.Error)))
}
if osbuildJobResult.OSBuildOutput.Errors != nil {
for _, err := range osbuildJobResult.OSBuildOutput.Errors {
osbErrors = append(osbErrors, fmt.Errorf("manifest validation error: %v", err))
osbErrors = append(osbErrors, fmt.Sprintf("manifest validation error: %v", err))
}
}
osbuildJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorBuildJob, "osbuild build failed", osbErrors)
Expand Down

0 comments on commit 61bf0c3

Please sign in to comment.