Merge pull request #86 from rumpl/feat-multierror

Add multierror

Dockerfile

@@ -45,7 +45,7 @@ COPY --from=make-cli /api/bin/* .
 FROM scratch AS cross
 COPY --from=make-cross /api/bin/* .
 
-FROM make-protos as test
+FROM fs as test
 RUN make -f builder.Makefile test
 
 FROM fs AS lint

cli/cmd/context.go

@@ -33,12 +33,12 @@ import (
 	"os"
 	"text/tabwriter"
 
-	"github.com/hashicorp/go-multierror"
 	"github.com/spf13/cobra"
 
 	cliconfig "github.com/docker/api/cli/config"
 	cliopts "github.com/docker/api/cli/options"
 	"github.com/docker/api/context/store"
+	"github.com/docker/api/multierror"
 )
 
 // ContextCommand manages contexts

cli/cmd/rm.go

@@ -2,12 +2,12 @@ package cmd
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
 	"github.com/docker/api/client"
+	"github.com/docker/api/multierror"
 )
 
 type rmOpts struct {
@@ -23,26 +23,22 @@ func RmCommand() *cobra.Command {
 		Short:   "Remove containers",
 		Args:    cobra.MinimumNArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			var errs []string
 			c, err := client.New(cmd.Context())
 			if err != nil {
 				return errors.Wrap(err, "cannot connect to backend")
 			}
 
+			var errs *multierror.Error
 			for _, id := range args {
 				err := c.ContainerService().Delete(cmd.Context(), id, opts.force)
 				if err != nil {
-					errs = append(errs, err.Error()+" "+id)
+					errs = multierror.Append(errs, err)
 					continue
 				}
 				fmt.Println(id)
 			}
 
-			if len(errs) > 0 {
+			return errs.ErrorOrNil()
-				return errors.New(strings.Join(errs, "\n"))
-			}
-
-			return nil
 		},
 	}
 

moby/backend.go

@@ -7,10 +7,12 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/client"
+	"github.com/pkg/errors"
 
 	"github.com/docker/api/backend"
 	"github.com/docker/api/compose"
 	"github.com/docker/api/containers"
+	"github.com/docker/api/errdefs"
 )
 
 type mobyService struct {
@@ -127,7 +129,11 @@ func (ms *mobyService) Logs(ctx context.Context, containerName string, request c
 }
 
 func (ms *mobyService) Delete(ctx context.Context, containerID string, force bool) error {
-	return ms.apiClient.ContainerRemove(ctx, containerID, types.ContainerRemoveOptions{
+	err := ms.apiClient.ContainerRemove(ctx, containerID, types.ContainerRemoveOptions{
 		Force: force,
 	})
+	if client.IsErrNotFound(err) {
+		return errors.Wrapf(errdefs.ErrNotFound, "container %q", containerID)
+	}
+	return err
 }

multierror/multierror.go

@@ -0,0 +1,92 @@
+package multierror
+
+import (
+	"strings"
+
+	"github.com/hashicorp/go-multierror"
+)
+
+// Error wraps a multierror.Error and defines a default
+// formatting function that fits cli needs
+type Error struct {
+	err *multierror.Error
+}
+
+func (e *Error) Error() string {
+	if e == nil || e.err == nil {
+		return ""
+	}
+	e.err.ErrorFormat = listErrorFunc
+	return e.err.Error()
+}
+
+// WrappedErrors returns the list of errors that this Error is wrapping.
+// It is an implementation of the errwrap.Wrapper interface so that
+// multierror.Error can be used with that library.
+//
+// This method is not safe to be called concurrently and is no different
+// than accessing the Errors field directly. It is implemented only to
+// satisfy the errwrap.Wrapper interface.
+func (e *Error) WrappedErrors() []error {
+	return e.err.WrappedErrors()
+}
+
+// Unwrap returns an error from Error (or nil if there are no errors)
+func (e *Error) Unwrap() error {
+	if e == nil || e.err == nil {
+		return nil
+	}
+	return e.err.Unwrap()
+}
+
+// ErrorOrNil returns an error interface if this Error represents
+// a list of errors, or returns nil if the list of errors is empty. This
+// function is useful at the end of accumulation to make sure that the value
+// returned represents the existence of errors.
+func (e *Error) ErrorOrNil() error {
+	if e == nil || e.err == nil {
+		return nil
+	}
+	if len(e.err.Errors) == 0 {
+		return nil
+	}
+
+	return e
+}
+
+// Append adds an error to a multierror, if err is
+// not a multierror it will be converted to one
+func Append(err error, errs ...error) *Error {
+	switch err := err.(type) {
+	case *Error:
+		if err == nil {
+			err = new(Error)
+		}
+		for _, e := range errs {
+			err.err = multierror.Append(err.err, e)
+		}
+		return err
+	default:
+		newErrs := make([]error, 0, len(errs)+1)
+		if err != nil {
+			newErrs = append(newErrs, err)
+		}
+		newErrs = append(newErrs, errs...)
+
+		return Append(&Error{}, newErrs...)
+	}
+}
+
+func listErrorFunc(errs []error) string {
+	if len(errs) == 1 {
+		return errs[0].Error()
+	}
+
+	messages := make([]string, len(errs))
+
+	for i, err := range errs {
+		messages[i] = "Error: " + err.Error()
+	}
+
+	return strings.Join(messages, "\n")
+}

multierror/multierror_test.go

@@ -0,0 +1,48 @@
+package multierror
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSingleError(t *testing.T) {
+	var err *Error
+	err = Append(err, errors.New("error"))
+	assert.Equal(t, 1, len(err.WrappedErrors()))
+}
+
+func TestGoError(t *testing.T) {
+	var err error
+	result := Append(err, errors.New("error"))
+	assert.Equal(t, 1, len(result.WrappedErrors()))
+}
+
+func TestMultiError(t *testing.T) {
+	var err *Error
+	err = Append(err,
+		errors.New("first"),
+		errors.New("second"),
+	)
+	assert.Equal(t, 2, len(err.WrappedErrors()))
+	assert.Equal(t, "Error: first\nError: second", err.Error())
+}
+
+func TestUnwrap(t *testing.T) {
+	var err *Error
+	assert.Equal(t, nil, errors.Unwrap(err))
+
+	err = Append(err, errors.New("first"))
+	e := errors.Unwrap(err)
+	assert.Equal(t, "first", e.Error())
+}
+
+func TestErrorOrNil(t *testing.T) {
+	var err *Error
+	assert.Equal(t, nil, err.ErrorOrNil())
+
+	err = Append(err, errors.New("error"))
+	e := err.ErrorOrNil()
+	assert.Equal(t, "error", e.Error())
+}