Better error messages / error codes (#5)

* Better error messages / error codes

* Fixed review issues

.golangci.yml

@@ -0,0 +1,8 @@
+linters:
+  enable:
+    - asciicheck
+    - bodyclose
+    - dupl
+    - errorlint
+    - exportloopref
+    - funlen

client_cluster.go

@@ -1,8 +1,6 @@
 package ovirtclient
 
 import (
-	"fmt"
-
 	ovirtsdk4 "github.com/ovirt/go-ovirt"
 )
 
@@ -25,12 +23,12 @@ type Cluster interface {
 func convertSDKCluster(sdkCluster *ovirtsdk4.Cluster) (Cluster, error) {
 	id, ok := sdkCluster.Id()
 	if !ok {
-		return nil, fmt.Errorf("failed to fetch ID for cluster")
+		return nil, newError(EFieldMissing, "failed to fetch ID for cluster")
 	}
 
 	name, ok := sdkCluster.Name()
 	if !ok {
-		return nil, fmt.Errorf("failed to fetch name for cluster %s", id)
+		return nil, newError(EFieldMissing, "failed to fetch name for cluster %s", id)
 	}
 	return &cluster{
 		id:   id,

client_cluster_get.go

@@ -1,21 +1,26 @@
 package ovirtclient
 
-import (
-	"fmt"
-)
-
 func (o *oVirtClient) GetCluster(id string) (cluster Cluster, err error) {
 	response, err := o.conn.SystemService().ClustersService().ClusterService(id).Get().Send()
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch cluster ID %s (%w)", id, err)
+		return nil, wrap(err, "failed to fetch cluster ID %s", id)
 	}
 	sdkCluster, ok := response.Cluster()
 	if !ok {
-		return nil, fmt.Errorf("no cluster returned when getting cluster ID %s", id)
+		return nil, newError(
+			ENotFound,
+			"no cluster returned when getting cluster ID %s",
+			id,
+		)
 	}
 	cluster, err = convertSDKCluster(sdkCluster)
 	if err != nil {
-		return nil, fmt.Errorf("failed to convert cluster %s (%w)", id, err)
+		return nil, wrap(
+			err,
+			EBug,
+			"failed to convert cluster %s",
+			id,
+		)
 	}
 	return cluster, nil
 }

client_cluster_list.go

@@ -1,26 +1,23 @@
 package ovirtclient
 
-import (
-	"fmt"
-)
-
 func (o *oVirtClient) ListClusters() ([]Cluster, error) {
 	clustersResponse, err := o.conn.SystemService().ClustersService().List().Send()
 	if err != nil {
-		return nil, fmt.Errorf(
-			"failed to list oVirt clusters (%w)",
+		return nil, wrap(
 			err,
+			EUnidentified,
+			"failed to list oVirt clusters",
 		)
 	}
 	sdkClusters, ok := clustersResponse.Clusters()
 	if !ok {
-		return nil, fmt.Errorf("no clusters returned from clusters list API call")
+		return []Cluster{}, nil
 	}
 	clusters := make([]Cluster, len(sdkClusters.Slice()))
 	for i, sdkCluster := range sdkClusters.Slice() {
 		clusters[i], err = convertSDKCluster(sdkCluster)
 		if err != nil {
-			return nil, fmt.Errorf("failed to convert cluster during cluster listing item %d (%w)", i, err)
+			return nil, wrap(err, EBug, "failed to convert cluster during cluster listing item %d", i)
 		}
 	}
 	return clusters, nil

client_disk.go

@@ -2,14 +2,13 @@ package ovirtclient
 
 import (
 	"context"
-	"fmt"
 	"io"
 
 	ovirtsdk4 "github.com/ovirt/go-ovirt"
 )
 
 type DiskClient interface {
-	// UploadImage uploads an image file into a disk. The actual upload takes place in the
+	// StartImageUpload uploads an image file into a disk. The actual upload takes place in the
 	// background and can be tracked using the returned UploadImageProgress object.
 	//
 	// Parameters are as follows:
@@ -57,22 +56,31 @@ type DiskClient interface {
 
 	// ListDisks lists all disks.
 	ListDisks() ([]Disk, error)
-	// GetDisk fetches a disk with a specific ID from the
+	// GetDisk fetches a disk with a specific ID from the oVirt Engine.
 	GetDisk(diskID string) (Disk, error)
 	// RemoveDisk removes a disk with a specific ID.
-	RemoveDisk(diskID string) error
+	RemoveDisk(ctx context.Context, diskID string) error
 }
 
+// UploadImageResult represents the completed image upload.
 type UploadImageResult interface {
+	// Disk returns the disk that has been created as the result of the image upload.
 	Disk() Disk
+	// CorrelationID returns the opaque correlation ID for the upload.
 	CorrelationID() string
 }
 
+// Disk is a disk in oVirt.
 type Disk interface {
+	// ID is the unique ID for this disk.
 	ID() string
+	// Alias is the name for this disk set by the user.
 	Alias() string
+	// ProvisionedSize is the size visible to the virtual machine.
 	ProvisionedSize() uint64
+	// Format is the format of the image.
 	Format() ImageFormat
+	// StorageDomainID is the ID of the storage system used for this disk.
 	StorageDomainID() string
 }
 
@@ -104,7 +112,7 @@ const (
 func convertSDKDisk(sdkDisk *ovirtsdk4.Disk) (Disk, error) {
 	id, ok := sdkDisk.Id()
 	if !ok {
-		return nil, fmt.Errorf("disk does not contain an ID")
+		return nil, newError(EFieldMissing, "disk does not contain an ID")
 	}
 	var storageDomainID string
 	if sdkStorageDomain, ok := sdkDisk.StorageDomain(); ok {
@@ -118,19 +126,19 @@ func convertSDKDisk(sdkDisk *ovirtsdk4.Disk) (Disk, error) {
 		}
 	}
 	if storageDomainID == "" {
-		return nil, fmt.Errorf("failed to find a valid storage domain ID for disk %s", id)
+		return nil, newError(EFieldMissing, "failed to find a valid storage domain ID for disk %s", id)
 	}
 	alias, ok := sdkDisk.Alias()
 	if !ok {
-		return nil, fmt.Errorf("disk %s does not contain an alias", id)
+		return nil, newError(EFieldMissing, "disk %s does not contain an alias", id)
 	}
 	provisionedSize, ok := sdkDisk.ProvisionedSize()
 	if !ok {
-		return nil, fmt.Errorf("disk %s does not contain a provisioned size", id)
+		return nil, newError(EFieldMissing, "disk %s does not contain a provisioned size", id)
 	}
 	format, ok := sdkDisk.Format()
 	if !ok {
-		return nil, fmt.Errorf("disk %s has no format field", id)
+		return nil, newError(EFieldMissing, "disk %s has no format field", id)
 	}
 	return &disk{
 		id:              id,

client_disk_get.go

@@ -1,21 +1,32 @@
 package ovirtclient
 
-import (
-	"fmt"
-)
-
 func (o *oVirtClient) GetDisk(diskID string) (Disk, error) {
 	response, err := o.conn.SystemService().DisksService().DiskService(diskID).Get().Send()
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch disk %s (%w)", diskID, err)
+		return nil, wrap(
+			err,
+			EUnidentified,
+			"failed to fetch disk %s",
+			diskID,
+		)
 	}
 	sdkDisk, ok := response.Disk()
 	if !ok {
-		return nil, fmt.Errorf("disk %s response did not contain a disk (%w)", diskID, err)
+		return nil, wrap(
+			err,
+			ENotFound,
+			"disk %s response did not contain a disk",
+			diskID,
+		)
 	}
 	disk, err := convertSDKDisk(sdkDisk)
 	if err != nil {
-		return nil, fmt.Errorf("failed to convert disk %s (%w)", diskID, err)
+		return nil, wrap(
+			err,
+			EBug,
+			"failed to convert disk %s",
+			diskID,
+		)
 	}
 	return disk, nil
 }

client_disk_list.go

@@ -1,23 +1,23 @@
 package ovirtclient
 
-import (
-	"fmt"
-)
-
 func (o *oVirtClient) ListDisks() ([]Disk, error) {
 	response, err := o.conn.SystemService().DisksService().List().Send()
 	if err != nil {
-		return nil, fmt.Errorf("failed to list disks (%w)", err)
+		return nil, wrap(
+			err,
+			EUnidentified,
+			"failed to list disks",
+		)
 	}
 	sdkDisks, ok := response.Disks()
 	if !ok {
-		return nil, fmt.Errorf("disk list response does not contain disks")
+		return []Disk{}, nil
 	}
 	result := make([]Disk, len(sdkDisks.Slice()))
 	for i, sdkDisk := range sdkDisks.Slice() {
 		disk, err := convertSDKDisk(sdkDisk)
 		if err != nil {
-			return nil, fmt.Errorf("failed to convert disk %d (%w)", i, err)
+			return nil, wrap(err, EBug, "failed to convert disk item %d", i)
 		}
 		result[i] = disk
 	}

client_disk_remove.go

@@ -1,12 +1,34 @@
 package ovirtclient
 
 import (
-	"fmt"
+	"context"
+	"time"
 )
 
-func (o *oVirtClient) RemoveDisk(diskID string) error {
-	if _, err := o.conn.SystemService().DisksService().DiskService(diskID).Remove().Send(); err != nil {
-		return fmt.Errorf("failed to remove disk %s (%w)", diskID, err)
+func (o *oVirtClient) RemoveDisk(ctx context.Context, diskID string) error {
+	var lastError EngineError
+	for {
+		_, err := o.conn.SystemService().DisksService().DiskService(diskID).Remove().Send()
+		if err == nil {
+			return err
+		}
+		lastError = wrap(
+			err,
+			EUnidentified,
+			"failed to remove disk %s",
+			diskID,
+		)
+		if !lastError.CanAutoRetry() {
+			return lastError
+		}
+		select {
+		case <-ctx.Done():
+			return wrap(
+				lastError,
+				ETimeout,
+				"timeout while removing disk",
+			)
+		case <-time.After(10 * time.Second):
+		}
 	}
-	return nil
 }