Skip to content

Commit 3280086

Browse files
committed
Respect TypeModifierOmitStackTrace in Builder
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.
1 parent d819af5 commit 3280086

File tree

5 files changed

+73
-24
lines changed

5 files changed

+73
-24
lines changed

builder.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,17 @@ func NewErrorBuilder(t *Type) ErrorBuilder {
3333
}
3434

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

4549
return eb
@@ -105,18 +109,25 @@ func (eb ErrorBuilder) Create() *Error {
105109
type callStackBuildMode int
106110

107111
const (
108-
stackTraceCollect callStackBuildMode = 1
109-
stackTraceBorrow callStackBuildMode = 2
110-
stackTraceEnhance callStackBuildMode = 3
111-
stackTraceOmit callStackBuildMode = 4
112+
stackTraceCollect callStackBuildMode = 1
113+
stackTraceBorrowOrCollect callStackBuildMode = 2
114+
stackTraceBorrowOnly callStackBuildMode = 3
115+
stackTraceEnhance callStackBuildMode = 4
116+
stackTraceOmit callStackBuildMode = 5
112117
)
113118

114119
func (eb ErrorBuilder) assembleStackTrace() *stackTrace {
115120
switch eb.mode {
116121
case stackTraceCollect:
117122
return eb.collectOriginalStackTrace()
118-
case stackTraceBorrow:
123+
case stackTraceBorrowOnly:
119124
return eb.borrowStackTraceFromCause()
125+
case stackTraceBorrowOrCollect:
126+
if st := eb.borrowStackTraceFromCause(); st != nil {
127+
return st
128+
}
129+
130+
return eb.collectOriginalStackTrace()
120131
case stackTraceEnhance:
121132
return eb.combineStackTraceWithCause()
122133
case stackTraceOmit:
@@ -131,11 +142,7 @@ func (eb ErrorBuilder) collectOriginalStackTrace() *stackTrace {
131142
}
132143

133144
func (eb ErrorBuilder) borrowStackTraceFromCause() *stackTrace {
134-
originalStackTrace := eb.extractStackTraceFromCause(eb.cause)
135-
if originalStackTrace != nil {
136-
return originalStackTrace
137-
}
138-
return collectStackTrace()
145+
return eb.extractStackTraceFromCause(eb.cause)
139146
}
140147

141148
func (eb ErrorBuilder) combineStackTraceWithCause() *stackTrace {

builder_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package errorx
22

33
import (
44
"errors"
5+
"fmt"
56
"testing"
67

78
"github.com/stretchr/testify/require"
@@ -20,3 +21,38 @@ func TestBuilderTransparency(t *testing.T) {
2021
require.NotEqual(t, testType, err.Type())
2122
})
2223
}
24+
25+
func testBuilderRespectsNoStackTraceMarkerFrame() error {
26+
return testType.NewWithNoMessage()
27+
}
28+
29+
func TestBuilderRespectsNoStackTrace(t *testing.T) {
30+
wrapperErrorTypes := []*Type{testTypeSilent, testTypeSilentTransparent}
31+
32+
for _, et := range wrapperErrorTypes {
33+
t.Run(et.String(), func(t *testing.T) {
34+
t.Run("Naked", func(t *testing.T) {
35+
err := NewErrorBuilder(et).
36+
WithCause(errors.New("naked error")).
37+
Create()
38+
require.Nil(t, err.stackTrace)
39+
})
40+
41+
t.Run("WithoutStacktrace", func(t *testing.T) {
42+
err := NewErrorBuilder(et).
43+
WithCause(testTypeSilent.NewWithNoMessage()).
44+
Create()
45+
require.Nil(t, err.stackTrace)
46+
})
47+
48+
t.Run("WithStacktrace", func(t *testing.T) {
49+
cause := testBuilderRespectsNoStackTraceMarkerFrame()
50+
err := NewErrorBuilder(et).
51+
WithCause(cause).
52+
Create()
53+
require.Same(t, err.stackTrace, Cast(cause).stackTrace)
54+
require.Contains(t, fmt.Sprintf("%+v", err), "testBuilderRespectsNoStackTraceMarkerFrame")
55+
})
56+
})
57+
}
58+
}

error_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@ import (
99
)
1010

1111
var (
12-
testNamespace = NewNamespace("foo")
13-
testType = testNamespace.NewType("bar")
14-
testTypeSilent = testType.NewSubtype("silent").ApplyModifiers(TypeModifierOmitStackTrace)
15-
testTypeTransparent = testType.NewSubtype("transparent").ApplyModifiers(TypeModifierTransparent)
16-
testSubtype0 = testType.NewSubtype("internal")
17-
testSubtype1 = testSubtype0.NewSubtype("wat")
18-
testTypeBar1 = testNamespace.NewType("bar1")
19-
testTypeBar2 = testNamespace.NewType("bar2")
12+
testNamespace = NewNamespace("foo")
13+
testType = testNamespace.NewType("bar")
14+
testTypeSilent = testType.NewSubtype("silent").ApplyModifiers(TypeModifierOmitStackTrace)
15+
testTypeTransparent = testType.NewSubtype("transparent").ApplyModifiers(TypeModifierTransparent)
16+
testTypeSilentTransparent = testType.NewSubtype("silent_transparent").ApplyModifiers(TypeModifierTransparent, TypeModifierOmitStackTrace)
17+
testSubtype0 = testType.NewSubtype("internal")
18+
testSubtype1 = testSubtype0.NewSubtype("wat")
19+
testTypeBar1 = testNamespace.NewType("bar1")
20+
testTypeBar2 = testNamespace.NewType("bar2")
2021
)
2122

2223
func TestError(t *testing.T) {

go.mod

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,5 @@ go 1.11
44

55
require (
66
github.com/davecgh/go-spew v1.1.1 // indirect
7-
github.com/pmezard/go-difflib v1.0.0 // indirect
8-
github.com/stretchr/testify v1.2.2
7+
github.com/stretchr/testify v1.4.0
98
)

go.sum

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1+
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
12
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
23
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
34
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
45
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
5-
github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
6-
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
6+
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
7+
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
8+
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
9+
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
10+
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
11+
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
12+
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

0 commit comments

Comments
 (0)