clienterrors: rename WorkerClientError to clienterrors.New

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.
osbuild · Jun 21, 2024 · 5b6f39f · 5b6f39f
1 parent 06d0bb6
commit 5b6f39f
Show file tree

Hide file tree

Showing 19 changed files with 188 additions and 188 deletions.
diff --git a/cmd/osbuild-worker/jobimpl-awsec2.go b/cmd/osbuild-worker/jobimpl-awsec2.go
@@ -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
@@ -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
 		}
 
@@ -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

diff --git a/cmd/osbuild-worker/jobimpl-container-resolve.go b/cmd/osbuild-worker/jobimpl-container-resolve.go
@@ -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{

diff --git a/cmd/osbuild-worker/jobimpl-depsolve.go b/cmd/osbuild-worker/jobimpl-depsolve.go
@@ -79,21 +79,21 @@ func workerClientErrorFrom(err error) (*clienterrors.Error, error) {
 		// Error originates from dnf-json
 		switch e.Kind {
 		case "DepsolveError":
-			return clienterrors.WorkerClientError(clienterrors.ErrorDNFDepsolveError, err.Error(), e.Reason), nil
+			return clienterrors.New(clienterrors.ErrorDNFDepsolveError, err.Error(), e.Reason), nil
 		case "MarkingErrors":
-			return clienterrors.WorkerClientError(clienterrors.ErrorDNFMarkingErrors, err.Error(), e.Reason), nil
+			return clienterrors.New(clienterrors.ErrorDNFMarkingErrors, err.Error(), e.Reason), nil
 		case "RepoError":
-			return clienterrors.WorkerClientError(clienterrors.ErrorDNFRepoError, err.Error(), e.Reason), nil
+			return clienterrors.New(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
+			return clienterrors.New(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
+		return clienterrors.New(clienterrors.ErrorRPMMDError, err.Error(), nil), err
 	}
 }
 
@@ -114,7 +114,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 {

diff --git a/cmd/osbuild-worker/jobimpl-file-resolve.go b/cmd/osbuild-worker/jobimpl-file-resolve.go
@@ -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",
@@ -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,

diff --git a/cmd/osbuild-worker/jobimpl-koji-finalize.go b/cmd/osbuild-worker/jobimpl-koji-finalize.go
@@ -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
 	}
 
@@ -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
 	}
 
@@ -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
 		}
 
@@ -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
 	}
 

diff --git a/cmd/osbuild-worker/jobimpl-koji-init.go b/cmd/osbuild-worker/jobimpl-koji-init.go
@@ -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)