Skip to content

Commit

Permalink
Respect TypeModifierOmitStackTrace in Builder
Browse files Browse the repository at this point in the history
Previous implementation of `WithCause` method didn't respect the
`TypeModifierOmitStackTrace` modifier. A stack trace was erroneously
collected if the error that was passed to `WithCause` didn't have
stack trace.

I had to upgrade `github.com/stretchr/testify` to a slightly more modern
version to be able to use `Same` assertion.
  • Loading branch information
g7r committed Nov 8, 2024
1 parent d819af5 commit b1898dc
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 24 deletions.
31 changes: 19 additions & 12 deletions builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,17 @@ func NewErrorBuilder(t *Type) ErrorBuilder {
}

// WithCause provides an original cause for error.
// For non-errorx errors, a stack trace is collected.
// For non-errorx errors, a stack trace is collected unless Type tells otherwise.
// Otherwise, it is inherited by default, as error wrapping is typically performed 'en passe'.
// Note that even if an original error explicitly omitted the stack trace, it could be added on wrap.
func (eb ErrorBuilder) WithCause(err error) ErrorBuilder {
eb.cause = err
if Cast(err) != nil {
eb.mode = stackTraceBorrow
if eb.errorType.modifiers.CollectStackTrace() {
eb.mode = stackTraceBorrowOrCollect
} else {
eb.mode = stackTraceBorrowOnly
}
}

return eb
Expand Down Expand Up @@ -105,18 +109,25 @@ func (eb ErrorBuilder) Create() *Error {
type callStackBuildMode int

const (
stackTraceCollect callStackBuildMode = 1
stackTraceBorrow callStackBuildMode = 2
stackTraceEnhance callStackBuildMode = 3
stackTraceOmit callStackBuildMode = 4
stackTraceCollect callStackBuildMode = 1
stackTraceBorrowOrCollect callStackBuildMode = 2
stackTraceBorrowOnly callStackBuildMode = 3
stackTraceEnhance callStackBuildMode = 4
stackTraceOmit callStackBuildMode = 5
)

func (eb ErrorBuilder) assembleStackTrace() *stackTrace {
switch eb.mode {
case stackTraceCollect:
return eb.collectOriginalStackTrace()
case stackTraceBorrow:
case stackTraceBorrowOnly:
return eb.borrowStackTraceFromCause()
case stackTraceBorrowOrCollect:
if st := eb.borrowStackTraceFromCause(); st != nil {
return st
}

return eb.collectOriginalStackTrace()
case stackTraceEnhance:
return eb.combineStackTraceWithCause()
case stackTraceOmit:
Expand All @@ -131,11 +142,7 @@ func (eb ErrorBuilder) collectOriginalStackTrace() *stackTrace {
}

func (eb ErrorBuilder) borrowStackTraceFromCause() *stackTrace {
originalStackTrace := eb.extractStackTraceFromCause(eb.cause)
if originalStackTrace != nil {
return originalStackTrace
}
return collectStackTrace()
return eb.extractStackTraceFromCause(eb.cause)
}

func (eb ErrorBuilder) combineStackTraceWithCause() *stackTrace {
Expand Down
36 changes: 36 additions & 0 deletions builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package errorx

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -20,3 +21,38 @@ func TestBuilderTransparency(t *testing.T) {
require.NotEqual(t, testType, err.Type())
})
}

func testBuilderRespectsNoStackTraceMarkerFrame() error {
return testType.NewWithNoMessage()
}

func TestBuilderRespectsNoStackTrace(t *testing.T) {
wrapperErrorTypes := []*Type{testTypeSilent, testTypeSilentTransparent}

for _, et := range wrapperErrorTypes {
t.Run(et.String(), func(t *testing.T) {
t.Run("Naked", func(t *testing.T) {
err := NewErrorBuilder(et).
WithCause(errors.New("naked error")).
Create()
require.Nil(t, err.stackTrace)
})

t.Run("WithoutStacktrace", func(t *testing.T) {
err := NewErrorBuilder(et).
WithCause(testTypeSilent.NewWithNoMessage()).
Create()
require.Nil(t, err.stackTrace)
})

t.Run("WithStacktrace", func(t *testing.T) {
cause := testBuilderRespectsNoStackTraceMarkerFrame()
err := NewErrorBuilder(et).
WithCause(cause).
Create()
require.Same(t, err.stackTrace, Cast(cause).stackTrace)
require.Contains(t, fmt.Sprintf("%+v", err), "testBuilderRespectsNoStackTraceMarkerFrame")
})
})
}
}
17 changes: 9 additions & 8 deletions error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ import (
)

var (
testNamespace = NewNamespace("foo")
testType = testNamespace.NewType("bar")
testTypeSilent = testType.NewSubtype("silent").ApplyModifiers(TypeModifierOmitStackTrace)
testTypeTransparent = testType.NewSubtype("transparent").ApplyModifiers(TypeModifierTransparent)
testSubtype0 = testType.NewSubtype("internal")
testSubtype1 = testSubtype0.NewSubtype("wat")
testTypeBar1 = testNamespace.NewType("bar1")
testTypeBar2 = testNamespace.NewType("bar2")
testNamespace = NewNamespace("foo")
testType = testNamespace.NewType("bar")
testTypeSilent = testType.NewSubtype("silent").ApplyModifiers(TypeModifierOmitStackTrace)
testTypeTransparent = testType.NewSubtype("transparent").ApplyModifiers(TypeModifierTransparent)
testTypeSilentTransparent = testType.NewSubtype("silent_transparent").ApplyModifiers(TypeModifierTransparent, TypeModifierOmitStackTrace)
testSubtype0 = testType.NewSubtype("internal")
testSubtype1 = testSubtype0.NewSubtype("wat")
testTypeBar1 = testNamespace.NewType("bar1")
testTypeBar2 = testNamespace.NewType("bar2")
)

func TestError(t *testing.T) {
Expand Down
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,5 @@ go 1.11

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/stretchr/testify v1.2.2
github.com/stretchr/testify v1.4.0
)
10 changes: 8 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
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/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
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.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

0 comments on commit b1898dc

Please sign in to comment.