diff --git a/.travis.yml b/.travis.yml index 93c7b13..680ece6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ cache: - vendor before_install: - - go get -u github.com/golang/lint/golint + - go get -u golang.org/x/lint/golint install: - go get -t -u ./... diff --git a/README.md b/README.md index 848d739..ceea7fd 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ as a value that satisfies error. It also records the stack trace at the point it was called. ```go -func Wrap(err error, opts ...Annotator) error { +func Wrap(err error, annotators ...Annotator) error ``` Wrap returns an error annotated with a stack trace from the point it was called, diff --git a/error.go b/error.go index e0c9e33..09ab761 100644 --- a/error.go +++ b/error.go @@ -46,12 +46,11 @@ func Errorf(format string, args ...interface{}) error { return err } -// Error implements error interface +// Error implements error interface. +// It returns a string of messages and the root error concatenated with ": ". func (e *Error) Error() string { - if message := e.FullMessage(); message != "" { - return message - } - return e.Err.Error() + messages := append(e.Messages, e.Err.Error()) + return strings.Join(messages, messageDelimiter) } // Copy creates a copy of the current object @@ -75,9 +74,10 @@ func (e *Error) LastMessage() string { return e.Messages[0] } -// FullMessage returns a string of messages concatenated with ": " +// FullMessage is marked as deprecated in favor of `Error`. +// This method will be removed in the next major release. func (e *Error) FullMessage() string { - return strings.Join(e.Messages, messageDelimiter) + return e.Error() } // Wrap returns an error annotated with a stack trace from the point it was called, diff --git a/error_test.go b/error_test.go index 70a8941..11fd331 100644 --- a/error_test.go +++ b/error_test.go @@ -9,23 +9,21 @@ import ( ) func TestNew(t *testing.T) { - err := New("message") - assert.Equal(t, "message", err.Error()) + err := New("err") + assert.Equal(t, "err", err.Error()) appErr := Unwrap(err) - assert.Equal(t, err.Error(), appErr.Err.Error()) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "err", appErr.Error()) assert.NotEmpty(t, appErr.StackTrace) assert.Equal(t, "TestNew", appErr.StackTrace[0].Func) } func TestErrorf(t *testing.T) { - err := Errorf("message %d", 123) - assert.Equal(t, "message 123", err.Error()) + err := Errorf("err %d", 123) + assert.Equal(t, "err 123", err.Error()) appErr := Unwrap(err) - assert.Equal(t, err.Error(), appErr.Err.Error()) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "err 123", appErr.Error()) assert.NotEmpty(t, appErr.StackTrace) assert.Equal(t, "TestErrorf", appErr.StackTrace[0].Func) } @@ -43,7 +41,7 @@ func TestError_FullMessage(t *testing.T) { Err: errors.New("err"), Messages: []string{"message 2", "message 1"}, } - assert.Equal(t, "message 2: message 1", err.FullMessage()) + assert.Equal(t, err.Error(), err.FullMessage()) } func TestWithMessage(t *testing.T) { @@ -53,18 +51,18 @@ func TestWithMessage(t *testing.T) { }) t.Run("bare", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithMessage("message")) - assert.Equal(t, "message", err1.Error()) + assert.Equal(t, "message: origin", err1.Error()) appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, err1.Error(), appErr.FullMessage()) + assert.Equal(t, err1.Error(), appErr.Error()) }) t.Run("already wrapped", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := &Error{ Err: err0, @@ -72,19 +70,19 @@ func TestWithMessage(t *testing.T) { Code: 400, } err2 := Wrap(err1, WithMessage("message 2")) - assert.Equal(t, "message 2: message 1", err2.Error()) + assert.Equal(t, "message 2: message 1: origin", err2.Error()) { appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, err1.Error(), appErr.FullMessage()) + assert.Equal(t, err1.Error(), appErr.Error()) assert.Equal(t, 400, appErr.Code) } { appErr := Unwrap(err2) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, err2.Error(), appErr.FullMessage()) + assert.Equal(t, err2.Error(), appErr.Error()) assert.Equal(t, 400, appErr.Code) } }) @@ -97,17 +95,17 @@ func TestWithCode(t *testing.T) { }) t.Run("bare", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithCode(200)) appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "origin", appErr.Error()) }) t.Run("already wrapped", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := &Error{ Err: err0, @@ -119,14 +117,14 @@ func TestWithCode(t *testing.T) { { appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, err1.Error(), appErr.FullMessage()) + assert.Equal(t, "message 1: origin", appErr.Error()) assert.Equal(t, 400, appErr.Code) } { appErr := Unwrap(err2) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, err1.Error(), appErr.FullMessage()) + assert.Equal(t, "message 1: origin", appErr.Error()) assert.Equal(t, 500, appErr.Code) } }) @@ -139,7 +137,7 @@ func TestWithTags(t *testing.T) { }) t.Run("bare", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithTags("http", "notice_only")) @@ -149,7 +147,7 @@ func TestWithTags(t *testing.T) { }) t.Run("already wrapped", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithTags("http", "notice_only")) err2 := Wrap(err1, WithTags("security")) @@ -175,7 +173,7 @@ func TestWithParams(t *testing.T) { }) t.Run("bare", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithParams(H{"foo": 1, "bar": "baz"})) @@ -185,7 +183,7 @@ func TestWithParams(t *testing.T) { }) t.Run("short", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithParam("foo", 1)) @@ -195,7 +193,7 @@ func TestWithParams(t *testing.T) { }) t.Run("already wrapped", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithParams(H{"foo": 1, "bar": "baz"})) err2 := Wrap(err1, WithParams(H{"qux": true, "foo": "quux"})) @@ -221,17 +219,17 @@ func TestWithIgnorable(t *testing.T) { }) t.Run("bare", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithIgnorable()) appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "origin", appErr.Error()) }) t.Run("already wrapped", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := Wrap(err0, WithIgnorable()) err2 := Wrap(err1, WithIgnorable()) @@ -264,56 +262,56 @@ func TestWrap(t *testing.T) { }) t.Run("bare", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := wrapOrigin(err0) - assert.Equal(t, "original", err1.Error()) + assert.Equal(t, "origin", err1.Error()) appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "origin", appErr.Error()) assert.NotEmpty(t, appErr.StackTrace) assert.Equal(t, "wrapOrigin", appErr.StackTrace[0].Func) }) t.Run("already wrapped", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := wrapOrigin(err0) err2 := wrapOrigin(err1) - assert.Equal(t, "original", err2.Error()) + assert.Equal(t, "origin", err2.Error()) appErr := Unwrap(err2) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "origin", appErr.Error()) assert.NotEmpty(t, appErr.StackTrace) assert.Equal(t, "wrapOrigin", appErr.StackTrace[0].Func) }) t.Run("with pkg/errors", func(t *testing.T) { t.Run("pkg/errors.New", func(t *testing.T) { - err0 := pkgErrorsNew("original") + err0 := pkgErrorsNew("origin") err1 := wrapOrigin(err0) - assert.Equal(t, "original", err1.Error()) + assert.Equal(t, "origin", err1.Error()) appErr := Unwrap(err1) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, "", appErr.FullMessage()) + assert.Equal(t, "origin", appErr.Error()) assert.NotEmpty(t, appErr.StackTrace) assert.Equal(t, "pkgErrorsNew", appErr.StackTrace[0].Func) }) t.Run("pkg/errors.Wrap", func(t *testing.T) { - err0 := errors.New("original") + err0 := errors.New("origin") err1 := pkgErrorsWrap(err0, "message") err2 := wrapOrigin(err1) - assert.Equal(t, "message: original", err2.Error()) + assert.Equal(t, "message: origin", err2.Error()) appErr := Unwrap(err2) assert.Equal(t, err0, appErr.Err) - assert.Equal(t, "message: original", appErr.FullMessage()) + assert.Equal(t, "message: origin", appErr.Error()) assert.NotEmpty(t, appErr.StackTrace) assert.Equal(t, "pkgErrorsWrap", appErr.StackTrace[0].Func) }) @@ -323,7 +321,7 @@ func TestWrap(t *testing.T) { func TestAll(t *testing.T) { { appErr := Unwrap(errFunc3()) - assert.Equal(t, "e2: e1: e0", appErr.FullMessage()) + assert.Equal(t, "e2: e1: e0", appErr.Error()) assert.Equal(t, nil, appErr.Code) assert.Equal(t, false, appErr.Ignorable) assert.Equal(t, []string{ @@ -337,7 +335,7 @@ func TestAll(t *testing.T) { { appErr := Unwrap(errFunc4()) - assert.Equal(t, "e4: e2: e1: e0", appErr.FullMessage()) + assert.Equal(t, "e4: e2: e1: e0", appErr.Error()) assert.Equal(t, 500, appErr.Code) assert.Equal(t, true, appErr.Ignorable) assert.Equal(t, []string{ @@ -352,7 +350,7 @@ func TestAll(t *testing.T) { { appErr := Unwrap(errFunc4Goroutine()) - assert.Equal(t, "e4: e2: e1: e0", appErr.FullMessage()) + assert.Equal(t, "e4: e2: e1: e0", appErr.Error()) assert.Equal(t, 500, appErr.Code) assert.Equal(t, true, appErr.Ignorable) assert.Equal(t, []string{ diff --git a/pkgerrors.go b/pkgerrors.go index ef71ae4..0dfde2d 100644 --- a/pkgerrors.go +++ b/pkgerrors.go @@ -1,6 +1,8 @@ package fail import ( + "strings" + pkgerrors "github.com/pkg/errors" ) @@ -10,6 +12,10 @@ type pkgError struct { StackTrace StackTrace } +const ( + pkgErrorsMessageDelimiter = ": " +) + // extractPkgError extracts the innermost error from the given error. // It converts the stack trace that is annotated by pkg/errors into fail.StackTrace. // If the error doesn't have a stack trace or a causer of pkg/errors, @@ -39,8 +45,8 @@ func extractPkgError(err error) pkgError { } var msg string - if err.Error() != rootErr.Error() { - msg = err.Error() + if strings.HasSuffix(err.Error(), pkgErrorsMessageDelimiter+rootErr.Error()) { + msg = strings.TrimSuffix(err.Error(), pkgErrorsMessageDelimiter+rootErr.Error()) } return pkgError{ diff --git a/pkgerrors_test.go b/pkgerrors_test.go index ef908df..49c7af1 100644 --- a/pkgerrors_test.go +++ b/pkgerrors_test.go @@ -27,7 +27,7 @@ func TestExtractPkgError(t *testing.T) { pkgErr := extractPkgError(err1) assert.NotNil(t, pkgErr) - assert.Equal(t, "message: error", pkgErr.Message) + assert.Equal(t, "message", pkgErr.Message) assert.Equal(t, err0, pkgErr.Err) assert.NotEmpty(t, pkgErr.StackTrace) assert.Equal(t, "pkgErrorsWrap", pkgErr.StackTrace[0].Func)