Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use runtime.CallersFrames to create StackFrames #113

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,8 @@ func (err *Error) Stack() []byte {
// stack.
func (err *Error) StackFrames() []StackFrame {
if err.frames == nil {
err.frames = make([]StackFrame, len(err.stack))

for i, pc := range err.stack {
err.frames[i] = NewStackFrame(pc)
}
err.frames = runtimeToErrorFrames(pcsToFrames(err.stack))
}

return err.frames
}

Expand Down
52 changes: 26 additions & 26 deletions errors/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,48 +4,57 @@ import (
"bytes"
"fmt"
"io"
"runtime/debug"
"reflect"
"runtime"
"testing"
)

func TestStackFormatMatches(t *testing.T) {
type prettyStack []StackFrame

func (s prettyStack) String() string {
buf := &bytes.Buffer{}
for _, f := range s {
_, _ = fmt.Fprintf(buf, "%#v\n", f)
}
return buf.String()
}

func callerFrames(skip int) []runtime.Frame {
callers := make([]uintptr, MaxStackDepth)
n := runtime.Callers(2+skip, callers)
return pcsToFrames(callers[:n])
}

func TestStackFrameMatches(t *testing.T) {
defer func() {
err := recover()
if err != 'a' {
t.Fatal(err)
}

bs := [][]byte{Errorf("hi").Stack(), debug.Stack()}
expected := runtimeToErrorFrames(callerFrames(0))[1:]
got := Errorf("hi").StackFrames()[1:]

// Ignore the first line (as it contains the PC of the .Stack() call)
bs[0] = bytes.SplitN(bs[0], []byte("\n"), 2)[1]
bs[1] = bytes.SplitN(bs[1], []byte("\n"), 2)[1]

if bytes.Compare(bs[0], bs[1]) != 0 {
t.Errorf("Stack didn't match")
t.Errorf("%s", bs[0])
t.Errorf("%s", bs[1])
if !reflect.DeepEqual(expected, got) {
t.Errorf("Stacks didn't match\nGot:\n%v\nExpected:\n%v", prettyStack(got), prettyStack(expected))
}
}()

a()
}

func TestSkipWorks(t *testing.T) {

defer func() {
err := recover()
if err != 'a' {
t.Fatal(err)
}

bs := [][]byte{New("hi", 2).Stack(), debug.Stack()}
expected := runtimeToErrorFrames(callerFrames(2))
got := New("hi", 2).StackFrames()

if !bytes.HasSuffix(bs[1], bs[0]) {
t.Errorf("Stack didn't match")
t.Errorf("%s", bs[0])
t.Errorf("%s", bs[1])
if !reflect.DeepEqual(expected, got) {
t.Errorf("Stacks didn't match\nGot:\n%v\nExpected:\n%v", prettyStack(got), prettyStack(expected))
}
}()

Expand Down Expand Up @@ -124,15 +133,6 @@ func ExampleNew_skip() {
}()
}

func ExampleError_Stack() {
e := New("Oh noes!", 1)
fmt.Printf("Error: %s\n", e.Error())
fmt.Printf("Stack is %d bytes", len(e.Stack()))
// Output:
// Error: Oh noes!
// Stack is 589 bytes
}

func a() error {
b(5)
return nil
Expand Down
33 changes: 32 additions & 1 deletion errors/stackframe.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type StackFrame struct {

// NewStackFrame popoulates a stack frame object from the program counter.
func NewStackFrame(pc uintptr) (frame StackFrame) {

frame = StackFrame{ProgramCounter: pc}
if frame.Func() == nil {
return
Expand All @@ -31,7 +30,18 @@ func NewStackFrame(pc uintptr) (frame StackFrame) {
// and we want to show the line that corresponds to the function call
frame.File, frame.LineNumber = frame.Func().FileLine(pc - 1)
return
}

// NewStackFrameFromRuntime populates a stack frame object from a runtime.Frame object.
func NewStackFrameFromRuntime(frame runtime.Frame) StackFrame {
pkg, name := packageAndName(frame.Func)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that frame.Func can still be nil sometimes. Might be worth attempting to salvage something from frame.Function in that case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that seems reasonable. I've pushed a commit that does this.

return StackFrame{
File: frame.File,
LineNumber: frame.Line,
Name: name,
Package: pkg,
ProgramCounter: frame.PC,
}
}

// Func returns the function that this stackframe corresponds to
Expand Down Expand Up @@ -95,3 +105,24 @@ func packageAndName(fn *runtime.Func) (string, string) {
name = strings.Replace(name, "·", ".", -1)
return pkg, name
}

func pcsToFrames(pcs []uintptr) []runtime.Frame {
frames := runtime.CallersFrames(pcs)
s := make([]runtime.Frame, 0, len(pcs))
for {
frame, more := frames.Next()
s = append(s, frame)
if !more {
break
}
}
return s
}

func runtimeToErrorFrames(rtFrames []runtime.Frame) []StackFrame {
frames := make([]StackFrame, len(rtFrames))
for i, f := range rtFrames {
frames[i] = NewStackFrameFromRuntime(f)
}
return frames
}
50 changes: 27 additions & 23 deletions notifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package bugsnag_test

import (
"fmt"
"runtime"
"testing"

simplejson "github.com/bitly/go-simplejson"
"github.com/bitly/go-simplejson"

"github.com/bugsnag/bugsnag-go"
. "github.com/bugsnag/bugsnag-go/testutil"
)
Expand All @@ -22,6 +24,26 @@ func crash(s interface{}) int {
return s.(int)
}

// numCrashFrames returns the number of frames to expect from calling crash(...)
func numCrashFrames(s interface{}) (n int) {
defer func() {
_ = recover()
pcs := make([]uintptr, 50)
// exclude Callers, deferred anon function, & numCrashFrames itself
m := runtime.Callers(3, pcs)
frames := runtime.CallersFrames(pcs[:m])
for {
n++
_, more := frames.Next()
if !more {
break
}
}
}()
crash(s)
return
}

func TestStackframesAreSkippedCorrectly(t *testing.T) {
ts, reports := Setup()
bugsnaggedReports = reports
Expand Down Expand Up @@ -61,54 +83,36 @@ func TestStackframesAreSkippedCorrectly(t *testing.T) {
assertStackframeCount(st, 3)
})

// Expect the following frames to be present for *.AutoNotify
/*
{ "file": "runtime/panic.go", "method": "gopanic" },
{ "file": "runtime/iface.go", "method": "panicdottypeE" },
{ "file": "$GOPATH/src/github.com/bugsnag/bugsnag-go/notifier_test.go", "method": "TestStackframesAreSkippedCorrectly.func2.1" },
{ "file": "$GOPATH/src/github.com/bugsnag/bugsnag-go/notifier_test.go", "method": "TestStackframesAreSkippedCorrectly.func3" },
{ "file": "testing/testing.go", "method": "tRunner" },
{ "file": "runtime/asm_amd64.s", "method": "goexit" }
*/
t.Run("notifier.AutoNotify", func(st *testing.T) {
func() {
defer func() { recover() }()
defer notifier.AutoNotify()
crash("NaN")
}()
assertStackframeCount(st, 6)
assertStackframeCount(st, numCrashFrames("NaN"))
})
t.Run("bugsnag.AutoNotify", func(st *testing.T) {
func() {
defer func() { recover() }()
defer bugsnag.AutoNotify()
crash("NaN")
}()
assertStackframeCount(st, 6)
assertStackframeCount(st, numCrashFrames("NaN"))
})

// Expect the following frames to be present for *.Recover
/*
{ "file": "runtime/panic.go", "method": "gopanic" },
{ "file": "runtime/iface.go", "method": "panicdottypeE" },
{ "file": "$GOPATH/src/github.com/bugsnag/bugsnag-go/notifier_test.go", "method": "TestStackframesAreSkippedCorrectly.func4.1" },
{ "file": "$GOPATH/src/github.com/bugsnag/bugsnag-go/notifier_test.go", "method": "TestStackframesAreSkippedCorrectly.func4" },
{ "file": "testing/testing.go", "method": "tRunner" },
{ "file": "runtime/asm_amd64.s", "method": "goexit" }
*/
t.Run("notifier.Recover", func(st *testing.T) {
func() {
defer notifier.Recover()
crash("NaN")
}()
assertStackframeCount(st, 6)
assertStackframeCount(st, numCrashFrames("NaN"))
})
t.Run("bugsnag.Recover", func(st *testing.T) {
func() {
defer bugsnag.Recover()
crash("NaN")
}()
assertStackframeCount(st, 6)
assertStackframeCount(st, numCrashFrames("NaN"))
})
}

Expand Down
25 changes: 19 additions & 6 deletions panicwrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func TestPanicHandlerHandledPanic(t *testing.T) {

startPanickingProcess(t, "handled", ts.URL)

// Yeah, we just caught a panic from the init() function below and sent it to the server running above (mindblown)
json, err := simplejson.NewJson(<-reports)
if err != nil {
t.Fatal(err)
Expand All @@ -42,12 +43,24 @@ func TestPanicHandlerHandledPanic(t *testing.T) {
event := getIndex(json, "events", 0)
assertValidSession(t, event, true)

// Yeah, we just caught a panic from the init() function below and sent it to the server running above (mindblown)
frame := getIndex(getIndex(event, "exceptions", 0), "stacktrace", 1)
if getBool(frame, "inProject") != true ||
getString(frame, "file") != "panicwrap_test.go" ||
getInt(frame, "lineNumber") == 0 {
t.Errorf("stack frame seems wrong at index 1: %v", frame)
frames := event.Get("exceptions").
GetIndex(0).
Get("stacktrace")

found := false
for i := range frames.MustArray() {
frame := frames.GetIndex(i)
if getString(frame, "file") == "panicwrap_test.go" &&
getBool(frame, "inProject") &&
getInt(frame, "lineNumber") != 0 {
found = true
break
}
}
if !found {
t.Errorf("stack frames seem wrong: can't find panicwrap_test.go frame")
s, _ := frames.EncodePretty()
t.Log(string(s))
}
}

Expand Down