osbuild-worker: do not use error in clienterror.Error.Details

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.
This commit is contained in:
Michael Vogt 2024-05-13 17:48:53 +02:00
parent 8ebefbdbc9
commit 61bf0c3235
2 changed files with 6 additions and 6 deletions

View file

@ -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 {

View file

@ -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
@ -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
}
@ -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)