Skip to content

Commit

Permalink
cmd/osbuild-worker/jobimpl-depsolve: show error.Reason only once
Browse files Browse the repository at this point in the history
as now the .Reason is properly passed over - it was printed twice
  • Loading branch information
schuellerf committed Jul 5, 2024
1 parent 928c525 commit 8028bbf
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
19 changes: 13 additions & 6 deletions cmd/osbuild-worker/jobimpl-depsolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 := "<nil>"
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
}
}

Expand Down
10 changes: 4 additions & 6 deletions cmd/osbuild-worker/jobimpl-depsolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

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

0 comments on commit 8028bbf

Please sign in to comment.