Start showing inlined functions in stack trace (#208)

* Start showing inlined functions in stack trace
* Fix notifier tests
* Added change to CHANGELOG

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## TBD
+
+### Bug fixes
+
+* Start showing inlined functions in stack trace
+  [#208](https://github.com/bugsnag/bugsnag-go/pull/208)
+
 ## 2.2.0 (2022-10-12)
 
 ### Enhancements

v2/errors/error.go

@@ -4,9 +4,10 @@ package errors
 import (
 	"bytes"
 	"fmt"
-	"github.com/pkg/errors"
 	"reflect"
 	"runtime"
+
+	"github.com/pkg/errors"
 )
 
 // The maximum number of stackframes on any error.
@@ -64,7 +65,7 @@ func New(e interface{}, skip int) *Error {
 		trace := e.StackTrace()
 		stack := make([]uintptr, len(trace))
 		for i, ptr := range trace {
-			stack[i] = uintptr(ptr) - 1
+			stack[i] = uintptr(ptr)
 		}
 		return &Error{
 			Err:   e,
@@ -132,20 +133,32 @@ func (err *Error) StackFrames() []StackFrame {
 	if err.frames == nil {
 		callers := runtime.CallersFrames(err.stack)
 		err.frames = make([]StackFrame, 0, len(err.stack))
+
 		for frame, more := callers.Next(); more; frame, more = callers.Next() {
-			if frame.Func == nil {
-				// Ignore fully inlined functions
-				continue
-			}
-			pkg, name := packageAndName(frame.Func)
-			err.frames = append(err.frames, StackFrame{
+			processedStackFrame := StackFrame{
 				function:       frame.Func,
 				File:           frame.File,
 				LineNumber:     frame.Line,
-				Name:           name,
-				Package:        pkg,
 				ProgramCounter: frame.PC,
-			})
+			}
+
+			frameFunc := frame.Func
+			if frameFunc == nil {
+				newFrameFunc := runtime.FuncForPC(frame.PC)
+				if newFrameFunc != nil {
+					file, line := newFrameFunc.FileLine(frame.PC)
+					// Unwrap fully inlined functions
+					processedStackFrame.File = file
+					processedStackFrame.LineNumber = line
+					processedStackFrame.function = newFrameFunc
+					frameFunc = newFrameFunc
+				}
+			}
+
+			pkg, name := packageAndName(frameFunc)
+			processedStackFrame.Name = name
+			processedStackFrame.Package = pkg
+			err.frames = append(err.frames, processedStackFrame)
 		}
 	}
 

v2/errors/error_test.go

@@ -13,7 +13,7 @@ import (
 
 // fixture functions doing work to avoid inlining
 func a(i int) error {
-	if b(i + 5) && b(i + 6) {
+	if b(i+5) && b(i+6) {
 		return nil
 	}
 	return fmt.Errorf("not gonna happen")
@@ -47,8 +47,28 @@ func TestParsePanicStack(t *testing.T) {
 		}
 		expected := []StackFrame{
 			StackFrame{Name: "TestParsePanicStack.func1", File: "errors/error_test.go"},
-			StackFrame{Name: "a", File: "errors/error_test.go", LineNumber: 16},
+			StackFrame{Name: "gopanic"},
+		}
+
+		golangVersion := runtime.Version()
+
+		// TODO remove this after dropping support for Golang 1.11
+		// Golang version < 1.12 cannot unwrap inlined functions correctly.
+		if strings.HasPrefix(golangVersion, "go1.11") {
+			expected = append(expected,
+				StackFrame{Name: "a", File: "errors/error_test.go", LineNumber: 29},
+				StackFrame{Name: "a", File: "errors/error_test.go", LineNumber: 29},
+				StackFrame{Name: "a", File: "errors/error_test.go", LineNumber: 16},
+			)
+		} else {
+			// For versions >= 1.12 inlined functions show normally
+			expected = append(expected,
+				StackFrame{Name: "c", File: "errors/error_test.go", LineNumber: 29},
+				StackFrame{Name: "b", File: "errors/error_test.go", LineNumber: 23},
+				StackFrame{Name: "a", File: "errors/error_test.go", LineNumber: 16},
+			)
 		}
+
 		assertStacksMatch(t, expected, err.StackFrames())
 	}()
 

v2/errors/stackframe.go

@@ -70,6 +70,10 @@ func (frame *StackFrame) SourceLine() (string, error) {
 }
 
 func packageAndName(fn *runtime.Func) (string, string) {
+	if fn == nil {
+		return "", ""
+	}
+
 	name := fn.Name()
 	pkg := ""
 

v2/go.mod

@@ -3,11 +3,9 @@ module github.com/bugsnag/bugsnag-go/v2
 go 1.15
 
 require (
-	github.com/bitly/go-simplejson v0.5.0
-	github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 // indirect
+	github.com/bitly/go-simplejson v0.5.1
 	github.com/bugsnag/panicwrap v1.3.4
-	github.com/google/uuid v1.3.0
+	github.com/google/uuid v1.6.0
 	github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 // indirect
-	github.com/kr/pretty v0.2.1 // indirect
 	github.com/pkg/errors v0.9.1
 )

v2/go.sum

@@ -1,17 +1,10 @@
-github.com/bitly/go-simplejson v0.5.0 h1:6IH+V8/tVMab511d5bn4M7EwGXZf9Hj6i2xSwkNEM+Y=
-github.com/bitly/go-simplejson v0.5.0/go.mod h1:cXHtHw4XUPsvGaxgjIAn8PhEWG9NfngEKAMDJEczWVA=
-github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY=
-github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4=
+github.com/bitly/go-simplejson v0.5.1 h1:xgwPbetQScXt1gh9BmoJ6j9JMr3TElvuIyjR8pgdoow=
+github.com/bitly/go-simplejson v0.5.1/go.mod h1:YOPVLzCfwK14b4Sff3oP1AmGhI9T9Vsg84etUnlyp+Q=
 github.com/bugsnag/panicwrap v1.3.4 h1:A6sXFtDGsgU/4BLf5JT0o5uYg3EeKgGx3Sfs+/uk3pU=
 github.com/bugsnag/panicwrap v1.3.4/go.mod h1:D/8v3kj0zr8ZAKg1AQ6crr+5VwKN5eIywRkfhyM/+dE=
-github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
-github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
+github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA=
 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8=
-github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
-github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
-github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
-github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
-github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

v2/notifier_test.go

@@ -2,6 +2,7 @@ package bugsnag_test
 
 import (
 	"fmt"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -61,21 +62,52 @@ func TestStackframesAreSkippedCorrectly(t *testing.T) {
 			defer notifier.AutoNotify()
 			crash("NaN")
 		}()
-		assertStackframesMatch(t, []errors.StackFrame{
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4.1", File: "notifier_test.go"},
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4", File: "notifier_test.go"},
-		})
+
+		var expected []errors.StackFrame
+		golangVersion := runtime.Version()
+		// TODO remove this after dropping support for Golang 1.11
+		// Golang version < 1.12 cannot unwrap inlined functions correctly.
+		if strings.HasPrefix(golangVersion, "go1.11") {
+			expected = append(expected,
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4.1", File: "notifier_test.go"}, //inlined crash func
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4", File: "notifier_test.go"},
+			)
+		} else {
+			expected = append(expected,
+				errors.StackFrame{Name: "crash", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func4", File: "notifier_test.go"},
+			)
+		}
+
+		assertStackframesMatch(t, expected)
 	})
 	t.Run("bugsnag.AutoNotify", func(st *testing.T) {
 		func() {
 			defer func() { recover() }()
 			defer bugsnag.AutoNotify()
 			crash("NaN")
 		}()
-		assertStackframesMatch(t, []errors.StackFrame{
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5.1", File: "notifier_test.go"},
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5", File: "notifier_test.go"},
-		})
+		var expected []errors.StackFrame
+		golangVersion := runtime.Version()
+		// TODO remove this after dropping support for Golang 1.11
+		// Golang version < 1.12 cannot unwrap inlined functions correctly.
+		if strings.HasPrefix(golangVersion, "go1.11") {
+			expected = append(expected,
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5.1", File: "notifier_test.go"}, //inlined crash func
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5", File: "notifier_test.go"},
+			)
+		} else {
+			expected = append(expected,
+				errors.StackFrame{Name: "crash", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func5", File: "notifier_test.go"},
+			)
+		}
+
+		assertStackframesMatch(t, expected)
 	})
 
 	// Expect the following frames to be present for *.Recover
@@ -92,20 +124,50 @@ func TestStackframesAreSkippedCorrectly(t *testing.T) {
 			defer notifier.Recover()
 			crash("NaN")
 		}()
-		assertStackframesMatch(t, []errors.StackFrame{
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6.1", File: "notifier_test.go"},
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6", File: "notifier_test.go"},
-		})
+		var expected []errors.StackFrame
+		golangVersion := runtime.Version()
+		// TODO remove this after dropping support for Golang 1.11
+		// Golang version < 1.12 cannot unwrap inlined functions correctly.
+		if strings.HasPrefix(golangVersion, "go1.11") {
+			expected = append(expected,
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6.1", File: "notifier_test.go"}, //inlined crash func
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6", File: "notifier_test.go"},
+			)
+		} else {
+			expected = append(expected,
+				errors.StackFrame{Name: "crash", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func6", File: "notifier_test.go"},
+			)
+		}
+
+		assertStackframesMatch(t, expected)
 	})
 	t.Run("bugsnag.Recover", func(st *testing.T) {
 		func() {
 			defer bugsnag.Recover()
 			crash("NaN")
 		}()
-		assertStackframesMatch(t, []errors.StackFrame{
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7.1", File: "notifier_test.go"},
-			errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7", File: "notifier_test.go"},
-		})
+		var expected []errors.StackFrame
+		golangVersion := runtime.Version()
+		// TODO remove this after dropping support for Golang 1.11
+		// Golang version < 1.12 cannot unwrap inlined functions correctly.
+		if strings.HasPrefix(golangVersion, "go1.11") {
+			expected = append(expected,
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7.1", File: "notifier_test.go"}, //inlined crash func
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7", File: "notifier_test.go"},
+			)
+		} else {
+			expected = append(expected,
+				errors.StackFrame{Name: "crash", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7.1", File: "notifier_test.go"},
+				errors.StackFrame{Name: "TestStackframesAreSkippedCorrectly.func7", File: "notifier_test.go"},
+			)
+		}
+
+		assertStackframesMatch(t, expected)
 	})
 }
 
@@ -187,6 +249,7 @@ func assertStackframesMatch(t *testing.T, expected []errors.StackFrame) {
 			if strings.HasSuffix(file, expectedFrame.File) && expectedFrame.Name == method {
 				lastmatch = index
 				matched++
+				break
 			}
 		}
 	}