Merge pull request #3 from mailgun/thrawn/find-all-fields

HasFields() now recursively finds all fields in the error chain

errors_test.go

@@ -24,7 +24,7 @@ func (e *ErrTest) Is(target error) bool {
 
 type ErrHasFields struct {
 	M string
-	F map[string]interface{}
+	F map[string]any
 }
 
 func (e *ErrHasFields) Error() string {
@@ -36,7 +36,7 @@ func (e *ErrHasFields) Is(target error) bool {
 	return ok
 }
 
-func (e *ErrHasFields) Fields() map[string]interface{} {
+func (e *ErrHasFields) HasFields() map[string]any {
 	return e.F
 }
 

fields.go

@@ -10,9 +10,11 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// HasFields Implement this interface to pass along unstructured context to the logger
+// HasFields Implement this interface to pass along unstructured context to the logger.
+// It is the responsibility of Fields() implementation to unwrap the error chain and
+// collect all errors that have `HasFields()` defined.
 type HasFields interface {
-	Fields() map[string]interface{}
+	HasFields() map[string]any
 }
 
 // HasFormat True if the interface has the format method (from fmt package)
@@ -21,12 +23,12 @@ type HasFormat interface {
 }
 
 // WithFields Creates errors that conform to the `HasFields` interface
-type WithFields map[string]interface{}
+type WithFields map[string]any
 
 // Wrapf returns an error annotating err with a stack trace
 // at the point Wrapf is call, and the format specifier.
 // If err is nil, Wrapf returns nil.
-func (f WithFields) Wrapf(err error, format string, args ...interface{}) error {
+func (f WithFields) Wrapf(err error, format string, args ...any) error {
 	if err == nil {
 		return nil
 	}
@@ -76,7 +78,7 @@ func (f WithFields) Error(msg string) error {
 	}
 }
 
-func (f WithFields) Errorf(format string, args ...interface{}) error {
+func (f WithFields) Errorf(format string, args ...any) error {
 	return &withFields{
 		stack:   callstack.New(1),
 		fields:  f,
@@ -115,20 +117,20 @@ func (c *withFields) StackTrace() callstack.StackTrace {
 	return c.stack.StackTrace()
 }
 
-func (c *withFields) Fields() map[string]interface{} {
-	result := make(map[string]interface{}, len(c.fields))
+func (c *withFields) HasFields() map[string]any {
+	result := make(map[string]any, len(c.fields))
 	for key, value := range c.fields {
 		result[key] = value
 	}
 
-	// downstream context values have precedence as they are closer to the cause
-	if child, ok := c.wrapped.(HasFields); ok {
-		downstream := child.Fields()
-		if downstream == nil {
+	// child fields have precedence as they are closer to the cause
+	var f HasFields
+	if errors.As(c.wrapped, &f) {
+		child := f.HasFields()
+		if child == nil {
 			return result
 		}
-
-		for key, value := range downstream {
+		for key, value := range child {
 			result[key] = value
 		}
 	}
@@ -167,10 +169,10 @@ func (c *withFields) FormatFields() string {
 	return buf.String()
 }
 
-// ToMap Returns the context for the underlying error as map[string]interface{}
-// If no context is available returns nil
-func ToMap(err error) map[string]interface{} {
-	result := map[string]interface{}{
+// ToMap Returns the fields for the underlying error as map[string]any
+// If no fields are available returns nil
+func ToMap(err error) map[string]any {
+	result := map[string]any{
 		"excValue": err.Error(),
 		"excType":  fmt.Sprintf("%T", Unwrap(err)),
 	}
@@ -185,9 +187,10 @@ func ToMap(err error) map[string]interface{} {
 		result["excFileName"] = caller.File
 	}
 
-	if child, ok := err.(HasFields); ok {
-		// Append the context map to our results
-		for key, value := range child.Fields() {
+	// Search the error chain for fields
+	var f HasFields
+	if errors.As(err, &f) {
+		for key, value := range f.HasFields() {
 			result[key] = value
 		}
 	}
@@ -214,15 +217,12 @@ func ToLogrus(err error) logrus.Fields {
 		result["excFileName"] = caller.File
 	}
 
-	// Add context if provided
-	child, ok := err.(HasFields)
-	if !ok {
-		return result
-	}
-
-	// Append the context map to our results
-	for key, value := range child.Fields() {
-		result[key] = value
+	// Search the error chain for fields
+	var f HasFields
+	if errors.As(err, &f) {
+		for key, value := range f.HasFields() {
+			result[key] = value
+		}
 	}
 	return result
 }

fields_test.go

@@ -14,6 +14,33 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestToMapToLogrusFindsLastStackTrace(t *testing.T) {
+	err := errors.New("this is an error")
+
+	// --- Should report this line number for the stack in the chain ---
+	err = errors.Wrap(err, "last")
+	// ----------------------------------
+
+	err = errors.Wrap(err, "second")
+	err = errors.Wrap(err, "first")
+
+	t.Run("ToMap() finds the last stack in the chain", func(t *testing.T) {
+		m := errors.ToMap(err)
+		assert.NotNil(t, m)
+		assert.Equal(t, 21, m["excLineNum"])
+	})
+
+	t.Run("ToLogrus() finds the last stack in the chain", func(t *testing.T) {
+		f := errors.ToLogrus(err)
+		require.NotNil(t, f)
+		b := bytes.Buffer{}
+		logrus.SetOutput(&b)
+		logrus.WithFields(f).Info("test logrus fields")
+		logrus.SetOutput(os.Stdout)
+		assert.Contains(t, b.String(), "excLineNum=21")
+	})
+}
+
 func TestWithFields(t *testing.T) {
 	err := &ErrTest{Msg: "query error"}
 	wrap := errors.WithFields{"key1": "value1"}.Wrap(err, "message")
@@ -78,7 +105,7 @@ func TestWithFields(t *testing.T) {
 	t.Run("ToLogrus() should extract the error with StackTrace() from the chain", func(t *testing.T) {
 		// This error has no  StackTrace() method
 		err := fmt.Errorf("I have no stack trace: %w", wrap)
-		// ToLogrus() should find the wrapped error in the chain that does have StackTrace() method.
+		// ToLogrus() should find the wrapped error in the chain that has the StackTrace() method.
 		f := errors.ToLogrus(err)
 		// t.Log(f)
 
@@ -87,7 +114,8 @@ func TestWithFields(t *testing.T) {
 		assert.Equal(t, "I have no stack trace: message: query error", f["excValue"])
 		assert.Equal(t, "errors_test.TestWithFields", f["excFuncName"])
 		assert.Equal(t, "*errors.withFields", f["excType"])
-		assert.Len(t, f, 5)
+		assert.Equal(t, "value1", f["key1"])
+		assert.Len(t, f, 6)
 
 		require.NotNil(t, f)
 	})
@@ -113,7 +141,9 @@ func TestErrorf(t *testing.T) {
 func TestNestedWithFields(t *testing.T) {
 	err := errors.New("this is an error")
 	err = errors.WithFields{"key1": "value1"}.Wrap(err, "message")
+	err = errors.Wrap(err, "second")
 	err = errors.WithFields{"key2": "value2"}.Wrap(err, "message")
+	err = errors.Wrap(err, "first")
 
 	t.Run("ToMap() collects all values from nested fields", func(t *testing.T) {
 		m := errors.ToMap(err)
@@ -135,30 +165,6 @@ func TestNestedWithFields(t *testing.T) {
 	})
 }
 
-func TestToMapToLogrusFindsLastStackTrace(t *testing.T) {
-	err := errors.New("this is an error")
-	// Should report this line number for the stack in the chain
-	err = errors.Wrap(err, "last")
-	err = errors.Wrap(err, "second")
-	err = errors.Wrap(err, "first")
-
-	t.Run("ToMap() finds the last stack in the chain", func(t *testing.T) {
-		m := errors.ToMap(err)
-		assert.NotNil(t, m)
-		assert.Equal(t, 141, m["excLineNum"])
-	})
-
-	t.Run("ToLogrus() finds the last stack in the chain", func(t *testing.T) {
-		f := errors.ToLogrus(err)
-		require.NotNil(t, f)
-		b := bytes.Buffer{}
-		logrus.SetOutput(&b)
-		logrus.WithFields(f).Info("test logrus fields")
-		logrus.SetOutput(os.Stdout)
-		assert.Contains(t, b.String(), "excLineNum=141")
-	})
-}
-
 func TestWithFieldsFmtDirectives(t *testing.T) {
 	t.Run("Wrap() with a message", func(t *testing.T) {
 		err := errors.WithFields{"key1": "value1"}.Wrap(errors.New("error"), "shit happened")
@@ -184,7 +190,7 @@ func TestWithFieldsErrorValue(t *testing.T) {
 }
 
 func TestHasFields(t *testing.T) {
-	hf := &ErrHasFields{M: "error", F: map[string]interface{}{"file": "errors.go"}}
+	hf := &ErrHasFields{M: "error", F: map[string]any{"file": "errors.go"}}
 	err := errors.WithFields{"key1": "value1"}.Wrap(hf, "")
 	m := errors.ToMap(err)
 	require.NotNil(t, m)

go.mod

@@ -3,11 +3,16 @@ module github.com/mailgun/errors
 go 1.19
 
 require (
-	github.com/ahmetb/go-linq v3.0.0+incompatible // indirect
+	github.com/ahmetb/go-linq v3.0.0+incompatible
+	github.com/mailgun/holster/v4 v4.11.0
+	github.com/sirupsen/logrus v1.9.0
+	github.com/stretchr/testify v1.8.1
+)
+
+require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	github.com/sirupsen/logrus v1.9.0 // indirect
-	github.com/stretchr/testify v1.8.1 // indirect
-	golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect
+	golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -3,6 +3,10 @@ github.com/ahmetb/go-linq v3.0.0+incompatible/go.mod h1:PFffvbdbtw+QTB0WKRP0cNht
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/mailgun/holster/v4 v4.11.0 h1:c9DT3QZCfiUgcmOYvxJI9rRhLbtX6IqfaClrMjLxkLE=
+github.com/mailgun/holster/v4 v4.11.0/go.mod h1:sXpF+rzEqA89uBEiX19FrVUdbCUKDfR4GRdXmIkINQQ=
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/sirupsen/logrus v1.9.0 h1:trlNQbNUG3OdDrDil03MCb1H2o9nJ1x4/5LYw7byDE0=
@@ -15,8 +19,10 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
-golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JCXC4ykBgQG4Fe06QRhQ=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 h1:h+EGohizhe9XlX18rfpa8k8RAc5XyaeamM+0VHRd4lc=
+golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=

stack.go

@@ -1,6 +1,7 @@
 package errors
 
 import (
+	"errors"
 	"fmt"
 	"io"
 
@@ -31,10 +32,16 @@ func (w *withStack) Is(target error) bool {
 	return ok
 }
 
-func (w *withStack) Fields() map[string]interface{} {
+func (w *withStack) HasFields() map[string]any {
 	if child, ok := w.error.(HasFields); ok {
-		return child.Fields()
+		return child.HasFields()
 	}
+
+	var f HasFields
+	if errors.As(w.error, &f) {
+		return f.HasFields()
+	}
+
 	return nil
 }