obuild-worker: extract workerClientErrorFrom() helper and add tests

Tiny commit to extract a helper from DepsolveJobImpl.Run() that
can then be unit tested.

This should help with osbuild/images#727

Author: mvo5

cmd/osbuild-worker/export_test.go

@@ -0,0 +1,3 @@
+package main
+
+var WorkerClientErrorFrom = workerClientErrorFrom

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 {

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: <nil>`)
+}
+
+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: <nil>")
+	assert.Equal(t, clientErr.String(), `Code: 23, Reason: rpmmd error in depsolve job: <nil>, Details: <nil>`)
+}