diff --git a/error.go b/error.go index adcd595..517f016 100644 --- a/error.go +++ b/error.go @@ -98,24 +98,23 @@ func Wrap(err error, opts ...Option) error { return appErr } -func wrap(err error) *Error { - pkgErr := extractPkgError(err) +func wrap(err error) (wrappedErr *Error) { + stackTrace := newStackTrace(1) + pkgErr := extractPkgError(err) if appErr, ok := pkgErr.Err.(*Error); ok { - return appErr.Copy() + wrappedErr = appErr.Copy() + } else { + wrappedErr = &Error{ + Err: pkgErr.Err, + StackTrace: pkgErr.StackTrace, + } + WithMessage(pkgErr.Message)(wrappedErr) } - stackTrace := pkgErr.StackTrace - if stackTrace == nil { - stackTrace = newStackTrace(1) - } + wrappedErr.StackTrace = mergeStackTraces(wrappedErr.StackTrace, stackTrace) - wrappedErr := &Error{ - Err: pkgErr.Err, - StackTrace: stackTrace, - } - WithMessage(pkgErr.Message)(wrappedErr) - return wrappedErr + return } // Unwrap extracts an underlying *fail.Error from an error. diff --git a/error_test.go b/error_test.go index 78a2a0d..70a8941 100644 --- a/error_test.go +++ b/error_test.go @@ -326,8 +326,13 @@ func TestAll(t *testing.T) { assert.Equal(t, "e2: e1: e0", appErr.FullMessage()) assert.Equal(t, nil, appErr.Code) assert.Equal(t, false, appErr.Ignorable) - assert.NotEmpty(t, appErr.StackTrace) - assert.Equal(t, "errFunc1", appErr.StackTrace[0].Func) + assert.Equal(t, []string{ + "errFunc1", + "errFunc2", + "errFunc3", + "TestAll", + "tRunner", + }, funcNamesFromStackTrace(appErr.StackTrace)) } { @@ -335,8 +340,29 @@ func TestAll(t *testing.T) { assert.Equal(t, "e4: e2: e1: e0", appErr.FullMessage()) assert.Equal(t, 500, appErr.Code) assert.Equal(t, true, appErr.Ignorable) - assert.NotEmpty(t, appErr.StackTrace) - assert.Equal(t, "errFunc1", appErr.StackTrace[0].Func) + assert.Equal(t, []string{ + "errFunc1", + "errFunc2", + "errFunc3", + "errFunc4", + "TestAll", + "tRunner", + }, funcNamesFromStackTrace(appErr.StackTrace)) + } + + { + appErr := Unwrap(errFunc4Goroutine()) + assert.Equal(t, "e4: e2: e1: e0", appErr.FullMessage()) + assert.Equal(t, 500, appErr.Code) + assert.Equal(t, true, appErr.Ignorable) + assert.Equal(t, []string{ + "errFunc1", + "errFunc2", + "errFunc3Goroutine.func1", + "errFunc4Goroutine", + "TestAll", + "tRunner", + }, funcNamesFromStackTrace(appErr.StackTrace)) } } @@ -359,3 +385,21 @@ func errFunc3() error { func errFunc4() error { return Wrap(errFunc3(), WithMessage("e4"), WithCode(500), WithIgnorable()) } + +func errFunc3Goroutine() chan error { + c := make(chan error) + go func() { + c <- Wrap(errFunc2()) + }() + return c +} +func errFunc4Goroutine() error { + return Wrap(<-errFunc3Goroutine(), WithMessage("e4"), WithCode(500), WithIgnorable()) +} + +func funcNamesFromStackTrace(stackTrace StackTrace) (funcNames []string) { + for _, frame := range stackTrace { + funcNames = append(funcNames, frame.Func) + } + return +} diff --git a/pkgerrors.go b/pkgerrors.go index fb8e63d..ef71ae4 100644 --- a/pkgerrors.go +++ b/pkgerrors.go @@ -1,9 +1,6 @@ package fail import ( - "fmt" - "strconv" - pkgerrors "github.com/pkg/errors" ) @@ -26,10 +23,11 @@ func extractPkgError(err error) pkgError { } rootErr := err - var st pkgerrors.StackTrace + var stackTraces []StackTrace for { - if stackTrace, ok := rootErr.(traceable); ok { - st = stackTrace.StackTrace() + if t, ok := rootErr.(traceable); ok { + stackTrace := convertStackTrace(t.StackTrace()) + stackTraces = append(stackTraces, stackTrace) } if cause, ok := rootErr.(causer); ok { @@ -40,21 +38,6 @@ func extractPkgError(err error) pkgError { break } - var frames []Frame - if st != nil { - for _, t := range st { - file := fmt.Sprintf("%s", t) - line, _ := strconv.ParseInt(fmt.Sprintf("%d", t), 10, 64) - funcName := fmt.Sprintf("%n", t) - - frames = append(frames, Frame{ - Func: funcName, - Line: line, - File: file, - }) - } - } - var msg string if err.Error() != rootErr.Error() { msg = err.Error() @@ -63,6 +46,16 @@ func extractPkgError(err error) pkgError { return pkgError{ Err: rootErr, Message: msg, - StackTrace: frames, + StackTrace: reduceStackTraces(stackTraces), + } +} + +// convertStackTrace converts pkg/errors.StackTrace into fail.StackTrace +func convertStackTrace(stackTrace pkgerrors.StackTrace) (frames StackTrace) { + for _, t := range stackTrace { + if frame, ok := newFrameFrom(uintptr(t)); ok { + frames = append(frames, frame) + } } + return } diff --git a/stack.go b/stack.go index 1cf55da..384a6ab 100644 --- a/stack.go +++ b/stack.go @@ -20,6 +20,27 @@ type Frame struct { Line int64 } +// newFrameFrom creates Frame from the specified program counter +func newFrameFrom(pc uintptr) (f Frame, ok bool) { + fpc := runtime.FuncForPC(pc) + if fpc == nil { + return + } + + file, line := fpc.FileLine(pc) + + f.Func = funcname(fpc.Name()) + f.File = trimGOPATH(fpc.Name(), file) + f.Line = int64(line) + + if strings.HasPrefix(f.File, "runtime/") { + return + } + + ok = true + return +} + // newStackTrace creates StackTrace by callers func newStackTrace(offset int) StackTrace { pcs := make([]uintptr, stackMaxSize) @@ -29,19 +50,10 @@ func newStackTrace(offset int) StackTrace { frames := make([]Frame, n) for _, pc := range pcs[0:n] { - f := runtime.FuncForPC(pc) - if f == nil { - continue - } - - file, line := f.FileLine(pc) - - frames[i] = Frame{ - Func: funcname(f.Name()), - File: trimGOPATH(f.Name(), file), - Line: int64(line), + if f, ok := newFrameFrom(pc); ok { + frames[i] = f + i++ } - i++ } return frames[:i] @@ -96,3 +108,34 @@ func trimGOPATH(name, file string) string { file = file[i+len(sep):] return file } + +// mergeStackTraces merges two stack traces +func mergeStackTraces(inner StackTrace, outer StackTrace) StackTrace { + innerLen := len(inner) + outerLen := len(outer) + + if innerLen > outerLen { + overlap := 0 + for overlap < outerLen { + if inner[innerLen-overlap-1] != outer[outerLen-overlap-1] { + break + } + overlap++ + } + + if overlap > 0 { + return append(inner[:innerLen-overlap], outer...) + } + } + + return append(inner, outer...) +} + +// reduceStackTraces incrementally merges multiple stack traces +// and returns a merged stack trace +func reduceStackTraces(stackTraces []StackTrace) (merged StackTrace) { + for i := len(stackTraces) - 1; i >= 0; i-- { + merged = mergeStackTraces(merged, stackTraces[i]) + } + return +} diff --git a/stack_test.go b/stack_test.go index dc2a889..a6fcdac 100644 --- a/stack_test.go +++ b/stack_test.go @@ -57,3 +57,113 @@ func TestTrimGOPATH(t *testing.T) { assert.Equal(t, "pkg/sub/file.go", trimGOPATH(funcName, file)) } + +func TestMergeStackTraces(t *testing.T) { + t.Run("empty", func(t *testing.T) { + inner := StackTrace{} + outer := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + } + result := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + } + + assert.Equal(t, result, mergeStackTraces(inner, outer)) + }) + + t.Run("inner < outer", func(t *testing.T) { + inner := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + } + outer := StackTrace{ + {Func: "f1", File: "main.go", Line: 157}, + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + } + result := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + {Func: "f1", File: "main.go", Line: 157}, + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + } + + assert.Equal(t, result, mergeStackTraces(inner, outer)) + }) + + t.Run("inner > outer (overlapping)", func(t *testing.T) { + inner := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + {Func: "f1", File: "main.go", Line: 157}, + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + } + outer := StackTrace{ + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + } + result := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + {Func: "f1", File: "main.go", Line: 157}, + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + } + + assert.Equal(t, result, mergeStackTraces(inner, outer)) + }) + + t.Run("inner > outer (no overlapping frames)", func(t *testing.T) { + inner := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + {Func: "f1", File: "main.go", Line: 157}, + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + } + outer := StackTrace{ + {Func: "g2", File: "main.go", Line: 1061}, + {Func: "g3.func1", File: "main.go", Line: 1067}, + } + result := StackTrace{ + {Func: "init", File: "main.go", Line: 154}, + {Func: "f1", File: "main.go", Line: 157}, + {Func: "f2", File: "main.go", Line: 161}, + {Func: "f3.func1", File: "main.go", Line: 167}, + {Func: "g2", File: "main.go", Line: 1061}, + {Func: "g3.func1", File: "main.go", Line: 1067}, + } + + assert.Equal(t, result, mergeStackTraces(inner, outer)) + }) +} + +func TestReduceStackTraces(t *testing.T) { + input := []StackTrace{ + { + {Func: "main", File: "main.go", Line: 179}, + }, + { + {Func: "f3.func1", File: "main.go", Line: 168}, + }, + { + {Func: "f2", File: "main.go", Line: 162}, + {Func: "f3.func1", File: "main.go", Line: 168}, + }, + { + {Func: "f1", File: "main.go", Line: 158}, + {Func: "f2", File: "main.go", Line: 162}, + {Func: "f3.func1", File: "main.go", Line: 168}, + }, + { + {Func: "init", File: "main.go", Line: 155}, + }, + {}, + } + result := StackTrace{ + {Func: "init", File: "main.go", Line: 155}, + {Func: "f1", File: "main.go", Line: 158}, + {Func: "f2", File: "main.go", Line: 162}, + {Func: "f3.func1", File: "main.go", Line: 168}, + {Func: "main", File: "main.go", Line: 179}, + } + + assert.Equal(t, result, reduceStackTraces(input)) +}