From 61bf0c3235c250fa4587b2da4e9c3dcfb120d20b Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 13 May 2024 17:48:53 +0200 Subject: [PATCH] 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. --- cmd/osbuild-worker/jobimpl-depsolve.go | 2 +- cmd/osbuild-worker/jobimpl-osbuild.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/osbuild-worker/jobimpl-depsolve.go b/cmd/osbuild-worker/jobimpl-depsolve.go index f50bd209a3..4dbc5495e8 100644 --- a/cmd/osbuild-worker/jobimpl-depsolve.go +++ b/cmd/osbuild-worker/jobimpl-depsolve.go @@ -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 { diff --git a/cmd/osbuild-worker/jobimpl-osbuild.go b/cmd/osbuild-worker/jobimpl-osbuild.go index 4dde72ce0b..6f3930cbb3 100644 --- a/cmd/osbuild-worker/jobimpl-osbuild.go +++ b/cmd/osbuild-worker/jobimpl-osbuild.go @@ -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)