8 better error handling (#13)

* wip

* clean up errors

Author: tafseer-khan

container.go

@@ -11,27 +11,26 @@ import (
 // Run starts the container and waits for the container to exit before returning the container logs.
 func (c *Container) Run(ctx context.Context) (*MuxedReadCloser, error) {
 	if err := c.image.client.ContainerStart(ctx, c.id, types.ContainerStartOptions{}); err != nil {
-		return nil, fmt.Errorf("starting container with id `%s` for image `%s` failed with: %s", c.id, c.image.image, err)
+		return nil, errorContainerStart(c.id, c.image.image, err)
 	}
 
-	err := c.Wait(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("waiting for container with id `%s` for image `%s` failed with: %s", c.id, c.image.image, err)
+	if err := c.Wait(ctx); err != nil {
+		return nil, err
 	}
 
 	info, err := c.image.client.ContainerInspect(ctx, c.id)
 	if err != nil {
-		return nil, fmt.Errorf("inspecting container with id `%s` for image `%s` failed with: %s", c.id, c.image.image, err)
+		return nil, errorContainerInspect(c.id, c.image.image, err)
 	}
 
 	var RetCodeErr error
 	if info.ContainerJSONBase.State.ExitCode != 0 {
-		RetCodeErr = fmt.Errorf("running container with id `%s` for image `%s` failed with exit-code: %d", c.id, c.image.image, info.ContainerJSONBase.State.ExitCode)
+		RetCodeErr = errorContainerExitCode(c.id, c.image.image, info.ContainerJSONBase.State.ExitCode)
 	}
 
 	muxed, err := c.image.client.ContainerLogs(ctx, c.id, types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true})
 	if err != nil {
-		return nil, fmt.Errorf("getting container logs for image `%s` failed with %s", c.image.image, err)
+		return nil, errorContainerLogs(c.id, c.image.image, err)
 	}
 
 	c.Cleanup(ctx)
@@ -46,7 +45,7 @@ func (c *Container) Wait(ctx context.Context) error {
 	select {
 	case err := <-errCh:
 		if err != nil {
-			return fmt.Errorf("container with id `%s` returned error: %s", c.id, err)
+			return errorContainerWait(c.id, c.image.image, err)
 		}
 	case <-statusCh:
 	}

errors.go

@@ -0,0 +1,92 @@
+package containers
+
+import (
+	"errors"
+	"fmt"
+)
+
+var errorBasicFormat = "%s with: %w"
+
+// Image Method Errors
+var (
+	ErrorImageOptions         = errors.New("image options failed")
+	ErrorImageBuild           = errors.New("building image failed")
+	ErrorImagePull            = errors.New("pulling image failed")
+	ErrorClientPull           = errors.New("client pull failed")
+	ErrorImageBuildDockerFile = errors.New("building Dockerfile failed")
+	ErrorImageBuildResCopy    = errors.New("copying response from image build failed")
+	ErrorImagePullStatus      = errors.New("copying pull status failed")
+
+	ErrorContainerOptions = errors.New("container options failed")
+	ErrorContainerCreate  = errors.New("creating container failed")
+
+	errorImageFormat = "%s for image `%s` with: %w"
+)
+
+func errorImageOptions(image string, err error) error {
+	return fmt.Errorf(errorImageFormat, ErrorImageOptions, image, err)
+}
+
+func errorImageBuild(image string, err error) error {
+	return fmt.Errorf(errorImageFormat, ErrorImageBuild, image, err)
+}
+
+func errorImagePull(image string, err error) error {
+	return fmt.Errorf(errorImageFormat, ErrorImagePull, image, err)
+}
+
+func errorClientPull(err error) error {
+	return fmt.Errorf(errorBasicFormat, ErrorClientPull, err)
+}
+
+func errorImageBuildDockerFile(err error) error {
+	return fmt.Errorf(errorBasicFormat, ErrorImageBuildDockerFile, err)
+}
+
+func errorImageBuildResCopy(err error) error {
+	return fmt.Errorf(errorBasicFormat, ErrorImageBuildResCopy, err)
+}
+
+func errorImagePullStatus(err error) error {
+	return fmt.Errorf(errorBasicFormat, ErrorImagePullStatus, err)
+}
+
+func errorContainerOptions(image string, err error) error {
+	return fmt.Errorf(errorImageFormat, ErrorContainerOptions, image, err)
+}
+
+func errorContainerCreate(image string, err error) error {
+	return fmt.Errorf(errorImageFormat, ErrorContainerCreate, image, err)
+}
+
+// Container Method Errors
+var (
+	ErrorContainerStart   = errors.New("start container failed")
+	ErrorContainerWait    = errors.New("container wait failed")
+	ErrorClientWait       = errors.New("client wait failed")
+	ErrorContainerInspect = errors.New("inspecting container failed")
+	ErrorExitCode         = errors.New("exit-code")
+	ErrorContainerLogs    = errors.New("getting container logs failed")
+
+	errorContainerFormat = "%s for container Id:`%s` image:`%s` with: %w"
+)
+
+func errorContainerStart(id, image string, err error) error {
+	return fmt.Errorf(errorContainerFormat, ErrorContainerStart, id, image, err)
+}
+
+func errorContainerWait(id, image string, err error) error {
+	return fmt.Errorf(errorContainerFormat, ErrorContainerWait, id, image, err)
+}
+
+func errorContainerInspect(id, image string, err error) error {
+	return fmt.Errorf(errorContainerFormat, ErrorContainerInspect, id, image, err)
+}
+
+func errorContainerExitCode(id, image string, code int) error {
+	return fmt.Errorf("container Id:`%s` image:`%s` failed with %w:%d", id, image, ErrorExitCode, code)
+}
+
+func errorContainerLogs(id, image string, err error) error {
+	return fmt.Errorf(errorContainerFormat, ErrorContainerLogs, id, image, err)
+}

image.go

@@ -17,49 +17,39 @@ import (
 
 // Image initializes the given image, and attempts to pull the container from docker hub.
 // If the Build() Option is provided then the given DockerFile tarball is built and returned.
-func (c *Client) Image(ctx context.Context, image string, options ...ImageOption) (*Image, error) {
-	_image := &Image{
+func (c *Client) Image(ctx context.Context, name string, options ...ImageOption) (image *Image, err error) {
+	image = &Image{
 		client: c,
-		image:  image,
+		image:  name,
 	}
 
 	for _, opt := range options {
-		err := opt(_image)
-		if err != nil {
-			return nil, fmt.Errorf("running image options for image `%s` failed with: %s", image, err)
+		if err := opt(image); err != nil {
+			return nil, errorImageOptions(name, err)
 		}
 	}
 
-	imageExists := _image.checkImageExists(ctx)
-	if _image.buildTarball != nil {
-		if ForceRebuild || !imageExists {
-			err := _image.buildImage(ctx)
-			if err != nil {
-				return nil, fmt.Errorf("building image `%s` failed with %s", image, err)
-			}
+	imageExists := image.checkImageExists(ctx)
+	if image.buildTarball != nil && (ForceRebuild || !imageExists) {
+		if err := image.buildImage(ctx); err != nil {
+			return nil, errorImageBuild(name, err)
+		}
+	} else {
+		if image, err = image.Pull(ctx); err != nil {
+			return nil, errorImagePull(name, err)
 		}
-
-		return _image, nil
-	}
-
-	_image, err := _image.Pull(ctx)
-	if err != nil && !imageExists {
-		return nil, fmt.Errorf("pulling image `%s` failed with %s", image, err)
 	}
 
-	return _image, nil
+	return
 }
 
 // checkImage checks the docker host client if the image is known.
 func (i *Image) checkImageExists(ctx context.Context) bool {
 	res, err := i.client.ImageList(ctx, types.ImageListOptions{
 		Filters: NewFilter("reference", i.image),
 	})
-	if err != nil || len(res) < 1 {
-		return false
-	}
 
-	return true
+	return err == nil && len(res) > 0
 }
 
 // buildImage builds a DockerFile tarball as a docker image.
@@ -71,16 +61,16 @@ func (i *Image) buildImage(ctx context.Context) error {
 			Context:    i.buildTarball,
 			Dockerfile: "Dockerfile",
 			Tags:       []string{i.image},
-			Remove:     true})
+			Remove:     true,
+		},
+	)
 	if err != nil {
-		return fmt.Errorf("building Dockerfile for image `%s` failed with: %s", i.image, err)
+		return errorImageBuildDockerFile(err)
 	}
-
 	defer imageBuildResponse.Body.Close()
 
-	_, err = io.Copy(os.Stdout, imageBuildResponse.Body)
-	if err != nil {
-		return fmt.Errorf("copying response from  image `%s` build failed with :%s", i.image, err)
+	if _, err = io.Copy(os.Stdout, imageBuildResponse.Body); err != nil {
+		return errorImageBuildResCopy(err)
 	}
 
 	return nil
@@ -90,14 +80,12 @@ func (i *Image) buildImage(ctx context.Context) error {
 func (i *Image) Pull(ctx context.Context) (*Image, error) {
 	reader, err := i.client.ImagePull(ctx, i.image, types.ImagePullOptions{})
 	if err != nil {
-		return i, fmt.Errorf("pulling image `%s`, failed with %s", i.image, err)
+		return i, errorClientPull(err)
 	}
-
 	defer reader.Close()
-	fileScanner := bufio.NewScanner(reader)
 
+	fileScanner := bufio.NewScanner(reader)
 	fileScanner.Split(bufio.ScanLines)
-
 	for fileScanner.Scan() {
 		var line = fileScanner.Bytes()
 		var status struct {
@@ -108,9 +96,9 @@ func (i *Image) Pull(ctx context.Context) (*Image, error) {
 			}
 			Id string
 		}
-		err = json.Unmarshal(line, &status)
-		if err != nil {
-			return i, fmt.Errorf("unmarshaling status from docker pull on image `%s` failed with: %s", i.image, err)
+
+		if err = json.Unmarshal(line, &status); err != nil {
+			return i, errorImagePullStatus(err)
 		}
 	}
 
@@ -125,18 +113,17 @@ func (i *Image) Instantiate(ctx context.Context, options ...ContainerOption) (*C
 	for _, opt := range options {
 		err := opt(c)
 		if err != nil {
-			return nil, fmt.Errorf("running image options for image `%s` failed with: %s", i.image, err)
+			return nil, errorContainerOptions(i.image, err)
 		}
 	}
 
-	var mounts []mount.Mount
-	_volume := mount.Mount{
-		Type: mount.TypeBind,
-	}
-	for _, volume := range c.volumes {
-		_volume.Source = volume.source
-		_volume.Target = volume.target
-		mounts = append(mounts, _volume)
+	mounts := make([]mount.Mount, len(c.volumes))
+	for idx, volume := range c.volumes {
+		mounts[idx] = mount.Mount{
+			Type:   mount.TypeBind,
+			Source: volume.source,
+			Target: volume.target,
+		}
 	}
 
 	config := &container.Config{
@@ -146,14 +133,13 @@ func (i *Image) Instantiate(ctx context.Context, options ...ContainerOption) (*C
 		Tty:   false,
 		Env:   c.env,
 	}
-
-	if len(c.workDir) != 0 {
+	if len(c.workDir) > 0 {
 		config.WorkingDir = c.workDir
 	}
 
 	resp, err := c.image.client.ContainerCreate(ctx, config, &container.HostConfig{Mounts: mounts}, nil, nil, "")
 	if err != nil {
-		return nil, fmt.Errorf("creating container for image `%s` failed with: %s", c.image.image, err)
+		return nil, errorContainerCreate(c.image.Name(), err)
 	}
 	c.id = resp.ID
 
@@ -162,21 +148,26 @@ func (i *Image) Instantiate(ctx context.Context, options ...ContainerOption) (*C
 
 // Clean removes all docker images that match the given filter, and max age.
 func (c *Client) Clean(ctx context.Context, age time.Duration, filter filters.Args) error {
-	images, _ := c.ImageList(context.Background(), types.ImageListOptions{Filters: filter})
+	images, _ := c.ImageList(ctx, types.ImageListOptions{Filters: filter})
 	timeNow := time.Now()
+
+	var err error
 	for _, image := range images {
 		if time.Unix(image.Created, 0).Add(age).Before(timeNow) {
-			_, err := c.ImageRemove(ctx, image.ID, types.ImageRemoveOptions{
+			if _, _err := c.ImageRemove(ctx, image.ID, types.ImageRemoveOptions{
 				Force:         true,
 				PruneChildren: true,
-			})
-			if err != nil {
-				return fmt.Errorf("cleaning image `%s` from docker host failed with %s", image.ID, err)
+			}); _err != nil {
+				if err != nil {
+					err = fmt.Errorf("%s:%w", err, _err)
+				} else {
+					err = _err
+				}
 			}
 		}
 	}
 
-	return nil
+	return err
 }
 
 // Name returns the name of the image

new.go

@@ -11,7 +11,7 @@ func New() (dockerClient *Client, err error) {
 	dockerClient = new(Client)
 	dockerClient.Client, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
 	if err != nil {
-		return nil, fmt.Errorf("creating new docker client failed with: %s", err)
+		return nil, fmt.Errorf("new docker client failed with: %w", err)
 	}
 
 	return