Merge pull request #112 from dhartunian/introduce-multierror

support multi-cause errors in go 1.20

.github/workflows/ci.yaml

@@ -2,9 +2,9 @@ name: Go
 
 on:
   push:
-    branches: [ master ]
+    branches: [ master, go-1.20-upgrade ]
   pull_request:
-    branches: [ master ]
+    branches: [ master, go-1.20-upgrade ]
 
 jobs:
 
@@ -13,10 +13,8 @@ jobs:
     strategy:
       matrix:
         go:
-          - "1.17"
-          - "1.18"
-          - "1.19"
           - "1.20"
+          - "1.21"
     steps:
     - uses: actions/checkout@v2
 

errbase/adapters_test.go

@@ -64,7 +64,33 @@ func TestAdaptGoSingleWrapErr(t *testing.T) {
 	tt := testutils.T{T: t}
 	// The library preserves the cause. It's not possible to preserve the fmt
 	// string.
+	tt.CheckEqual(newErr.Error(), origErr.Error())
+	tt.CheckContains(newErr.Error(), "hello")
+}
+
+func TestAdaptBaseGoJoinErr(t *testing.T) {
+	origErr := goErr.Join(goErr.New("hello"), goErr.New("world"))
+	t.Logf("start err: %# v", pretty.Formatter(origErr))
+
+	newErr := network(t, origErr)
+
+	tt := testutils.T{T: t}
+	// The library preserves the error message.
+	tt.CheckEqual(newErr.Error(), origErr.Error())
+
+}
+
+func TestAdaptGoMultiWrapErr(t *testing.T) {
+	origErr := fmt.Errorf("an error %w and also %w", goErr.New("hello"), goErr.New("world"))
+	t.Logf("start err: %# v", pretty.Formatter(origErr))
+
+	newErr := network(t, origErr)
+
+	tt := testutils.T{T: t}
+	// The library preserves the causes. It's not possible to preserve the fmt string.
+	tt.CheckEqual(newErr.Error(), origErr.Error())
 	tt.CheckContains(newErr.Error(), "hello")
+	tt.CheckContains(newErr.Error(), "world")
 }
 
 func TestAdaptPkgWithMessage(t *testing.T) {

errbase/decode.go

@@ -66,6 +66,19 @@ func decodeLeaf(ctx context.Context, enc *errorspb.EncodedErrorLeaf) error {
 		}
 	}
 
+	if len(enc.MultierrorCauses) > 0 {
+		causes := make([]error, len(enc.MultierrorCauses))
+		for i, e := range enc.MultierrorCauses {
+			causes[i] = DecodeError(ctx, *e)
+		}
+		leaf := &opaqueLeafCauses{
+			causes: causes,
+		}
+		leaf.msg = enc.Message
+		leaf.details = enc.Details
+		return leaf
+	}
+
 	// No decoder and no error type: we'll keep what we received and
 	// make it ready to re-encode exactly (if the error leaves over the
 	// network again).

errbase/encode.go

@@ -33,18 +33,24 @@ func EncodeError(ctx context.Context, err error) EncodedError {
 	if cause := UnwrapOnce(err); cause != nil {
 		return encodeWrapper(ctx, err, cause)
 	}
-	// Not a causer.
-	return encodeLeaf(ctx, err)
+	return encodeLeaf(ctx, err, UnwrapMulti(err))
 }
 
-// encodeLeaf encodes a leaf error.
-func encodeLeaf(ctx context.Context, err error) EncodedError {
+// encodeLeaf encodes a leaf error. This function accepts a `causes`
+// argument because we encode multi-cause errors using the Leaf
+// protobuf. This was done to enable backwards compatibility when
+// introducing this functionality since the Wrapper type already has a
+// required single `cause` field.
+func encodeLeaf(ctx context.Context, err error, causes []error) EncodedError {
 	var msg string
 	var details errorspb.EncodedErrorDetails
 
 	if e, ok := err.(*opaqueLeaf); ok {
 		msg = e.msg
 		details = e.details
+	} else if e, ok := err.(*opaqueLeafCauses); ok {
+		msg = e.msg
+		details = e.details
 	} else {
 		details.OriginalTypeName, details.ErrorTypeMark.FamilyName, details.ErrorTypeMark.Extension = getTypeDetails(err, false /*onlyFamily*/)
 
@@ -74,11 +80,21 @@ func encodeLeaf(ctx context.Context, err error) EncodedError {
 		details.FullDetails = encodeAsAny(ctx, err, payload)
 	}
 
+	var cs []*EncodedError
+	if len(causes) > 0 {
+		cs = make([]*EncodedError, len(causes))
+		for i, ee := range causes {
+			ee := EncodeError(ctx, ee)
+			cs[i] = &ee
+		}
+	}
+
 	return EncodedError{
 		Error: &errorspb.EncodedError_Leaf{
 			Leaf: &errorspb.EncodedErrorLeaf{
-				Message: msg,
-				Details: details,
+				Message:          msg,
+				Details:          details,
+				MultierrorCauses: cs,
 			},
 		},
 	}
@@ -207,6 +223,8 @@ func getTypeDetails(
 	switch t := err.(type) {
 	case *opaqueLeaf:
 		return t.details.OriginalTypeName, t.details.ErrorTypeMark.FamilyName, t.details.ErrorTypeMark.Extension
+	case *opaqueLeafCauses:
+		return t.details.OriginalTypeName, t.details.ErrorTypeMark.FamilyName, t.details.ErrorTypeMark.Extension
 	case *opaqueWrapper:
 		return t.details.OriginalTypeName, t.details.ErrorTypeMark.FamilyName, t.details.ErrorTypeMark.Extension
 	}

errbase/opaque.go

@@ -30,11 +30,30 @@ type opaqueLeaf struct {
 	details errorspb.EncodedErrorDetails
 }
 
+// opaqueLeafCauses is used when receiving an unknown multi-cause
+// wrapper type. Its important property is that if it is communicated
+// back to some network system that _does_ know about the type, the
+// original object can be restored. We encode multi-cause errors as
+// leaf nodes over the network, in order to support backwards
+// compatibility with existing single-cause wrapper messages.
+//
+// This struct *must* be initialized with a non-nil causes value in
+// order to comply with go stdlib expectations for `Unwrap()`.
+type opaqueLeafCauses struct {
+	opaqueLeaf
+	causes []error
+}
+
 var _ error = (*opaqueLeaf)(nil)
 var _ SafeDetailer = (*opaqueLeaf)(nil)
 var _ fmt.Formatter = (*opaqueLeaf)(nil)
 var _ SafeFormatter = (*opaqueLeaf)(nil)
 
+var _ error = (*opaqueLeafCauses)(nil)
+var _ SafeDetailer = (*opaqueLeafCauses)(nil)
+var _ fmt.Formatter = (*opaqueLeafCauses)(nil)
+var _ SafeFormatter = (*opaqueLeafCauses)(nil)
+
 // opaqueWrapper is used when receiving an unknown wrapper type.
 // Its important property is that if it is communicated
 // back to some network system that _does_ know about
@@ -70,8 +89,12 @@ func (e *opaqueWrapper) Unwrap() error { return e.cause }
 func (e *opaqueLeaf) SafeDetails() []string    { return e.details.ReportablePayload }
 func (e *opaqueWrapper) SafeDetails() []string { return e.details.ReportablePayload }
 
-func (e *opaqueLeaf) Format(s fmt.State, verb rune)    { FormatError(e, s, verb) }
-func (e *opaqueWrapper) Format(s fmt.State, verb rune) { FormatError(e, s, verb) }
+func (e *opaqueLeaf) Format(s fmt.State, verb rune)       { FormatError(e, s, verb) }
+func (e *opaqueLeafCauses) Format(s fmt.State, verb rune) { FormatError(e, s, verb) }
+func (e *opaqueWrapper) Format(s fmt.State, verb rune)    { FormatError(e, s, verb) }
+
+// opaqueLeafCauses is a multi-cause wrapper
+func (e *opaqueLeafCauses) Unwrap() []error { return e.causes }
 
 func (e *opaqueLeaf) SafeFormatError(p Printer) (next error) {
 	p.Print(e.msg)

errbase/unwrap.go

@@ -27,6 +27,16 @@ package errbase
 // It supports both errors implementing causer (`Cause()` method, from
 // github.com/pkg/errors) and `Wrapper` (`Unwrap()` method, from the
 // Go 2 error proposal).
+//
+// UnwrapOnce treats multi-errors (those implementing the
+// `Unwrap() []error` interface as leaf-nodes since they cannot
+// reasonably be iterated through to a single cause. These errors
+// are typically constructed as a result of `fmt.Errorf` which results
+// in a `wrapErrors` instance that contains an interpolated error
+// string along with a list of causes.
+//
+// The go stdlib does not define output on `Unwrap()` for a multi-cause
+// error, so we default to nil here.
 func UnwrapOnce(err error) (cause error) {
 	switch e := err.(type) {
 	case interface{ Cause() error }:
@@ -39,6 +49,7 @@ func UnwrapOnce(err error) (cause error) {
 
 // UnwrapAll accesses the root cause object of the error.
 // If the error has no cause (leaf error), it is returned directly.
+// UnwrapAll treats multi-errors as leaf nodes.
 func UnwrapAll(err error) error {
 	for {
 		if cause := UnwrapOnce(err); cause != nil {
@@ -49,3 +60,12 @@ func UnwrapAll(err error) error {
 	}
 	return err
 }
+
+// UnwrapMulti access the slice of causes that an error contains, if it is a
+// multi-error.
+func UnwrapMulti(err error) []error {
+	if me, ok := err.(interface{ Unwrap() []error }); ok {
+		return me.Unwrap()
+	}
+	return nil
+}

errbase/unwrap_test.go

@@ -16,6 +16,7 @@ package errbase_test
 
 import (
 	"errors"
+	"fmt"
 	"testing"
 
 	"github.com/cockroachdb/errors/errbase"
@@ -58,6 +59,18 @@ func TestMixedErrorWrapping(t *testing.T) {
 	tt.CheckEqual(errbase.UnwrapAll(err3), err)
 }
 
+func TestMultiErrorUnwrap(t *testing.T) {
+	tt := testutils.T{T: t}
+
+	err := errors.New("hello")
+	err2 := pkgErr.WithMessage(err, "woo")
+	err3 := fmt.Errorf("%w %w", err, err2)
+
+	tt.CheckEqual(errbase.UnwrapOnce(err3), nil)
+	tt.CheckEqual(errbase.UnwrapAll(err3), err3)
+	tt.CheckDeepEqual(errbase.UnwrapMulti(err3), []error{err, err2})
+}
+
 type myWrapper struct{ cause error }
 
 func (w *myWrapper) Error() string { return w.cause.Error() }