From dbbe32c482126ae51b26b4a5204e9a40117c6a42 Mon Sep 17 00:00:00 2001 From: Daria Bialobrzeska Date: Thu, 22 Feb 2024 17:12:50 +0100 Subject: [PATCH 1/2] Add unit tests for different error types' stacks --- v2/errors/error.go | 4 + v2/errors/error_test.go | 22 ++++-- v2/errors/error_types_test.go | 143 ++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 8 deletions(-) create mode 100644 v2/errors/error_types_test.go diff --git a/v2/errors/error.go b/v2/errors/error.go index a878673..cbe0e98 100644 --- a/v2/errors/error.go +++ b/v2/errors/error.go @@ -65,6 +65,10 @@ func New(e interface{}, skip int) *Error { trace := e.StackTrace() stack := make([]uintptr, len(trace)) for i, ptr := range trace { + // We do not modify the uintptr representation of the stack frame + // stack is processed by runtime.CallersFrames and then by Next() on Frames slice + // it's already doing uintptr-1 + // refer to: https://github.com/golang/go/blob/897b3da2e079b9b940b309747305a5379fffa6ec/src/runtime/symtab.go#L108 stack[i] = uintptr(ptr) } return &Error{ diff --git a/v2/errors/error_test.go b/v2/errors/error_test.go index fcf2d3d..423fba7 100644 --- a/v2/errors/error_test.go +++ b/v2/errors/error_test.go @@ -291,23 +291,29 @@ func TestUnwrapCustomCause(t *testing.T) { } } -func ExampleErrorf() { +func TestExampleErrorf(t *testing.T) { + errorStr := "" for i := 1; i <= 2; i++ { if i%2 == 1 { e := Errorf("can only halve even numbers, got %d", i) - fmt.Printf("Error: %+v", e) + errorStr += fmt.Sprintf("Error: %+v", e) } } - // Output: - // Error: can only halve even numbers, got 1 + + expected := "Error: can only halve even numbers, got 1" + if expected != errorStr { + t.Errorf("Actual error does not match expected") + } } -func ExampleNew() { +func TestExampleNew(t *testing.T) { // Wrap io.EOF with the current stack-trace and return it e := New(io.EOF, 0) - fmt.Printf("%+v", e) - // Output: - // EOF + errorStr := fmt.Sprintf("%+v", e) + expected := "EOF" + if expected != errorStr { + t.Errorf("Actual error does not match expected") + } } func ExampleNew_skip() { diff --git a/v2/errors/error_types_test.go b/v2/errors/error_types_test.go new file mode 100644 index 0000000..ee1cd63 --- /dev/null +++ b/v2/errors/error_types_test.go @@ -0,0 +1,143 @@ +package errors + +import ( + "fmt" + "testing" + + pkgerror "github.com/pkg/errors" +) + +const ( + bugsnagType = "bugsnagError" + callersType = "callersType" + stackFramesType = "stackFramesType" + stackType = "stackType" + internalType = "internalType" + stringType = "stringType" +) + +// Prepared to test inlining +func AInternal() interface{} { return fmt.Errorf("pure golang error") } +func BInternal() interface{} { return AInternal() } +func CInternal() interface{} { return BInternal() } + +func AString() interface{} { defer func() interface{} { return recover() }(); panic("panic") } +func BString() interface{} { return AString() } +func CString() interface{} { return BString() } + +func AStack() interface{} { return pkgerror.Errorf("from package") } +func BStack() interface{} { return AStack() } +func CStack() interface{} { return BStack() } + +func ACallers() interface{} { return newCustomErr("oh no an error", fmt.Errorf("parent error")) } +func BCallers() interface{} { return ACallers() } +func CCallers() interface{} { return BCallers() } + +func AFrames() interface{} { return &testErrorWithStackFrames{Err: New("foo", 0)} } +func BFrames() interface{} { return AFrames() } +func CFrames() interface{} { return BFrames() } + +// Golang internal errors don't have stacktrace +// StackFrames are going to report only the line where internal golang error was wrapped in Bugsnag error +func TestInternalError(t *testing.T) { + err := CInternal() + typeAssert(t, err, internalType) + + bgError := New(err, 0) + actualStack := bgError.StackFrames() + expected := []StackFrame{ + {Name: "TestInternalError", File: "errors/error_types_test.go", LineNumber: 46}, + } + assertStacksMatch(t, expected, actualStack) +} + +// Errors from panic contain only the message about panic +// Same as above - StackFrames are going to contain only line numer of wrapping +func TestStringError(t *testing.T) { + err := CString() + typeAssert(t, err, stringType) + + bgError := New(err, 0) + actualStack := bgError.StackFrames() + expected := []StackFrame{ + {Name: "TestStringError", File: "errors/error_types_test.go", LineNumber: 60}, + } + assertStacksMatch(t, expected, actualStack) +} + +// Errors from pkg/errors have their own stack +// Inlined functions should be visible in StackFrames +func TestStackError(t *testing.T) { + err := CStack() + typeAssert(t, err, stackType) + + bgError := New(err, 0) + actualStack := bgError.StackFrames() + expected := []StackFrame{ + {Name: "AStack", File: "errors/error_types_test.go", LineNumber: 28}, + {Name: "BStack", File: "errors/error_types_test.go", LineNumber: 29}, + {Name: "CStack", File: "errors/error_types_test.go", LineNumber: 30}, + {Name: "TestStackError", File: "errors/error_types_test.go", LineNumber: 71}, + } + + assertStacksMatch(t, expected, actualStack) +} + +// Errors implementing Callers() interface should have their own stack +// Inlined functions should be visible in StackFrames +func TestCallersError(t *testing.T) { + err := CCallers() + typeAssert(t, err, callersType) + + bgError := New(err, 0) + actualStack := bgError.StackFrames() + expected := []StackFrame{ + {Name: "ACallers", File: "errors/error_types_test.go", LineNumber: 32}, + {Name: "BCallers", File: "errors/error_types_test.go", LineNumber: 33}, + {Name: "CCallers", File: "errors/error_types_test.go", LineNumber: 34}, + {Name: "TestCallersError", File: "errors/error_types_test.go", LineNumber: 89}, + } + assertStacksMatch(t, expected, actualStack) +} + +// Errors with StackFrames are explicilty adding stacktrace to error +// Inlined functions should be visible in StackFrames +func TestFramesError(t *testing.T) { + err := CFrames() + typeAssert(t, err, stackFramesType) + + bgError := New(err, 0) + actualStack := bgError.StackFrames() + expected := []StackFrame{ + {Name: "AFrames", File: "errors/error_types_test.go", LineNumber: 36}, + {Name: "BFrames", File: "errors/error_types_test.go", LineNumber: 37}, + {Name: "CFrames", File: "errors/error_types_test.go", LineNumber: 38}, + {Name: "TestFramesError", File: "errors/error_types_test.go", LineNumber: 106}, + } + + assertStacksMatch(t, expected, actualStack) +} + +func typeAssert(t *testing.T, err interface{}, expectedType string) { + actualType := checkType(err) + if actualType != expectedType { + t.Errorf("Types don't match. Actual: %+v and expected: %+v\n", actualType, expectedType) + } +} + +func checkType(err interface{}) string { + switch err.(type) { + case *Error: + return bugsnagType + case ErrorWithCallers: + return callersType + case errorWithStack: + return stackType + case ErrorWithStackFrames: + return stackFramesType + case error: + return internalType + default: + return stringType + } +} From 2a43fd1129e1c6404efe662e649f1c98cc73e7f5 Mon Sep 17 00:00:00 2001 From: Daria Bialobrzeska Date: Fri, 23 Feb 2024 16:02:00 +0100 Subject: [PATCH 2/2] Make line numbers relative to make tests less fragile --- v2/errors/error_types_test.go | 38 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/v2/errors/error_types_test.go b/v2/errors/error_types_test.go index ee1cd63..4d86924 100644 --- a/v2/errors/error_types_test.go +++ b/v2/errors/error_types_test.go @@ -2,6 +2,7 @@ package errors import ( "fmt" + "runtime" "testing" pkgerror "github.com/pkg/errors" @@ -16,7 +17,7 @@ const ( stringType = "stringType" ) -// Prepared to test inlining +// Prepare to test inlining func AInternal() interface{} { return fmt.Errorf("pure golang error") } func BInternal() interface{} { return AInternal() } func CInternal() interface{} { return BInternal() } @@ -43,10 +44,11 @@ func TestInternalError(t *testing.T) { err := CInternal() typeAssert(t, err, internalType) + _, _, line, _ := runtime.Caller(0) // grab line immediately before error generator bgError := New(err, 0) actualStack := bgError.StackFrames() expected := []StackFrame{ - {Name: "TestInternalError", File: "errors/error_types_test.go", LineNumber: 46}, + {Name: "TestInternalError", File: "errors/error_types_test.go", LineNumber: line + 1}, } assertStacksMatch(t, expected, actualStack) } @@ -57,10 +59,11 @@ func TestStringError(t *testing.T) { err := CString() typeAssert(t, err, stringType) + _, _, line, _ := runtime.Caller(0) // grab line immediately before error generator bgError := New(err, 0) actualStack := bgError.StackFrames() expected := []StackFrame{ - {Name: "TestStringError", File: "errors/error_types_test.go", LineNumber: 60}, + {Name: "TestStringError", File: "errors/error_types_test.go", LineNumber: line + 1}, } assertStacksMatch(t, expected, actualStack) } @@ -68,16 +71,17 @@ func TestStringError(t *testing.T) { // Errors from pkg/errors have their own stack // Inlined functions should be visible in StackFrames func TestStackError(t *testing.T) { + _, _, line, _ := runtime.Caller(0) // grab line immediately before error generator err := CStack() typeAssert(t, err, stackType) bgError := New(err, 0) actualStack := bgError.StackFrames() expected := []StackFrame{ - {Name: "AStack", File: "errors/error_types_test.go", LineNumber: 28}, - {Name: "BStack", File: "errors/error_types_test.go", LineNumber: 29}, - {Name: "CStack", File: "errors/error_types_test.go", LineNumber: 30}, - {Name: "TestStackError", File: "errors/error_types_test.go", LineNumber: 71}, + {Name: "AStack", File: "errors/error_types_test.go", LineNumber: 29}, + {Name: "BStack", File: "errors/error_types_test.go", LineNumber: 30}, + {Name: "CStack", File: "errors/error_types_test.go", LineNumber: 31}, + {Name: "TestStackError", File: "errors/error_types_test.go", LineNumber: line + 1}, } assertStacksMatch(t, expected, actualStack) @@ -86,33 +90,35 @@ func TestStackError(t *testing.T) { // Errors implementing Callers() interface should have their own stack // Inlined functions should be visible in StackFrames func TestCallersError(t *testing.T) { + _, _, line, _ := runtime.Caller(0) // grab line immediately before error generator err := CCallers() typeAssert(t, err, callersType) bgError := New(err, 0) actualStack := bgError.StackFrames() expected := []StackFrame{ - {Name: "ACallers", File: "errors/error_types_test.go", LineNumber: 32}, - {Name: "BCallers", File: "errors/error_types_test.go", LineNumber: 33}, - {Name: "CCallers", File: "errors/error_types_test.go", LineNumber: 34}, - {Name: "TestCallersError", File: "errors/error_types_test.go", LineNumber: 89}, + {Name: "ACallers", File: "errors/error_types_test.go", LineNumber: 33}, + {Name: "BCallers", File: "errors/error_types_test.go", LineNumber: 34}, + {Name: "CCallers", File: "errors/error_types_test.go", LineNumber: 35}, + {Name: "TestCallersError", File: "errors/error_types_test.go", LineNumber: line + 1}, } assertStacksMatch(t, expected, actualStack) } -// Errors with StackFrames are explicilty adding stacktrace to error +// Errors with StackFrames are explicitly adding stacktrace to error // Inlined functions should be visible in StackFrames func TestFramesError(t *testing.T) { + _, _, line, _ := runtime.Caller(0) // grab line immediately before error generator err := CFrames() typeAssert(t, err, stackFramesType) bgError := New(err, 0) actualStack := bgError.StackFrames() expected := []StackFrame{ - {Name: "AFrames", File: "errors/error_types_test.go", LineNumber: 36}, - {Name: "BFrames", File: "errors/error_types_test.go", LineNumber: 37}, - {Name: "CFrames", File: "errors/error_types_test.go", LineNumber: 38}, - {Name: "TestFramesError", File: "errors/error_types_test.go", LineNumber: 106}, + {Name: "AFrames", File: "errors/error_types_test.go", LineNumber: 37}, + {Name: "BFrames", File: "errors/error_types_test.go", LineNumber: 38}, + {Name: "CFrames", File: "errors/error_types_test.go", LineNumber: 39}, + {Name: "TestFramesError", File: "errors/error_types_test.go", LineNumber: line + 1}, } assertStacksMatch(t, expected, actualStack)