2 frames shown per xerrors wrap instead of 1

[yonderblue]

Summary

Stack trace shown on sentry for wrapped xerrors show 2 frames per wrap instead of 1.

Steps To Reproduce

package main

import (
	"fmt"
	"log"
	"time"

	"github.com/getsentry/sentry-go"
	"golang.org/x/xerrors"
)

func main() {
	err := sentry.Init(sentry.ClientOptions{
		Dsn: "THE DSN",
	})
	if err != nil {
		log.Fatal(err)
	}
	defer sentry.Flush(2 * time.Second)

	err = xerrors.Errorf("context in main: %w", A())
	fmt.Printf("%+v\n", err)

	sentry.CaptureException(err)
}

func A() error {
	err := B{}.Boo()
	return xerrors.Errorf("context in A: %w", err)
}

type B struct{}

func (B) Boo() error {
	err := C()
	return xerrors.Errorf("context in B: %w", err)
}

func C() error {
	return xerrors.New("the xerror")
}

Prints:

context in main:
    main.main
        /Users/yonderblue/go/github.com/yonderblue/tmpsentry/tmpsentry.go:21
  - context in A:
    main.A
        /Users/yonderblue/go/github.com/yonderblue/tmpsentry/tmpsentry.go:29
  - context in B:
    main.B.Boo
        /Users/yonderblue/go/github.com/yonderblue/tmpsentry/tmpsentry.go:36
  - the xerror:
    main.C
        /Users/yonderblue/go/github.com/yonderblue/tmpsentry/tmpsentry.go:40

Shown in UI:

Expected Behavior

A single frame per wrap as in the %+v output above. There is little point to showing two, which is not a full stack and at the same time is redundant, adding confusion.

The only reason I can see that two is being shown is simply an artifact/mistake of grabbing all the frames for the PCs that were stored here https://github.com/golang/xerrors/blob/master/frame.go#L31, since that is what was being done for the other error providers. However you can clearly see that while xerrors.Frame does store 3 PCs, it is only because of what runtime.CallersFrames requires. location() only returns a single func/file/line. So in that spirit and for what xerror users are likely looking for in sentry, showing a single frame makes the most sense.

Please see #298

[rhcarvalho]

@yonderblue thanks for opening the PR and the issue. I see what you mean now.

There's a small patch that reduces the number of frames down to one.

diff --git a/stacktrace.go b/stacktrace.go
--- a/stacktrace.go
+++ b/stacktrace.go
@@ -149,7 +149,10 @@ func extractXErrorsPC(err error) []uintptr {
 
 	field := reflect.ValueOf(err).Elem().FieldByName("frame") // type Frame struct{ frames [3]uintptr }
 	field = field.FieldByName("frames")
-	field = field.Slice(1, field.Len()) // drop first pc pointing to xerrors.New
+	// drop first pc pointing to xerrors.New, take second frame, ignore
+	// third frame (redundant, see
+	// https://github.com/getsentry/sentry-go/issues/306)
+	field = field.Slice(1, 2)
 	pc := make([]uintptr, field.Len())
 	for i := 0; i < field.Len(); i++ {
 		pc[i] = uintptr(field.Index(i).Uint())

I need some more time to test if this is correct.

[yonderblue]

@rhcarvalho that is certainly the simpler way, but take a look at the link I put in the #298 description, hopefully they did it that way for good reason, which is why this PR is using the same.

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀