From 8028bbf3c90944f4bb021a4ad62aaa1047b26ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Sch=C3=BCller?= Date: Thu, 4 Jul 2024 17:06:45 +0200 Subject: [PATCH] cmd/osbuild-worker/jobimpl-depsolve: show error.Reason only once as now the .Reason is properly passed over - it was printed twice --- cmd/osbuild-worker/jobimpl-depsolve.go | 19 +++++++++++++------ cmd/osbuild-worker/jobimpl-depsolve_test.go | 10 ++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/cmd/osbuild-worker/jobimpl-depsolve.go b/cmd/osbuild-worker/jobimpl-depsolve.go index 2e4973e982c..e6eedf2f095 100644 --- a/cmd/osbuild-worker/jobimpl-depsolve.go +++ b/cmd/osbuild-worker/jobimpl-depsolve.go @@ -78,23 +78,30 @@ func workerClientErrorFrom(err error) (*clienterrors.Error, error) { case dnfjson.Error: // Error originates from dnf-json reason := fmt.Sprintf("DNF error occurred: %s", e.Kind) + details := e.Reason switch e.Kind { case "DepsolveError": - return clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, reason, e.Reason), nil + return clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, reason, details), nil case "MarkingErrors": - return clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingErrors, reason, e.Reason), nil + return clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingErrors, reason, details), nil case "RepoError": - return clienterrors.WorkerClientError(clienterrors.ErrorDNFRepoError, reason, e.Reason), nil + return clienterrors.WorkerClientError(clienterrors.ErrorDNFRepoError, reason, details), 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, reason, e.Reason), err + return clienterrors.WorkerClientError(clienterrors.ErrorDNFOtherError, reason, details), err } default: - err := fmt.Errorf("rpmmd error in depsolve job: %v", err) + reason := "rpmmd error in depsolve job" + details := "" + if err == nil { + err = fmt.Errorf("workerClientErrorFrom expected an error to be processed. Not nil") + } else { + details = err.Error() + } // Error originates from internal/rpmmd, not from dnf-json - return clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, err.Error(), nil), err + return clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, reason, details), err } } diff --git a/cmd/osbuild-worker/jobimpl-depsolve_test.go b/cmd/osbuild-worker/jobimpl-depsolve_test.go index 6c4f5db0b37..e3dcfcdc486 100644 --- a/cmd/osbuild-worker/jobimpl-depsolve_test.go +++ b/cmd/osbuild-worker/jobimpl-depsolve_test.go @@ -18,7 +18,6 @@ func TestWorkerClientErrorFromDnfJson(t *testing.T) { } 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, `Code: 20, Reason: DNF error occurred: DepsolveError, Details: something is terribly wrong`, clientErr.String()) } @@ -28,13 +27,12 @@ func TestWorkerClientErrorFromOtherError(t *testing.T) { // 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, `Code: 23, Reason: rpmmd error in depsolve job: some error, Details: `, clientErr.String()) + assert.EqualError(t, err, "some error") + assert.Equal(t, `Code: 23, Reason: rpmmd error in depsolve job, Details: some error`, clientErr.String()) } 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, `Code: 23, Reason: rpmmd error in depsolve job: , Details: `, clientErr.String()) + assert.EqualError(t, err, "workerClientErrorFrom expected an error to be processed. Not nil") + assert.Equal(t, `Code: 23, Reason: rpmmd error in depsolve job, Details: `, clientErr.String()) }