Skip to content

Commit

Permalink
clienterrors: rename WorkerClientError to clienterrors.New
Browse files Browse the repository at this point in the history
The usual convention to create new object is to prefix `New*` so
this commit renames the `WorkerClientError`. Initially I thought
it would be `NewWorkerClientError()` but looking at the package
prefix it seems unneeded, i.e. `clienterrors.New()` already
provides enough context it seems and it's the only error we
construct.

We could consider renaming it to `clienterror` (singular) too
but that could be a followup.

I would also like to make `clienterror.Error` implement the
`error` interface but that should be a followup to make this
(mechanical) rename trivial to review.
  • Loading branch information
mvo5 committed Jul 31, 2024
1 parent 09445a1 commit 573b349
Show file tree
Hide file tree
Showing 19 changed files with 188 additions and 188 deletions.
30 changes: 15 additions & 15 deletions cmd/osbuild-worker/jobimpl-awsec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,31 +36,31 @@ func (impl *AWSEC2CopyJobImpl) Run(job worker.Job) error {
var args worker.AWSEC2CopyJob
err := job.Args(&args)
if err != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorParsingJobArgs, fmt.Sprintf("Error parsing arguments: %v", err), nil)
result.JobError = clienterrors.New(clienterrors.ErrorParsingJobArgs, fmt.Sprintf("Error parsing arguments: %v", err), nil)
return err
}

aws, err := getAWS(impl.AWSCreds, args.TargetRegion)
if err != nil {
logWithId.Errorf("Error creating aws client: %v", err)
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, "Invalid worker config", nil)
result.JobError = clienterrors.New(clienterrors.ErrorInvalidConfig, "Invalid worker config", nil)
return err
}

ami, err := aws.CopyImage(args.TargetName, args.Ami, args.SourceRegion)
if err != nil {
logWithId.Errorf("Error copying ami: %v", err)
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Error copying ami %s", args.Ami), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Error copying ami %s", args.Ami), nil)
if aerr, ok := err.(awserr.Error); ok {
switch aerr.Code() {
case "InvalidRegion":
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Invalid source region '%s'", args.SourceRegion), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Invalid source region '%s'", args.SourceRegion), nil)
case "InvalidAMIID.Malformed":
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Malformed source ami id '%s'", args.Ami), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Malformed source ami id '%s'", args.Ami), nil)
case "InvalidAMIID.NotFound":
fallthrough // CopyImage returns InvalidRequest instead of InvalidAMIID.NotFound
case "InvalidRequest":
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Source ami '%s' not found", args.Ami), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Source ami '%s' not found", args.Ami), nil)
}
}
return err
Expand Down Expand Up @@ -89,24 +89,24 @@ func (impl *AWSEC2ShareJobImpl) Run(job worker.Job) error {
var args worker.AWSEC2ShareJob
err := job.Args(&args)
if err != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorParsingJobArgs, fmt.Sprintf("Error parsing arguments: %v", err), nil)
result.JobError = clienterrors.New(clienterrors.ErrorParsingJobArgs, fmt.Sprintf("Error parsing arguments: %v", err), nil)
return err
}

if args.Ami == "" || args.Region == "" {
if job.NDynamicArgs() != 1 {
logWithId.Error("No arguments given and dynamic args empty")
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorNoDynamicArgs, "An ec2 share job should have args or depend on an ec2 copy job", nil)
result.JobError = clienterrors.New(clienterrors.ErrorNoDynamicArgs, "An ec2 share job should have args or depend on an ec2 copy job", nil)
return nil
}
var cjResult worker.AWSEC2CopyJobResult
err = job.DynamicArgs(0, &cjResult)
if err != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorParsingDynamicArgs, "Error parsing dynamic args as ec2 copy job", nil)
result.JobError = clienterrors.New(clienterrors.ErrorParsingDynamicArgs, "Error parsing dynamic args as ec2 copy job", nil)
return err
}
if cjResult.JobError != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorJobDependency, "AWSEC2CopyJob dependency failed", nil)
result.JobError = clienterrors.New(clienterrors.ErrorJobDependency, "AWSEC2CopyJob dependency failed", nil)
return nil
}

Expand All @@ -117,22 +117,22 @@ func (impl *AWSEC2ShareJobImpl) Run(job worker.Job) error {
aws, err := getAWS(impl.AWSCreds, args.Region)
if err != nil {
logWithId.Errorf("Error creating aws client: %v", err)
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorInvalidConfig, "Invalid worker config", nil)
result.JobError = clienterrors.New(clienterrors.ErrorInvalidConfig, "Invalid worker config", nil)
return err
}

err = aws.ShareImage(args.Ami, args.ShareWithAccounts)
if err != nil {
logWithId.Errorf("Error sharing image: %v", err)
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Error sharing image with target %v", args.ShareWithAccounts), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Error sharing image with target %v", args.ShareWithAccounts), nil)
if aerr, ok := err.(awserr.Error); ok {
switch aerr.Code() {
case "InvalidAMIID.Malformed":
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Malformed ami id '%s'", args.Ami), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Malformed ami id '%s'", args.Ami), nil)
case "InvalidAMIID.NotFound":
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Ami '%s' not found in region '%s'", args.Ami, args.Region), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Ami '%s' not found in region '%s'", args.Ami, args.Region), nil)
case "InvalidAMIAttributeItemValue":
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorSharingTarget, fmt.Sprintf("Invalid user id to share ami with: %v", args.ShareWithAccounts), nil)
result.JobError = clienterrors.New(clienterrors.ErrorSharingTarget, fmt.Sprintf("Invalid user id to share ami with: %v", args.ShareWithAccounts), nil)
}
}
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-worker/jobimpl-container-resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (impl *ContainerResolveJobImpl) Run(job worker.Job) error {
specs, err := resolver.Finish()

if err != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorContainerResolution, err.Error(), nil)
result.JobError = clienterrors.New(clienterrors.ErrorContainerResolution, err.Error(), nil)
} else {
for i, spec := range specs {
result.Specs[i] = worker.ContainerSpec{
Expand Down
12 changes: 6 additions & 6 deletions cmd/osbuild-worker/jobimpl-depsolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,16 @@ func workerClientErrorFrom(err error) (*clienterrors.Error, error) {
details := e.Reason
switch e.Kind {
case "DepsolveError":
return clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, reason, details), nil
return clienterrors.New(clienterrors.ErrorDNFDepsolveError, reason, details), nil
case "MarkingErrors":
return clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingErrors, reason, details), nil
return clienterrors.New(clienterrors.ErrorDNFMarkingErrors, reason, details), nil
case "RepoError":
return clienterrors.WorkerClientError(clienterrors.ErrorDNFRepoError, reason, details), nil
return clienterrors.New(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, details), err
return clienterrors.New(clienterrors.ErrorDNFOtherError, reason, details), err
}
default:
reason := "rpmmd error in depsolve job"
Expand All @@ -101,7 +101,7 @@ func workerClientErrorFrom(err error) (*clienterrors.Error, error) {
details = err.Error()
}
// Error originates from internal/rpmmd, not from dnf-json
return clienterrors.WorkerClientError(clienterrors.ErrorRPMMDError, reason, details), err
return clienterrors.New(clienterrors.ErrorRPMMDError, reason, details), err
}
}

Expand All @@ -122,7 +122,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.Error())
result.JobError = clienterrors.New(clienterrors.ErrorInvalidRepositoryURL, "Repository URL is malformed", err.Error())
return err
}
if match {
Expand Down
4 changes: 2 additions & 2 deletions cmd/osbuild-worker/jobimpl-file-resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (impl *FileResolveJobImpl) Run(job worker.Job) error {

if result.Results == nil || len(result.Results) == 0 {
logWithId.Infof("Resolving file contents failed: %v", err)
result.JobError = clienterrors.WorkerClientError(
result.JobError = clienterrors.New(
clienterrors.ErrorRemoteFileResolution,
"Error resolving file contents",
"All remote file contents returned empty",
Expand Down Expand Up @@ -70,7 +70,7 @@ func (impl *FileResolveJobImpl) Run(job worker.Job) error {
if len(resolutionErrors) == 0 {
result.Success = true
} else {
result.JobError = clienterrors.WorkerClientError(
result.JobError = clienterrors.New(
clienterrors.ErrorRemoteFileResolution,
"at least one file resolution failed",
resolutionErrors,
Expand Down
10 changes: 5 additions & 5 deletions cmd/osbuild-worker/jobimpl-koji-finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error {

err := job.Args(args)
if err != nil {
kojiFinalizeJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorParsingJobArgs, "Error parsing job args", err.Error())
kojiFinalizeJobResult.JobError = clienterrors.New(clienterrors.ErrorParsingJobArgs, "Error parsing job args", err.Error())
return err
}

Expand All @@ -124,13 +124,13 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error {
var osbuildResults []worker.OSBuildJobResult
initArgs, osbuildResults, err = extractDynamicArgs(job)
if err != nil {
kojiFinalizeJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorParsingDynamicArgs, "Error parsing dynamic args", err.Error())
kojiFinalizeJobResult.JobError = clienterrors.New(clienterrors.ErrorParsingDynamicArgs, "Error parsing dynamic args", err.Error())
return err
}

// Check the dependencies early.
if hasFailedDependency(*initArgs, osbuildResults) {
kojiFinalizeJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiFailedDependency, "At least one job dependency failed", nil)
kojiFinalizeJobResult.JobError = clienterrors.New(clienterrors.ErrorKojiFailedDependency, "At least one job dependency failed", nil)
return nil
}

Expand All @@ -149,7 +149,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error {
kojiTargetResults := buildResult.TargetResultsByName(target.TargetNameKoji)
// Only a single Koji target is allowed per osbuild job
if len(kojiTargetResults) != 1 {
kojiFinalizeJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiFinalize, "Exactly one Koji target result is expected per osbuild job", nil)
kojiFinalizeJobResult.JobError = clienterrors.New(clienterrors.ErrorKojiFinalize, "Exactly one Koji target result is expected per osbuild job", nil)
return nil
}

Expand Down Expand Up @@ -301,7 +301,7 @@ func (impl *KojiFinalizeJobImpl) Run(job worker.Job) error {

err = impl.kojiImport(args.Server, build, buildRoots, outputs, args.KojiDirectory, initArgs.Token)
if err != nil {
kojiFinalizeJobResult.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiFinalize, err.Error(), nil)
kojiFinalizeJobResult.JobError = clienterrors.New(clienterrors.ErrorKojiFinalize, err.Error(), nil)
return err
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-worker/jobimpl-koji-init.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (impl *KojiInitJobImpl) Run(job worker.Job) error {
var result worker.KojiInitJobResult
result.Token, result.BuildID, err = impl.kojiInit(args.Server, args.Name, args.Version, args.Release)
if err != nil {
result.JobError = clienterrors.WorkerClientError(clienterrors.ErrorKojiInit, err.Error(), nil)
result.JobError = clienterrors.New(clienterrors.ErrorKojiInit, err.Error(), nil)
}

err = job.Update(&result)
Expand Down
Loading

0 comments on commit 573b349

Please sign in to comment.