diff --git a/.travis.yml b/.travis.yml index 31a0ac7..7c1a9a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,7 @@ go: - 1.9.x - 1.10.x - 1.11.x + - 1.12.x - tip before_install: @@ -37,6 +38,9 @@ matrix: - name: "golint 1.11.x" go: 1.11.x script: ./scripts/lint.sh + - name: "golint 1.12.x" + go: 1.12.x + script: ./scripts/lint.sh allow_failures: - go: tip diff --git a/example/error_packages.go b/example/error_packages.go new file mode 100644 index 0000000..c73f7bd --- /dev/null +++ b/example/error_packages.go @@ -0,0 +1,72 @@ +// +build errors_packages + +package main + +import ( + "fmt" + + "github.com/davecgh/go-spew/spew" + raven "github.com/getsentry/raven-go" + goErrors "github.com/go-errors/errors" + pingcapErrors "github.com/pingcap/errors" + pkgErrors "github.com/pkg/errors" +) + +//============================== +// https://github.com/pkg/errors +//============================== + +func pkgBar() error { + return pkgErrors.New("this is bad from pkgErrors") +} + +func pkgFoo() error { + return pkgBar() +} + +//================================== +// https://github.com/pingcap/errors +//================================== + +func pingcapBar() error { + return pingcapErrors.New("this is bad from pingcapErrors") +} + +func pingcapFoo() error { + return pingcapBar() +} + +//==================================== +// https://github.com/go-errors/errors +//==================================== + +func goErrorsBar() error { + return goErrors.New(goErrors.Errorf("this is bad from goErrors")) +} + +func goErrorsFoo() error { + return goErrorsBar() +} + +//============================== + +func main() { + pkgErr := pkgFoo() + pkgStacktrace := raven.GetOrNewStacktrace(pkgErr, 0, 3, []string{}) + spew.Dump(pkgStacktrace) + spew.Dump(len(pkgStacktrace.Frames)) + + fmt.Print("\n\n\n") + + pingcapErr := pingcapFoo() + pingcapStacktrace := raven.GetOrNewStacktrace(pingcapErr, 0, 3, []string{}) + spew.Dump(pingcapStacktrace) + spew.Dump(len(pingcapStacktrace.Frames)) + + fmt.Print("\n\n\n") + + goErrorsErr := goErrorsFoo() + goErrorsStacktrace := raven.GetOrNewStacktrace(goErrorsErr, 0, 3, []string{}) + spew.Dump(goErrorsStacktrace) + spew.Dump(len(goErrorsStacktrace.Frames)) +} diff --git a/stacktrace.go b/stacktrace.go index 1a76db4..0aa1682 100644 --- a/stacktrace.go +++ b/stacktrace.go @@ -10,6 +10,7 @@ import ( "go/build" "io/ioutil" "path/filepath" + "reflect" "runtime" "strings" "sync" @@ -52,35 +53,63 @@ type StacktraceFrame struct { InApp bool `json:"in_app"` } -// GetOrNewStacktrace tries to get stacktrace from err as an interface of github.com/pkg/errors, or else NewStacktrace() -func GetOrNewStacktrace(err error, skip int, context int, appPackagePrefixes []string) *Stacktrace { - type stackTracer interface { - StackTrace() []runtime.Frame - } - stacktrace, ok := err.(stackTracer) - if !ok { - return NewStacktrace(skip+1, context, appPackagePrefixes) - } +// extractFramesFromPcs translates slice of stack trace pointers into usable frames +func extractFramesFromPcs(pcs []uintptr, context int, appPackagePrefixes []string) []*StacktraceFrame { var frames []*StacktraceFrame - for f := range stacktrace.StackTrace() { - pc := uintptr(f) - 1 - fn := runtime.FuncForPC(pc) - var fName string - var file string - var line int - if fn != nil { - file, line = fn.FileLine(pc) - fName = fn.Name() - } else { - file = "unknown" - fName = "unknown" - } - frame := NewStacktraceFrame(pc, fName, file, line, context, appPackagePrefixes) + callersFrames := runtime.CallersFrames(pcs) + + for { + fr, more := callersFrames.Next() + frame := NewStacktraceFrame(fr.PC, fr.Function, fr.File, fr.Line, context, appPackagePrefixes) if frame != nil { frames = append([]*StacktraceFrame{frame}, frames...) } + if !more { + break + } + } + + return frames +} + +// GetOrNewStacktrace tries to get stacktrace from err as an interface of github.com/pkg/errors, or else NewStacktrace() +// Use of reflection allows us to not have a hard dependency on any given package, so we don't have to import it +func GetOrNewStacktrace(err error, skip int, context int, appPackagePrefixes []string) *Stacktrace { + // https://github.com/pkg/errors + // https://github.com/pingcap/errors + methodStackTrace := reflect.ValueOf(err).MethodByName("StackTrace") + // https://github.com/go-errors/errors + methodStackFrames := reflect.ValueOf(err).MethodByName("StackFrames") + + if methodStackTrace.IsValid() { + stacktrace := methodStackTrace.Call(make([]reflect.Value, 0))[0] + if stacktrace.Kind() != reflect.Slice { + return NewStacktrace(skip+1, context, appPackagePrefixes) + } + + pcs := make([]uintptr, stacktrace.Len()) + for i := 0; i < stacktrace.Len(); i++ { + pcs = append(pcs, uintptr(stacktrace.Index(i).Uint())) + } + frames := extractFramesFromPcs(pcs, context, appPackagePrefixes) + + return &Stacktrace{frames} + } else if methodStackFrames.IsValid() { + stacktrace := methodStackFrames.Call(make([]reflect.Value, 0))[0] + if stacktrace.Kind() != reflect.Slice { + return NewStacktrace(skip+1, context, appPackagePrefixes) + } + + pcs := make([]uintptr, stacktrace.Len()) + for i := 0; i < stacktrace.Len(); i++ { + pcs = append(pcs, uintptr(stacktrace.Index(i).FieldByName("ProgramCounter").Uint())) + } + frames := extractFramesFromPcs(pcs, context, appPackagePrefixes) + + return &Stacktrace{frames} } - return &Stacktrace{Frames: frames} + + return NewStacktrace(skip+1, context, appPackagePrefixes) } // NewStacktrace intializes and populates a new stacktrace, skipping skip frames. @@ -92,8 +121,6 @@ func GetOrNewStacktrace(err error, skip int, context int, appPackagePrefixes []s // appPackagePrefixes is a list of prefixes used to check whether a package should // be considered "in app". func NewStacktrace(skip int, context int, appPackagePrefixes []string) *Stacktrace { - var frames []*StacktraceFrame - callerPcs := make([]uintptr, 100) numCallers := runtime.Callers(skip+2, callerPcs) @@ -102,32 +129,18 @@ func NewStacktrace(skip int, context int, appPackagePrefixes []string) *Stacktra return nil } - callersFrames := runtime.CallersFrames(callerPcs) + frames := extractFramesFromPcs(callerPcs, context, appPackagePrefixes) - for { - fr, more := callersFrames.Next() - if fr.Func != nil { - frame := NewStacktraceFrame(fr.PC, fr.Function, fr.File, fr.Line, context, appPackagePrefixes) - if frame != nil { - frames = append(frames, frame) - } - } - if !more { - break - } - } // If there are no frames, the entire stacktrace is nil if len(frames) == 0 { return nil } + // Optimize the path where there's only 1 frame if len(frames) == 1 { return &Stacktrace{frames} } - // Sentry wants the frames with the oldest first, so reverse them - for i, j := 0, len(frames)-1; i < j; i, j = i+1, j-1 { - frames[i], frames[j] = frames[j], frames[i] - } + return &Stacktrace{frames} } @@ -140,6 +153,14 @@ func NewStacktrace(skip int, context int, appPackagePrefixes []string) *Stacktra // appPackagePrefixes is a list of prefixes used to check whether a package should // be considered "in app". func NewStacktraceFrame(pc uintptr, fName, file string, line, context int, appPackagePrefixes []string) *StacktraceFrame { + if file == "" { + file = "unknown" + } + + if fName == "" { + fName = "unknown" + } + frame := &StacktraceFrame{AbsolutePath: file, Filename: trimPath(file), Lineno: line} frame.Module, frame.Function = functionName(fName) frame.InApp = isInAppFrame(*frame, appPackagePrefixes) @@ -150,6 +171,11 @@ func NewStacktraceFrame(pc uintptr, fName, file string, line, context int, appPa return nil } + // Skip useless frames + if frame.Filename == "unknown" && frame.Function == "unknown" && frame.Lineno == 0 && frame.Colno == 0 { + return nil + } + if context > 0 { contextLines, lineIdx := sourceCodeLoader.Load(file, line, context) if len(contextLines) > 0 { @@ -179,11 +205,11 @@ func isInAppFrame(frame StacktraceFrame, appPackagePrefixes []string) bool { if frame.Module == "main" { return true } - for _, prefix := range appPackagePrefixes { - if strings.HasPrefix(frame.Module, prefix) && !strings.Contains(frame.Module, "vendor") && !strings.Contains(frame.Module, "third_party") { - return true - } - } + for _, prefix := range appPackagePrefixes { + if strings.HasPrefix(frame.Module, prefix) && !strings.Contains(frame.Module, "vendor") && !strings.Contains(frame.Module, "third_party") { + return true + } + } return false }