From b1898dc31ad9d48d7610b072b3d1fd63dacfdc8c Mon Sep 17 00:00:00 2001 From: Sergey Zagursky Date: Thu, 7 Nov 2024 21:45:18 +0000 Subject: [PATCH] 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. --- builder.go | 31 +++++++++++++++++++------------ builder_test.go | 36 ++++++++++++++++++++++++++++++++++++ error_test.go | 17 +++++++++-------- go.mod | 3 +-- go.sum | 10 ++++++++-- 5 files changed, 73 insertions(+), 24 deletions(-) diff --git a/builder.go b/builder.go index 1cc7a25..ec29429 100644 --- a/builder.go +++ b/builder.go @@ -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 @@ -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: @@ -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 { diff --git a/builder_test.go b/builder_test.go index d623bd3..ef49064 100644 --- a/builder_test.go +++ b/builder_test.go @@ -2,6 +2,7 @@ package errorx import ( "errors" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -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") + }) + }) + } +} diff --git a/error_test.go b/error_test.go index a8f6262..406dc23 100644 --- a/error_test.go +++ b/error_test.go @@ -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) { diff --git a/go.mod b/go.mod index d5bef06..0fa1864 100644 --- a/go.mod +++ b/go.mod @@ -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 ) diff --git a/go.sum b/go.sum index e03ee77..300e03c 100644 --- a/go.sum +++ b/go.sum @@ -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=