diff --git a/errors_test.go b/errors_test.go index d21426b..5d1bbe1 100644 --- a/errors_test.go +++ b/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 } diff --git a/fields.go b/fields.go index e5eb691..299c76e 100644 --- a/fields.go +++ b/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 } diff --git a/fields_test.go b/fields_test.go index 7d361f6..92912ed 100644 --- a/fields_test.go +++ b/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) diff --git a/go.mod b/go.mod index 149a34f..17301ba 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index 81700b2..d4ff782 100644 --- a/go.sum +++ b/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= diff --git a/stack.go b/stack.go index 0df5301..442054e 100644 --- a/stack.go +++ b/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 }