From 1aa6c6f2c013d86b9574aba826c0860efb82dd39 Mon Sep 17 00:00:00 2001 From: William Perron Date: Wed, 8 Feb 2023 09:29:02 -0500 Subject: [PATCH 1/6] pull 'errors' package from go-npt --- errors/errors.go | 122 +++++++++++++++++++++++++++++++++++++++ errors/errors_test.go | 60 +++++++++++++++++++ errors/logfields.go | 34 +++++++++++ errors/logfields_test.go | 50 ++++++++++++++++ errors/stack.go | 71 +++++++++++++++++++++++ errors/stack_test.go | 54 +++++++++++++++++ errors/stdlib.go | 55 ++++++++++++++++++ 7 files changed, 446 insertions(+) create mode 100644 errors/errors.go create mode 100644 errors/errors_test.go create mode 100644 errors/logfields.go create mode 100644 errors/logfields_test.go create mode 100644 errors/stack.go create mode 100644 errors/stack_test.go create mode 100644 errors/stdlib.go diff --git a/errors/errors.go b/errors/errors.go new file mode 100644 index 0000000..b1891dc --- /dev/null +++ b/errors/errors.go @@ -0,0 +1,122 @@ +package errors + +import ( + "context" + "fmt" + "runtime" + + bugsnag_errors "github.com/bugsnag/bugsnag-go/v2/errors" + "github.com/sirupsen/logrus" +) + +// Fields can be attached to errors like this: +// errors.Wrap(err, "invalid value", errors.Fields{"value": value}) +type Fields map[string]interface{} + +type ErrorWithDetails interface { + ErrorDetails() map[string]interface{} +} + +type baseError struct { + err error + message string + fields Fields + stack []uintptr +} + +func (e *baseError) Error() string { + return e.message +} + +func (e *baseError) Unwrap() error { + return e.err +} + +func (e *baseError) Callers() []uintptr { + return findStackFromError(e) // lazy stack lookup in wrapped errors +} + +func (e *baseError) LogFields() logrus.Fields { + return logrus.Fields(e.fields) +} + +var _ bugsnag_errors.ErrorWithCallers = &baseError{} + +// Errorf formats according to a format specifier and returns the string +// as a value that satisfies error. +// Errorf also records the stack trace at the point it was called. +func Errorf(format string, args ...interface{}) error { + return &baseError{ + message: fmt.Sprintf(format, args...), + stack: captureStack(), + } +} + +// Wrap returns an error annotating err with a stack trace +// at the point Wrap is called, and the supplied message. +// If err is nil, Wrap returns nil. +func Wrap(err error, message string, fields ...Fields) error { + if err == nil { + return nil + } + + return &baseError{ + err: err, + message: fmt.Sprintf("%s: %s", message, err.Error()), + fields: mergeFields(nil, err, fields...), + stack: captureStack(), + } +} + +// WrapCtx returns an error annotating err with a stack trace and log fields. +// The log fields are captured from context.Context and arguments. +// If err is nil, WrapCtx returns nil. +func WrapCtx(ctx context.Context, err error, message string, fields ...Fields) error { + if err == nil { + return nil + } + + return &baseError{ + err: err, + message: message + ": " + err.Error(), + fields: mergeFields(ctx, err, fields...), + stack: captureStack(), + } +} + +// With returns an error annotating err with a stack trace and log fields. +// If err is nil, With returns nil. +func With(err error, fields ...Fields) error { + if err == nil { + return nil + } + + return &baseError{ + err: err, + message: err.Error(), + fields: mergeFields(nil, err, fields...), + stack: captureStack(), + } +} + +// WithCtx returns an error annotating err with a stack trace and log fields. +// The log fields are captured from context.Context and arguments. +// If err is nil, WithCtx returns nil. +func WithCtx(ctx context.Context, err error, fields ...Fields) error { + if err == nil { + return nil + } + + return &baseError{ + err: err, + message: err.Error(), + fields: mergeFields(ctx, err, fields...), + stack: captureStack(), + } +} + +func captureStack() []uintptr { + var pcs [32]uintptr + n := runtime.Callers(3, pcs[:]) // Wrap/WrapCtx -> findStack -> runtime.Callers + return pcs[0:n] +} diff --git a/errors/errors_test.go b/errors/errors_test.go new file mode 100644 index 0000000..400792b --- /dev/null +++ b/errors/errors_test.go @@ -0,0 +1,60 @@ +package errors + +import ( + "context" + stderrors "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNew(t *testing.T) { + err := New("MSG") + require.Equal(t, "MSG", err.Error()) +} + +func TestErrorf(t *testing.T) { + err := Errorf("MSG %s", "ME") + require.Equal(t, "MSG ME", err.Error()) +} + +func TestWrap_Nil(t *testing.T) { + err := Wrap(nil, "") + require.Nil(t, err) +} + +func TestWrap(t *testing.T) { + err := Wrap(stderrors.New("inner"), "outer") + require.NotNil(t, err) + require.Equal(t, "outer: inner", err.Error()) +} + +func TestWrapCtx_Nil(t *testing.T) { + ctx := context.Background() + err := WrapCtx(ctx, nil, "") + require.Nil(t, err) +} + +func TestWrapCtx(t *testing.T) { + ctx := context.Background() + err := WrapCtx(ctx, stderrors.New("inner"), "outer") + require.NotNil(t, err) + require.Equal(t, "outer: inner", err.Error()) +} + +func TestWithCtx(t *testing.T) { + ctx := context.Background() + err := WithCtx(ctx, stderrors.New("inner"), Fields{"key": "val"}) + require.NotNil(t, err) + require.Equal(t, "inner", err.Error()) + + require.Equal(t, Fields{"key": "val"}, FieldsFromError(err)) +} + +func TestWith(t *testing.T) { + err := With(stderrors.New("inner"), Fields{"key": "val"}) + require.NotNil(t, err) + require.Equal(t, "inner", err.Error()) + + require.Equal(t, Fields{"key": "val"}, FieldsFromError(err)) +} diff --git a/errors/logfields.go b/errors/logfields.go new file mode 100644 index 0000000..162715b --- /dev/null +++ b/errors/logfields.go @@ -0,0 +1,34 @@ +package errors + +import ( + "github.com/Shopify/goose/logger" + "github.com/pkg/errors" +) + +type LoggableError interface { + error + logger.Loggable +} + +func FieldsFromError(err error) Fields { + var loggable LoggableError + if errors.As(err, &loggable) { + return Fields(loggable.LogFields()) + } + + return Fields{} +} + +func mergeFields(ctx logger.Valuer, err error, fieldsList ...Fields) Fields { + // context > fields > parent error + fields := Fields{} + fieldsList = append([]Fields{FieldsFromError(err)}, fieldsList...) + fieldsList = append(fieldsList, Fields(logger.GetLoggableValues(ctx))) + + for _, fs := range fieldsList { + for k, v := range fs { + fields[k] = v + } + } + return fields +} diff --git a/errors/logfields_test.go b/errors/logfields_test.go new file mode 100644 index 0000000..4660019 --- /dev/null +++ b/errors/logfields_test.go @@ -0,0 +1,50 @@ +package errors + +import ( + "context" + "testing" + + "github.com/Shopify/goose/logger" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +func Test_FieldsFromError_NoFields(t *testing.T) { + require.Empty(t, FieldsFromError(nil)) + require.Empty(t, FieldsFromError(New(""))) +} + +type testError struct { + error + fields logrus.Fields +} + +func (e *testError) Unwrap() error { + return e.error +} + +func (e *testError) LogFields() logrus.Fields { + return e.fields +} + +func Test_FieldsFromError(t *testing.T) { + err1 := &testError{error: New("foo"), fields: logrus.Fields{"KEY1": "VAL1", "KEY2": "VAL2"}} + err2 := Wrap(err1, "bar", Fields{"KEY2": "VAL3", "KEY3": "VAL3"}) + + // Fields from outer error have precedence + require.Equal(t, Fields{"KEY1": "VAL1", "KEY2": "VAL3", "KEY3": "VAL3"}, FieldsFromError(err2)) +} + +func Test_FieldsFromError_From_Context(t *testing.T) { + originalErr := Wrap(New(""), "", Fields{"KEY1": "VAL1"}) + + ctx := context.Background() + ctx = logger.WithFields(ctx, logrus.Fields{"KEY1": "VAL1", "KEY2": "VAL2"}) + + err := WrapCtx(ctx, originalErr, "", Fields{"KEY2": "EXTRA", "EXTRA": "EXTRA"}) // KEY2 overlap + require.Equal(t, Fields{ + "KEY1": "VAL1", + "KEY2": "VAL2", // fields from inner error have precedence. + "EXTRA": "EXTRA", + }, FieldsFromError(err)) +} diff --git a/errors/stack.go b/errors/stack.go new file mode 100644 index 0000000..23526e6 --- /dev/null +++ b/errors/stack.go @@ -0,0 +1,71 @@ +package errors + +import ( + stderrors "errors" + "fmt" + "runtime" + "strings" + + pkgerrors "github.com/pkg/errors" +) + +// findStackFromError returns the deepest stacktrace found. Support Courier errors and pkg/errors. +func findStackFromError(err error) []uintptr { + if wrappedErr := stderrors.Unwrap(err); wrappedErr != nil { + stack := findStackFromError(wrappedErr) // recursion + if stack != nil { + return stack // an inner error has a stacktrace, escape the recursion + } + } + + // starting from the inner error, look for stacktrace + if stack := stackFromBaseError(err); stack != nil { + return stack + } + return stackFromPkgError(err) +} + +type pkgErrorWithStacktrace interface { + StackTrace() pkgerrors.StackTrace +} + +func stackFromPkgError(err error) []uintptr { + var errWithStacktrace pkgErrorWithStacktrace + if !stderrors.As(err, &errWithStacktrace) { + return nil + } + + stacktrace := errWithStacktrace.StackTrace() + callers := make([]uintptr, len(stacktrace)) + for i, pc := range stacktrace { + callers[i] = uintptr(pc) // de-alias from pkgerrors.Frame + } + return callers +} + +func stackFromBaseError(err error) []uintptr { + var errWithStacktrace *baseError + if !stderrors.As(err, &errWithStacktrace) { + return nil + } + + return errWithStacktrace.stack +} + +func formatStack(s []uintptr) string { // useful for debugging and writing tests. + var builder strings.Builder + + frames := runtime.CallersFrames(s) + for { + frame, more := frames.Next() + + builder.WriteString(fmt.Sprintf("%s\n\t%s:%d", frame.Function, frame.File, frame.Line)) + builder.WriteString("\n") + + if !more { + break + } + } + + return builder.String() +} diff --git a/errors/stack_test.go b/errors/stack_test.go new file mode 100644 index 0000000..8abfb1c --- /dev/null +++ b/errors/stack_test.go @@ -0,0 +1,54 @@ +package errors + +import ( + "testing" + + bugsnagerrors "github.com/bugsnag/bugsnag-go/v2/errors" + pkgErrors "github.com/pkg/errors" + "github.com/stretchr/testify/require" +) + +func rawStdError() error { + return New("") +} + +func nestedPkgError() error { + return pkgErrors.Wrap(New(""), "") +} + +func nestedBaseError() error { + return Wrap(New(""), "") +} + +func Test_baseError_Callers(t *testing.T) { + tests := []struct { + test string + wrappedErr func() error + stackLen int + }{ + { + test: "baseError-stdError", + wrappedErr: rawStdError, + stackLen: 3, // asm_amd64.s - testing.go - Wrap(tt.wrappedErr(), "") + }, + { + test: "baseError-baseError-stdError", + wrappedErr: nestedBaseError, + stackLen: 4, // asm_amd64.s - testing.go - Wrap(tt.wrappedErr(), "") - nestedBaseError + }, + { + test: "baseError-pkgError-stdError", + wrappedErr: nestedPkgError, + stackLen: 4, // asm_amd64.s - testing.go - Wrap(tt.wrappedErr(), "") - nestedPkgError + }, + } + + for _, tt := range tests { + t.Run(tt.test, func(t *testing.T) { + err := Wrap(tt.wrappedErr(), "") + + stack := err.(bugsnagerrors.ErrorWithCallers).Callers() //nolint:errorlint + require.Len(t, stack, tt.stackLen, formatStack(stack)) + }) + } +} diff --git a/errors/stdlib.go b/errors/stdlib.go new file mode 100644 index 0000000..dea77cc --- /dev/null +++ b/errors/stdlib.go @@ -0,0 +1,55 @@ +package errors + +import stderrors "errors" + +// New returns an error that formats as the given text. +// Each call to New returns a distinct error value even if the text is identical. +func New(text string) error { + return stderrors.New(text) +} + +// Is reports whether any error in err's chain matches target. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error is considered to match a target if it is equal to that target or if +// it implements a method Is(error) bool such that Is(target) returns true. +// +// An error type might provide an Is method so it can be treated as equivalent +// to an existing error. For example, if MyError defines +// +// func (m MyError) Is(target error) bool { return target == os.ErrExist } +// +// then Is(MyError{}, os.ErrExist) returns true. See syscall.Errno.Is for +// an example in the standard library. +func Is(err, target error) bool { + return stderrors.Is(err, target) +} + +// As finds the first error in err's chain that matches target, and if so, sets +// target to that error value and returns true. Otherwise, it returns false. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error matches target if the error's concrete value is assignable to the value +// pointed to by target, or if the error has a method As(interface{}) bool such that +// As(target) returns true. In the latter case, the As method is responsible for +// setting target. +// +// An error type might provide an As method so it can be treated as if it were a +// a different error type. +// +// As panics if target is not a non-nil pointer to either a type that implements +// error, or to any interface type. +func As(err error, target interface{}) bool { + return stderrors.As(err, target) +} + +// Unwrap returns the result of calling the Unwrap method on err, if err's +// type contains an Unwrap method returning error. +// Otherwise, Unwrap returns nil. +func Unwrap(err error) error { + return stderrors.Unwrap(err) +} From 6fcdea3afb8ca1d4cf88c9fbaebf48c0f33205c1 Mon Sep 17 00:00:00 2001 From: William Perron Date: Wed, 8 Feb 2023 10:09:58 -0500 Subject: [PATCH 2/6] update error stack to account for joined errors --- errors/logfields.go | 2 +- errors/logfields_test.go | 2 +- errors/stack.go | 9 ++++++++- errors/stack_test.go | 19 +++++++++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/errors/logfields.go b/errors/logfields.go index 162715b..418526c 100644 --- a/errors/logfields.go +++ b/errors/logfields.go @@ -1,7 +1,7 @@ package errors import ( - "github.com/Shopify/goose/logger" + "github.com/Shopify/goose/v2/logger" "github.com/pkg/errors" ) diff --git a/errors/logfields_test.go b/errors/logfields_test.go index 4660019..0d524a9 100644 --- a/errors/logfields_test.go +++ b/errors/logfields_test.go @@ -4,7 +4,7 @@ import ( "context" "testing" - "github.com/Shopify/goose/logger" + "github.com/Shopify/goose/v2/logger" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) diff --git a/errors/stack.go b/errors/stack.go index 23526e6..9b86309 100644 --- a/errors/stack.go +++ b/errors/stack.go @@ -11,7 +11,14 @@ import ( // findStackFromError returns the deepest stacktrace found. Support Courier errors and pkg/errors. func findStackFromError(err error) []uintptr { - if wrappedErr := stderrors.Unwrap(err); wrappedErr != nil { + if joined, ok := err.(interface{ Unwrap() []error }); ok { + for _, e := range joined.Unwrap() { + stack := findStackFromError(e) // recursion + if stack != nil { + return stack // an inner error has a stacktrace, escape the recursion + } + } + } else if wrappedErr := stderrors.Unwrap(err); wrappedErr != nil { stack := findStackFromError(wrappedErr) // recursion if stack != nil { return stack // an inner error has a stacktrace, escape the recursion diff --git a/errors/stack_test.go b/errors/stack_test.go index 8abfb1c..a6aa2bd 100644 --- a/errors/stack_test.go +++ b/errors/stack_test.go @@ -1,6 +1,7 @@ package errors import ( + "errors" "testing" bugsnagerrors "github.com/bugsnag/bugsnag-go/v2/errors" @@ -20,6 +21,14 @@ func nestedBaseError() error { return Wrap(New(""), "") } +func joinedError() error { + return errors.Join(New("first"), New("second")) +} + +func nestedJoinedError() error { + return Wrap(errors.Join(New("second"), New("third")), "first") +} + func Test_baseError_Callers(t *testing.T) { tests := []struct { test string @@ -41,6 +50,16 @@ func Test_baseError_Callers(t *testing.T) { wrappedErr: nestedPkgError, stackLen: 4, // asm_amd64.s - testing.go - Wrap(tt.wrappedErr(), "") - nestedPkgError }, + { + test: "baseError-stdJoinedError", + wrappedErr: joinedError, + stackLen: 3, + }, + { + test: "baseError-stdNestedJoinedError", + wrappedErr: nestedJoinedError, + stackLen: 4, + }, } for _, tt := range tests { From 7531a96ae3ab1c737f74cd41802c464945a800de Mon Sep 17 00:00:00 2001 From: William Perron Date: Wed, 8 Feb 2023 10:16:08 -0500 Subject: [PATCH 3/6] add failing test for stdlib wrapper --- errors/stack_test.go | 6 +++--- errors/stdlib_test.go | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 errors/stdlib_test.go diff --git a/errors/stack_test.go b/errors/stack_test.go index a6aa2bd..233b5c3 100644 --- a/errors/stack_test.go +++ b/errors/stack_test.go @@ -1,7 +1,7 @@ package errors import ( - "errors" + stderrors "errors" "testing" bugsnagerrors "github.com/bugsnag/bugsnag-go/v2/errors" @@ -22,11 +22,11 @@ func nestedBaseError() error { } func joinedError() error { - return errors.Join(New("first"), New("second")) + return stderrors.Join(New("first"), New("second")) } func nestedJoinedError() error { - return Wrap(errors.Join(New("second"), New("third")), "first") + return Wrap(stderrors.Join(New("second"), New("third")), "first") } func Test_baseError_Callers(t *testing.T) { diff --git a/errors/stdlib_test.go b/errors/stdlib_test.go new file mode 100644 index 0000000..2b0225b --- /dev/null +++ b/errors/stdlib_test.go @@ -0,0 +1,15 @@ +package errors + +import ( + stderrors "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestUnwrapJoinedErrors(t *testing.T) { + joined := stderrors.Join(New("first"), New("second")) + + unwrapped := Unwrap(joined) + assert.NotNil(t, unwrapped) +} From 7e543509a080e9f9c662aa584ad4c8b8c6dbb38d Mon Sep 17 00:00:00 2001 From: William Perron Date: Wed, 8 Feb 2023 11:54:05 -0500 Subject: [PATCH 4/6] fix LogFields from error with joined errors --- errors/errors.go | 8 ++++---- errors/logfields.go | 17 ++++++++++++++--- errors/logfields_test.go | 25 +++++++++++++++++++++++++ errors/stdlib.go | 1 + errors/stdlib_test.go | 2 +- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index b1891dc..98137af 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -63,7 +63,7 @@ func Wrap(err error, message string, fields ...Fields) error { return &baseError{ err: err, message: fmt.Sprintf("%s: %s", message, err.Error()), - fields: mergeFields(nil, err, fields...), + fields: mergeFieldsCtx(nil, err, fields...), stack: captureStack(), } } @@ -79,7 +79,7 @@ func WrapCtx(ctx context.Context, err error, message string, fields ...Fields) e return &baseError{ err: err, message: message + ": " + err.Error(), - fields: mergeFields(ctx, err, fields...), + fields: mergeFieldsCtx(ctx, err, fields...), stack: captureStack(), } } @@ -94,7 +94,7 @@ func With(err error, fields ...Fields) error { return &baseError{ err: err, message: err.Error(), - fields: mergeFields(nil, err, fields...), + fields: mergeFieldsCtx(nil, err, fields...), stack: captureStack(), } } @@ -110,7 +110,7 @@ func WithCtx(ctx context.Context, err error, fields ...Fields) error { return &baseError{ err: err, message: err.Error(), - fields: mergeFields(ctx, err, fields...), + fields: mergeFieldsCtx(ctx, err, fields...), stack: captureStack(), } } diff --git a/errors/logfields.go b/errors/logfields.go index 418526c..7b6be9f 100644 --- a/errors/logfields.go +++ b/errors/logfields.go @@ -12,6 +12,15 @@ type LoggableError interface { func FieldsFromError(err error) Fields { var loggable LoggableError + if joined, ok := err.(interface{ Unwrap() []error }); ok { + fs := []Fields{} + for _, e := range joined.Unwrap() { + if errors.As(e, &loggable) { + fs = append(fs, Fields(loggable.LogFields())) + } + } + return mergeFields(fs) + } if errors.As(err, &loggable) { return Fields(loggable.LogFields()) } @@ -19,12 +28,14 @@ func FieldsFromError(err error) Fields { return Fields{} } -func mergeFields(ctx logger.Valuer, err error, fieldsList ...Fields) Fields { - // context > fields > parent error - fields := Fields{} +func mergeFieldsCtx(ctx logger.Valuer, err error, fieldsList ...Fields) Fields { fieldsList = append([]Fields{FieldsFromError(err)}, fieldsList...) fieldsList = append(fieldsList, Fields(logger.GetLoggableValues(ctx))) + return mergeFields(fieldsList) +} +func mergeFields(fieldsList []Fields) Fields { + fields := Fields{} for _, fs := range fieldsList { for k, v := range fs { fields[k] = v diff --git a/errors/logfields_test.go b/errors/logfields_test.go index 0d524a9..52d7ea6 100644 --- a/errors/logfields_test.go +++ b/errors/logfields_test.go @@ -2,6 +2,7 @@ package errors import ( "context" + stderrors "errors" "testing" "github.com/Shopify/goose/v2/logger" @@ -48,3 +49,27 @@ func Test_FieldsFromError_From_Context(t *testing.T) { "EXTRA": "EXTRA", }, FieldsFromError(err)) } + +func Test_FieldsFromJoinedError(t *testing.T) { + err1 := Wrap(New(""), "", Fields{"FOO": "BAR"}) + err2 := stderrors.Join(Wrap(err1, "", Fields{"BAZ": "BOO"}), New("second")) + + extracted := FieldsFromError(err2) + require.Equal(t, Fields{"FOO": "BAR", "BAZ": "BOO"}, extracted) + + err3 := stderrors.Join(Wrap(err1, "", Fields{"BAZ": "BOO"}), Wrap(New(""), "", Fields{"FOO": "BAR"})) + + extracted = FieldsFromError(err3) + require.Equal(t, Fields{"FOO": "BAR", "BAZ": "BOO"}, extracted) + + err4 := stderrors.Join(Wrap(New(""), "", Fields{"FOO": "BAR"}), Wrap(New(""), "", Fields{"BAZ": "BOO"})) + + extracted = FieldsFromError(err4) + require.Equal(t, Fields{"FOO": "BAR", "BAZ": "BOO"}, extracted) + + err5 := stderrors.Join(Wrap(New(""), "", Fields{"FRUIT": "BANANA"}), New("")) + err6 := Wrap(stderrors.Join(Wrap(err5, "", Fields{"BAZ": "BOO"}), Wrap(New(""), "", Fields{"FOO": "BAR"})), "", Fields{"JOINED": "YES"}) + + extracted = FieldsFromError(err6) + require.Equal(t, Fields{"FOO": "BAR", "BAZ": "BOO", "JOINED": "YES", "FRUIT": "BANANA"}, extracted) +} diff --git a/errors/stdlib.go b/errors/stdlib.go index dea77cc..4965b8c 100644 --- a/errors/stdlib.go +++ b/errors/stdlib.go @@ -50,6 +50,7 @@ func As(err error, target interface{}) bool { // Unwrap returns the result of calling the Unwrap method on err, if err's // type contains an Unwrap method returning error. // Otherwise, Unwrap returns nil. +// Unwrap returns nil if the Unwrap method returns []error. func Unwrap(err error) error { return stderrors.Unwrap(err) } diff --git a/errors/stdlib_test.go b/errors/stdlib_test.go index 2b0225b..ebb7ca4 100644 --- a/errors/stdlib_test.go +++ b/errors/stdlib_test.go @@ -11,5 +11,5 @@ func TestUnwrapJoinedErrors(t *testing.T) { joined := stderrors.Join(New("first"), New("second")) unwrapped := Unwrap(joined) - assert.NotNil(t, unwrapped) + assert.Nil(t, unwrapped) } From 2d8d1b6f617d9e3033d1aaf4e02fa4dcaf2f7500 Mon Sep 17 00:00:00 2001 From: William Perron Date: Wed, 8 Feb 2023 14:09:57 -0500 Subject: [PATCH 5/6] add errors_test cases for errors.Join --- errors/errors_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/errors/errors_test.go b/errors/errors_test.go index 400792b..823c29b 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -2,6 +2,7 @@ package errors import ( "context" + "errors" stderrors "errors" "testing" @@ -29,6 +30,18 @@ func TestWrap(t *testing.T) { require.Equal(t, "outer: inner", err.Error()) } +func TestWrapJoined(t *testing.T) { + err := Wrap(stderrors.Join(New("inner 1"), New("inner 2")), "outer") + require.NotNil(t, err) + require.Equal(t, "outer: inner 1\ninner 2", err.Error()) +} + +func TestJoinWrapped(t *testing.T) { + err := errors.Join(New("first"), Wrap(New("inner"), "outer")) + require.NotNil(t, err) + require.Equal(t, "first\nouter: inner", err.Error()) +} + func TestWrapCtx_Nil(t *testing.T) { ctx := context.Background() err := WrapCtx(ctx, nil, "") @@ -42,6 +55,13 @@ func TestWrapCtx(t *testing.T) { require.Equal(t, "outer: inner", err.Error()) } +func TestWrapCtxJoined(t *testing.T) { + ctx := context.Background() + err := WrapCtx(ctx, stderrors.Join(New("first"), New("second")), "outer") + require.NotNil(t, err) + require.Equal(t, "outer: first\nsecond", err.Error()) +} + func TestWithCtx(t *testing.T) { ctx := context.Background() err := WithCtx(ctx, stderrors.New("inner"), Fields{"key": "val"}) @@ -51,6 +71,14 @@ func TestWithCtx(t *testing.T) { require.Equal(t, Fields{"key": "val"}, FieldsFromError(err)) } +func TestWithCtxJoined(t *testing.T) { + ctx := context.Background() + err := WithCtx(ctx, stderrors.Join(New("first"), New("second")), Fields{"key": "val"}) + require.NotNil(t, err) + require.Equal(t, "first\nsecond", err.Error()) + require.Equal(t, Fields{"key": "val"}, FieldsFromError(err)) +} + func TestWith(t *testing.T) { err := With(stderrors.New("inner"), Fields{"key": "val"}) require.NotNil(t, err) @@ -58,3 +86,10 @@ func TestWith(t *testing.T) { require.Equal(t, Fields{"key": "val"}, FieldsFromError(err)) } + +func TestWithJoined(t *testing.T) { + err := With(stderrors.Join(New("first"), New("second")), Fields{"key": "val"}) + require.NotNil(t, err) + require.Equal(t, "first\nsecond", err.Error()) + require.Equal(t, Fields{"key": "val"}, FieldsFromError(err)) +} From bd5b0668c3025401abffce6fd083c97ed1ae8c47 Mon Sep 17 00:00:00 2001 From: William Perron Date: Wed, 8 Feb 2023 14:15:36 -0500 Subject: [PATCH 6/6] fix lint --- errors/errors_test.go | 3 +-- errors/logfields.go | 3 ++- errors/logfields_test.go | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/errors/errors_test.go b/errors/errors_test.go index 823c29b..86ef320 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -2,7 +2,6 @@ package errors import ( "context" - "errors" stderrors "errors" "testing" @@ -37,7 +36,7 @@ func TestWrapJoined(t *testing.T) { } func TestJoinWrapped(t *testing.T) { - err := errors.Join(New("first"), Wrap(New("inner"), "outer")) + err := stderrors.Join(New("first"), Wrap(New("inner"), "outer")) require.NotNil(t, err) require.Equal(t, "first\nouter: inner", err.Error()) } diff --git a/errors/logfields.go b/errors/logfields.go index 7b6be9f..fac79c2 100644 --- a/errors/logfields.go +++ b/errors/logfields.go @@ -1,8 +1,9 @@ package errors import ( - "github.com/Shopify/goose/v2/logger" "github.com/pkg/errors" + + "github.com/Shopify/goose/v2/logger" ) type LoggableError interface { diff --git a/errors/logfields_test.go b/errors/logfields_test.go index 52d7ea6..74add83 100644 --- a/errors/logfields_test.go +++ b/errors/logfields_test.go @@ -5,9 +5,10 @@ import ( stderrors "errors" "testing" - "github.com/Shopify/goose/v2/logger" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" + + "github.com/Shopify/goose/v2/logger" ) func Test_FieldsFromError_NoFields(t *testing.T) {