diff --git a/cmd/osbuild-worker/export_test.go b/cmd/osbuild-worker/export_test.go new file mode 100644 index 00000000000..bd6e3143612 --- /dev/null +++ b/cmd/osbuild-worker/export_test.go @@ -0,0 +1,3 @@ +package main + +var WorkerClientErrorFrom = workerClientErrorFrom diff --git a/cmd/osbuild-worker/jobimpl-depsolve.go b/cmd/osbuild-worker/jobimpl-depsolve.go index f50bd209a39..fb8bc5f8abc 100644 --- a/cmd/osbuild-worker/jobimpl-depsolve.go +++ b/cmd/osbuild-worker/jobimpl-depsolve.go @@ -73,6 +73,30 @@ func (impl *DepsolveJobImpl) depsolve(packageSets map[string][]rpmmd.PackageSet, return depsolvedSets, repoConfigs, nil } +func workerClientErrorFrom(err error) (*clienterrors.Error, error) { + switch e := err.(type) { + case dnfjson.Error: + // Error originates from dnf-json + switch e.Kind { + case "DepsolveError": + return clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, err.Error(), e.Reason), nil + case "MarkingErrors": + return clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingErrors, err.Error(), e.Reason), nil + case "RepoError": + return clienterrors.WorkerClientError(clienterrors.ErrorDNFRepoError, err.Error(), e.Reason), nil + default: + err := fmt.Errorf("Unhandled dnf-json error in depsolve job: %v", err) + // This still has the kind/reason format but a kind that's returned + // by dnf-json and not explicitly handled here. + return clienterrors.WorkerClientError(clienterrors.ErrorDNFOtherError, err.Error(), e.Reason), err + } + default: + err := fmt.Errorf("rpmmd error in depsolve job: %v", err) + // Error originates from internal/rpmmd, not from dnf-json + return clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, err.Error(), nil), err + } +} + func (impl *DepsolveJobImpl) Run(job worker.Job) error { logWithId := logrus.WithField("jobId", job.Id()) var args worker.DepsolveJob @@ -106,26 +130,9 @@ func (impl *DepsolveJobImpl) Run(job worker.Job) error { result.PackageSpecs, result.RepoConfigs, err = impl.depsolve(args.PackageSets, args.ModulePlatformID, args.Arch, args.Releasever) if err != nil { - switch e := err.(type) { - case dnfjson.Error: - // Error originates from dnf-json - switch e.Kind { - case "DepsolveError": - result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, err.Error(), e.Reason) - case "MarkingErrors": - result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingErrors, err.Error(), e.Reason) - case "RepoError": - result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFRepoError, err.Error(), e.Reason) - default: - // This still has the kind/reason format but a kind that's returned - // by dnf-json and not explicitly handled here. - result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorDNFOtherError, err.Error(), e.Reason) - logWithId.Errorf("Unhandled dnf-json error in depsolve job: %v", err) - } - case error: - // Error originates from internal/rpmmd, not from dnf-json - result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, err.Error(), nil) - logWithId.Errorf("rpmmd error in depsolve job: %v", err) + result.JobError, err = workerClientErrorFrom(err) + if err != nil { + logWithId.Errorf(err.Error()) } } if err := impl.Solver.CleanCache(); err != nil { diff --git a/cmd/osbuild-worker/jobimpl-depsolve_test.go b/cmd/osbuild-worker/jobimpl-depsolve_test.go new file mode 100644 index 00000000000..1f1c2ba0470 --- /dev/null +++ b/cmd/osbuild-worker/jobimpl-depsolve_test.go @@ -0,0 +1,40 @@ +package main_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/osbuild/images/pkg/dnfjson" + + worker "github.com/osbuild/osbuild-composer/cmd/osbuild-worker" +) + +func TestWorkerClientErrorFromDnfJson(t *testing.T) { + dnfJsonErr := dnfjson.Error{ + Kind: "DepsolveError", + Reason: "something is terribly wrong", + } + clientErr, err := worker.WorkerClientErrorFrom(dnfJsonErr) + assert.NoError(t, err) + // XXX: this is duplicating the details, see https://github.com/osbuild/images/issues/727 + assert.Equal(t, clientErr.String(), `Code: 20, Reason: DNF error occurred: DepsolveError: something is terribly wrong, Details: something is terribly wrong`) +} + +func TestWorkerClientErrorFromOtherError(t *testing.T) { + otherErr := fmt.Errorf("some error") + clientErr, err := worker.WorkerClientErrorFrom(otherErr) + // XXX: this is probably okay but it seems slightly dangerous to + // assume that any "error" we get there is coming from rpmmd, can + // we generate a more typed error from dnfjson here for rpmmd errors? + assert.EqualError(t, err, "rpmmd error in depsolve job: some error") + assert.Equal(t, clientErr.String(), `Code: 23, Reason: rpmmd error in depsolve job: some error, Details: `) +} + +func TestWorkerClientErrorFromNil(t *testing.T) { + clientErr, err := worker.WorkerClientErrorFrom(nil) + // XXX: this is wrong, it should generate an internal error + assert.EqualError(t, err, "rpmmd error in depsolve job: ") + assert.Equal(t, clientErr.String(), `Code: 23, Reason: rpmmd error in depsolve job: , Details: `) +}