From 0ecd556754232fcb372dd18fcdbbb6587836c851 Mon Sep 17 00:00:00 2001
From: Tiago Cesar Katcipis <tiagokatcipis@gmail.com>
Date: Mon, 29 Nov 2021 21:53:03 +0100
Subject: [PATCH] feat: filter out nil errors on errutil.Chain (#16)

---
 errutil/error.go      |  19 +++++---
 errutil/error_test.go | 108 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 105 insertions(+), 22 deletions(-)

diff --git a/errutil/error.go b/errutil/error.go
index c04871d..a7c315c 100644
--- a/errutil/error.go
+++ b/errutil/error.go
@@ -28,12 +28,12 @@ func (e Error) Error() string {
 //
 // An empty list of errors will return a nil error.
 func Chain(errs ...error) error {
-	// TODO(katcipis): should we do something when
-	// in the middle of the errs slice we have nils ?
-	// prone to filtering nils out, or they will break the chain anyway.
+	errs = removeNils(errs)
+
 	if len(errs) == 0 {
 		return nil
 	}
+
 	return errorChain{
 		head: errs[0],
 		tail: Chain(errs[1:]...),
@@ -47,9 +47,6 @@ type errorChain struct {
 
 // Error return a string representation of the chain of errors.
 func (e errorChain) Error() string {
-	if e.head == nil {
-		return ""
-	}
 	if e.tail == nil {
 		return e.head.Error()
 	}
@@ -67,3 +64,13 @@ func (e errorChain) Is(target error) bool {
 func (e errorChain) As(target interface{}) bool {
 	return errors.As(e.head, target)
 }
+
+func removeNils(errs []error) []error {
+	res := make([]error, 0, len(errs))
+	for _, err := range errs {
+		if err != nil {
+			res = append(res, err)
+		}
+	}
+	return res
+}
diff --git a/errutil/error_test.go b/errutil/error_test.go
index 68ec60e..64306db 100644
--- a/errutil/error_test.go
+++ b/errutil/error_test.go
@@ -36,29 +36,100 @@ func TestErrorRepresentation(t *testing.T) {
 }
 
 func TestErrorChain(t *testing.T) {
-	testcases := [][]error{
-		[]error{errors.New("single error")},
-		[]error{
-			errors.New("top error"),
-			errors.New("wrapped error 1"),
+	type testcase struct {
+		name string
+		errs []error
+		want []error
+	}
+
+	const (
+		sentinelErr  errutil.Error = "a sentinel error"
+		sentinel2Err errutil.Error = "another sentinel error"
+		sentinel3Err errutil.Error = "YASE"
+	)
+
+	testcases := []testcase{
+		{
+			name: "single error",
+			errs: []error{errors.New("single error")},
+		},
+		{
+			name: "two errors",
+			errs: []error{
+				errors.New("top error"),
+				errors.New("wrapped error 1"),
+			},
 		},
-		[]error{
-			errors.New("top error"),
-			errors.New("wrapped error 1"),
-			errors.New("wrapped error 2"),
+		{
+			name: "three errors",
+			errs: []error{
+				errors.New("top error"),
+				errors.New("wrapped error 1"),
+				errors.New("wrapped error 2"),
+			},
+		},
+		{
+			name: "errors is nil and err",
+			errs: []error{
+				nil,
+				sentinelErr,
+			},
+			want: []error{
+				sentinelErr,
+			},
+		},
+		{
+			name: "errors is err and nil",
+			errs: []error{
+				sentinelErr,
+				nil,
+			},
+			want: []error{
+				sentinelErr,
+			},
+		},
+		{
+			name: "errors is nil,err,nil",
+			errs: []error{
+				nil,
+				sentinelErr,
+				nil,
+			},
+			want: []error{
+				sentinelErr,
+			},
+		},
+		{
+			name: "errors interleaved with nils",
+			errs: []error{
+				sentinelErr,
+				nil,
+				sentinel2Err,
+				nil,
+				sentinel3Err,
+				nil,
+			},
+			want: []error{
+				sentinelErr,
+				sentinel2Err,
+				sentinel3Err,
+			},
 		},
 	}
 
-	for _, errs := range testcases {
-
-		name := fmt.Sprintf("%dErrors", len(errs))
-		t.Run(name, func(t *testing.T) {
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
 
-			err := errutil.Chain(errs...)
+			err := errutil.Chain(tc.errs...)
 			assert.Error(t, err)
 
 			got := err
-			for i, want := range errs {
+
+			if tc.want == nil {
+				tc.want = tc.errs
+			}
+
+			for i, want := range tc.want {
 				if got == nil {
 					t.Fatal("expected error to exist, got nil")
 				}
@@ -160,12 +231,17 @@ func TestErrorChainTypeSelection(t *testing.T) {
 	}
 }
 
-func TestErrorChainForEmptyErrList(t *testing.T) {
+func TestErrorChainForEmptyErrListIsNil(t *testing.T) {
 	assert.NoError(t, errutil.Chain())
 	errs := []error{}
 	assert.NoError(t, errutil.Chain(errs...))
 }
 
+func TestErrorChainWithOnlyNilErrorsIsNil(t *testing.T) {
+	assert.NoError(t, errutil.Chain(nil))
+	assert.NoError(t, errutil.Chain(nil, nil))
+}
+
 func TestErrorChainRespectIsMethodOfChainedErrors(t *testing.T) {
 	var neverIs errorThatNeverIs