fix: return correct stack for local/global vars and add stack tests

eris.go

@@ -48,9 +48,11 @@ func wrap(err error, msg string) error {
 		return nil
 	}
 
-	// callers(4) skips this method, Wrap(f), stack.callers, and runtime.Callers
+	// callers(4) skips runtime.Callers, stack.callers, this method, and Wrap(f)
 	stack := callers(4)
-	frame := caller(4)
+	// caller(3) skips stack.caller, this method, and Wrap(f)
+	// caller(skip) has a slightly different meaning which is why it's not 4 as above
+	frame := caller(3)
 	switch e := err.(type) {
 	case *rootError:
 		if e.global {
@@ -60,6 +62,9 @@ func wrap(err error, msg string) error {
 				msg:    e.msg,
 				stack:  stack,
 			}
+		} else {
+			// insert the frame into the stack
+			e.stack.insertPC(*stack)
 		}
 	case *wrapError:
 		// insert the frame into the stack

eris_test.go

@@ -63,7 +63,7 @@ func TestErrorWrapping(t *testing.T) {
 			input:  []string{"additional context", "even more context"},
 			output: "external error: additional context: even more context",
 		},
-		"no error wrapping with a local root cause (eris.Errorf)": { // todo: also test globals with Errorf (wrapping included)
+		"no error wrapping with a local root cause (eris.Errorf)": {
 			cause:  eris.Errorf("%v root error", "formatted"),
 			output: "formatted root error",
 		},
@@ -260,40 +260,35 @@ func TestErrorFormatting(t *testing.T) {
 				t.Errorf("%v: expected { %v } got { %v }", desc, tc.output, err)
 			}
 
-			// todo: automate stack trace verification
 			_ = fmt.Sprintf("error formatting results (%v):\n", desc)
 			_ = fmt.Sprintf("%v\n", err)
 			_ = fmt.Sprintf("%+v", err)
 		})
 	}
 }
 
-func getFrames(frames []uintptr) []eris.StackFrame {
-	var sFrames []eris.StackFrame
-	for _, u := range frames {
-		pc := u - 1
-		fn := runtime.FuncForPC(pc)
-		if fn == nil {
-			frame := eris.StackFrame{
-				Name: "unknown",
-				File: "unknown",
-			}
-			sFrames = append(sFrames, frame)
-		}
-
-		name := fn.Name()
-		i := strings.LastIndex(name, "/")
-		name = name[i+1:]
-		file, line := fn.FileLine(pc)
+func getFrames(pc []uintptr) []eris.StackFrame {
+	var stackFrames []eris.StackFrame
+	if len(pc) == 0 {
+		return stackFrames
+	}
 
-		frame := eris.StackFrame{
+	frames := runtime.CallersFrames(pc)
+	for {
+		frame, more := frames.Next()
+		i := strings.LastIndex(frame.Function, "/")
+		name := frame.Function[i+1:]
+		stackFrames = append(stackFrames, eris.StackFrame{
 			Name: name,
-			File: file,
-			Line: line,
+			File: frame.File,
+			Line: frame.Line,
+		})
+		if !more {
+			break
 		}
-		sFrames = append(sFrames, frame)
 	}
-	return sFrames
+
+	return stackFrames
 }
 
 func TestStackFrames(t *testing.T) {
@@ -326,13 +321,14 @@ func TestStackFrames(t *testing.T) {
 			cause: nil,
 		},
 	}
+
 	for desc, tc := range tests {
 		t.Run(desc, func(t *testing.T) {
 			err := setupTestCase(false, tc.cause, tc.input)
 			uErr := eris.Unpack(err)
 			sFrames := eris.Stack(getFrames(eris.StackFrames(err)))
 			if !reflect.DeepEqual(uErr.ErrRoot.Stack, sFrames) {
-				t.Errorf("Stackframes() returned { %v }, was expecting { %v }", sFrames, uErr.ErrRoot.Stack)
+				t.Errorf("%v: expected { %v } got { %v }", desc, sFrames, uErr.ErrRoot.Stack)
 			}
 		})
 	}

examples/logging/example.go

@@ -12,17 +12,17 @@ import (
 
 var (
 	// global error values can be useful when wrapping errors or inspecting error types
-	ErrInternalServer = eris.New("error internal server")
+	errInternalServer = eris.New("error internal server")
 
 	// declaring an error with pkg/errors for comparison
-	ErrNotFound = errors.New("error not found")
+	errNotFound = errors.New("error not found")
 )
 
-type Request struct {
+type request struct {
 	ID string
 }
 
-func (req *Request) Validate() error {
+func (req *request) validate() error {
 	if req.ID == "" {
 		// create a new local error and wrap it with some context
 		err := eris.New("error bad request")
@@ -31,73 +31,73 @@ func (req *Request) Validate() error {
 	return nil
 }
 
-type Resource struct {
+type resource struct {
 	ID      string
 	AbsPath string
 }
 
-func GetResource(req Request) (*Resource, error) {
+func getResource(req request) (*resource, error) {
 	if req.ID == "res2" {
-		return &Resource{
+		return &resource{
 			ID:      req.ID,
 			AbsPath: "./some/malformed/absolute/path/data.json", // malformed absolute filepath to simulate a "bug"
 		}, nil
 	} else if req.ID == "res3" {
-		return &Resource{
+		return &resource{
 			ID:      req.ID,
 			AbsPath: "/some/correct/path/data.json",
 		}, nil
 	}
 
-	return nil, errors.Wrapf(ErrNotFound, "failed to get resource '%v'", req.ID)
+	return nil, errors.Wrapf(errNotFound, "failed to get resource '%v'", req.ID)
 }
 
-func GetRelPath(base string, path string) (string, error) {
+func getRelPath(base string, path string) (string, error) {
 	relPath, err := filepath.Rel(base, path)
 	if err != nil {
 		// it's generally useful to wrap external errors with a type that you know how to handle
 		// first (e.g. ErrInternalServer). this will help if/when you want to do error inspection
 		// via eris.Is(err, ErrInternalServer) or eris.Cause(err).
-		return "", eris.Wrap(ErrInternalServer, err.Error())
+		return "", eris.Wrap(errInternalServer, err.Error())
 	}
 	return relPath, nil
 }
 
-type Response struct {
+type response struct {
 	RelPath string
 }
 
-func ProcessResource(req Request) (*Response, error) {
-	if err := req.Validate(); err != nil {
+func processResource(req request) (*response, error) {
+	if err := req.validate(); err != nil {
 		// simply return the error if there's no additional context
 		return nil, err
 	}
 
-	resource, err := GetResource(req)
+	resource, err := getResource(req)
 	if err != nil {
 		return nil, err
 	}
 
 	// do some processing on the data
-	relPath, err := GetRelPath("/Users/roti/", resource.AbsPath)
+	relPath, err := getRelPath("/Users/roti/", resource.AbsPath)
 	if err != nil {
 		// wrap the error if you want to add more context
 		return nil, eris.Wrapf(err, "failed to get relative path for resource '%v'", resource.ID)
 	}
 
-	return &Response{
+	return &response{
 		RelPath: relPath,
 	}, nil
 }
 
-type LogReq struct {
+type logReq struct {
 	Method string
-	Req    Request
-	Res    *Response
+	Req    request
+	Res    *response
 	Err    error
 }
 
-func LogRequest(logger *logrus.Logger, logReq LogReq) {
+func logRequest(logger *logrus.Logger, logReq logReq) {
 	fields := logrus.Fields{
 		"method": logReq.Method,
 	}
@@ -123,7 +123,7 @@ func main() {
 	logger.SetFormatter(&logrus.JSONFormatter{})
 
 	// example requests
-	reqs := []Request{
+	reqs := []request{
 		{
 			ID: "", // bad request
 		},
@@ -140,10 +140,10 @@ func main() {
 
 	// process the example requests and log the results
 	for _, req := range reqs {
-		res, err := ProcessResource(req)
+		res, err := processResource(req)
 		if req.ID != "res1" {
 			// log the eris error
-			LogRequest(logger, LogReq{
+			logRequest(logger, logReq{
 				Method: "ProcessResource",
 				Req:    req,
 				Res:    res,

examples/sentry/example.go

@@ -15,20 +15,20 @@ func init() {
 	flag.StringVar(&dsn, "dsn", "", "Sentry DSN for logging stack traces")
 }
 
-func Example() error {
+func example() error {
 	return eris.New("test")
 }
 
-func WrapExample() error {
-	err := Example()
+func wrapExample() error {
+	err := example()
 	if err != nil {
 		return eris.Wrap(err, "wrap 1")
 	}
 	return nil
 }
 
-func WrapSecondExample() error {
-	err := WrapExample()
+func wrapSecondExample() error {
+	err := wrapExample()
 	if err != nil {
 		return eris.Wrap(err, "wrap 2")
 	}
@@ -41,12 +41,15 @@ func main() {
 		log.Fatal("Sentry DSN is a required flag, please pass it with '-dsn'")
 	}
 
-	err := WrapSecondExample()
+	err := wrapSecondExample()
 	err = eris.Wrap(err, "wrap 3")
 
-	sentry.Init(sentry.ClientOptions{
+	initErr := sentry.Init(sentry.ClientOptions{
 		Dsn: dsn,
 	})
+	if initErr != nil {
+		log.Fatalf("failed to initialize Sentry: %v", initErr)
+	}
 
 	sentry.CaptureException(err)
 	sentry.Flush(time.Second * 5)

stack.go

@@ -65,37 +65,27 @@ type frame uintptr
 
 // get returns a human readable stack frame.
 func (f frame) get() StackFrame {
-	frame := StackFrame{
-		Name: "unknown",
-		File: "unknown",
-	}
-
 	pc := uintptr(f) - 1
-	fn := runtime.FuncForPC(pc)
-	if fn != nil {
-		name := fn.Name()
-		i := strings.LastIndex(name, "/")
-		name = name[i+1:]
-		file, line := fn.FileLine(pc)
-
-		frame = StackFrame{
-			Name: name,
-			File: file,
-			Line: line,
-		}
-	}
+	frames := runtime.CallersFrames([]uintptr{pc})
+	frame, _ := frames.Next()
+
+	i := strings.LastIndex(frame.Function, "/")
+	name := frame.Function[i+1:]
 
-	return frame
+	return StackFrame{
+		Name: name,
+		File: frame.File,
+		Line: frame.Line,
+	}
 }
 
-// callers returns a stack trace. the argument skip is the number of stack frames to skip
-// before recording in pc, with 0 identifying the frame for Callers itself and 1 identifying
-// the caller of Callers.
+// callers returns a stack trace. the argument skip is the number of stack frames to skip before recording
+// in pc, with 0 identifying the frame for Callers itself and 1 identifying the caller of Callers.
 func callers(skip int) *stack {
 	const depth = 64
 	var pcs [depth]uintptr
 	n := runtime.Callers(skip, pcs[:])
-	var st stack = pcs[0 : n-2]
+	var st stack = pcs[0 : n-2] // todo: change this to filtering out runtime instead of hardcoding n-2
 	return &st
 }
 
@@ -104,13 +94,24 @@ type stack []uintptr
 
 // get returns a human readable stack trace.
 func (s *stack) get() []StackFrame {
-	var sFrames []StackFrame
-	for _, f := range *s {
-		frame := frame(f)
-		sFrame := frame.get()
-		sFrames = append(sFrames, sFrame)
+	var stackFrames []StackFrame
+
+	frames := runtime.CallersFrames(*s)
+	for {
+		frame, more := frames.Next()
+		i := strings.LastIndex(frame.Function, "/")
+		name := frame.Function[i+1:]
+		stackFrames = append(stackFrames, StackFrame{
+			Name: name,
+			File: frame.File,
+			Line: frame.Line,
+		})
+		if !more {
+			break
+		}
 	}
-	return sFrames
+
+	return stackFrames
 }
 
 // isGlobal determines if the stack trace represents a global error