From 06410e92ad734edb3334c60370a37d7c5f1ba181 Mon Sep 17 00:00:00 2001 From: Dave Cheney Date: Mon, 6 Jun 2016 23:01:57 +1000 Subject: [PATCH 1/2] Introduce Stacktrace and Frame This PR is a continuation of a series aimed at exposing the stack trace information embedded in each error value. The secondary effect is to deprecated the `Fprintf` helper. Taking cues from from @chrishines' `stack` package this PR introduces a new interface `Stacktrace() []Frame` and a `Frame` type, similar in function to the `runtime.Frame` type (although lacking its iterator type). Each `Frame` implemnts `fmt.Formatter` allowing it to print itself. The older `Location` interface is still supported but also deprecated. --- README.md | 13 ++++- errors.go | 103 +++++++++-------------------------- errors_test.go | 5 +- example_test.go | 16 ++++++ stack.go | 141 ++++++++++++++++++++++++++++++++++++++++++++++++ stack_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 325 insertions(+), 81 deletions(-) create mode 100644 stack.go create mode 100644 stack_test.go diff --git a/README.md b/README.md index f9acbd4..631f2cc 100644 --- a/README.md +++ b/README.md @@ -24,10 +24,19 @@ if err != nil { `New`, `Errorf`, `Wrap`, and `Wrapf` record a stack trace at the point they are invoked. This information can be retrieved with the following interface. ```go -type Stack interface { - Stack() []uintptr +type Stacktrace interface { + Stacktrace() []Frame } ``` +The `Frame` type represents a call site in the stacktrace. +`Frame` supports the `fmt.Formatter` interface that can be used for printing information about the stacktrace of this error. For example +``` +if err, ok := err.(Stacktrace); ok { + fmt.Printf("%+s:%d", err.Stacktrace()) +} +``` +See [the documentation for `Frame.Format`](https://godoc.org/github.com/pkg/errors#Frame_Format) for more details. + ## Retrieving the cause of an error Using `errors.Wrap` constructs a stack of errors, adding context to the preceding error. Depending on the nature of the error it may be necessary to recurse the operation of errors.Wrap to retrieve the original error for inspection. Any error value which implements this interface can be inspected by `errors.Cause`. diff --git a/errors.go b/errors.go index 3d515d8..fa30f02 100644 --- a/errors.go +++ b/errors.go @@ -21,15 +21,6 @@ // return errors.Wrap(err, "read failed") // } // -// Retrieving the stack trace of an error or wrapper -// -// New, Errorf, Wrap, and Wrapf record a stack trace at the point they are -// invoked. This information can be retrieved with the following interface. -// -// type Stack interface { -// Stack() []uintptr -// } -// // Retrieving the cause of an error // // Using errors.Wrap constructs a stack of errors, adding context to the @@ -51,25 +42,23 @@ // default: // // unknown error // } +// +// Retrieving the stack trace of an error or wrapper +// +// New, Errorf, Wrap, and Wrapf record a stack trace at the point they are +// invoked. This information can be retrieved with the following interface. +// +// type Stacktrace interface { +// Stacktrace() []Frame +// } package errors import ( "errors" "fmt" "io" - "runtime" - "strings" ) -// stack represents a stack of program counters. -type stack []uintptr - -func (s *stack) Stack() []uintptr { return *s } - -func (s *stack) Location() (string, int) { - return location((*s)[0] - 1) -} - // New returns an error that formats as the given text. func New(text string) error { return struct { @@ -167,7 +156,11 @@ func Cause(err error) error { // Fprint prints the error to the supplied writer. // If the error implements the Causer interface described in Cause // Print will recurse into the error's cause. -// If the error implements the inteface: +// If the error implements one of the following interfaces: +// +// type Stacktrace interface { +// Stacktrace() []Frame +// } // // type Location interface { // Location() (file string, line int) @@ -175,18 +168,29 @@ func Cause(err error) error { // // Print will also print the file and line of the error. // If err is nil, nothing is printed. +// +// Deprecated: Fprint will be removed in version 0.7. func Fprint(w io.Writer, err error) { type location interface { Location() (string, int) } + type stacktrace interface { + Stacktrace() []Frame + } type message interface { Message() string } for err != nil { - if err, ok := err.(location); ok { + switch err := err.(type) { + case stacktrace: + frame := err.Stacktrace()[0] + fmt.Fprintf(w, "%+s:%d: ", frame, frame) + case location: file, line := err.Location() fmt.Fprintf(w, "%s:%d: ", file, line) + default: + // de nada } switch err := err.(type) { case message: @@ -202,58 +206,3 @@ func Fprint(w io.Writer, err error) { err = cause.Cause() } } - -func callers() *stack { - const depth = 32 - var pcs [depth]uintptr - n := runtime.Callers(3, pcs[:]) - var st stack = pcs[0:n] - return &st -} - -// location returns the source file and line matching pc. -func location(pc uintptr) (string, int) { - fn := runtime.FuncForPC(pc) - if fn == nil { - return "unknown", 0 - } - - // Here we want to get the source file path relative to the compile time - // GOPATH. As of Go 1.6.x there is no direct way to know the compiled - // GOPATH at runtime, but we can infer the number of path segments in the - // GOPATH. We note that fn.Name() returns the function name qualified by - // the import path, which does not include the GOPATH. Thus we can trim - // segments from the beginning of the file path until the number of path - // separators remaining is one more than the number of path separators in - // the function name. For example, given: - // - // GOPATH /home/user - // file /home/user/src/pkg/sub/file.go - // fn.Name() pkg/sub.Type.Method - // - // We want to produce: - // - // pkg/sub/file.go - // - // From this we can easily see that fn.Name() has one less path separator - // than our desired output. We count separators from the end of the file - // path until it finds two more than in the function name and then move - // one character forward to preserve the initial path segment without a - // leading separator. - const sep = "/" - goal := strings.Count(fn.Name(), sep) + 2 - file, line := fn.FileLine(pc) - i := len(file) - for n := 0; n < goal; n++ { - i = strings.LastIndex(file[:i], sep) - if i == -1 { - // not enough separators found, set i so that the slice expression - // below leaves file unmodified - i = -len(sep) - break - } - } - // get back to 0 or trim the leading separator - file = file[i+len(sep):] - return file, line -} diff --git a/errors_test.go b/errors_test.go index 2795bb2..8238a85 100644 --- a/errors_test.go +++ b/errors_test.go @@ -102,7 +102,7 @@ func TestCause(t *testing.T) { } } -func TestFprint(t *testing.T) { +func TestFprintError(t *testing.T) { x := New("error") tests := []struct { err error @@ -234,7 +234,8 @@ func TestStack(t *testing.T) { } st := x.Stack() for i, want := range tt.want { - file, line := location(st[i] - 1) + frame := Frame(st[i]) + file, line := fmt.Sprintf("%+s", frame), frame.line() if file != want.file || line != want.line { t.Errorf("frame %d: expected %s:%d, got %s:%d", i, want.file, want.line, file, line) } diff --git a/example_test.go b/example_test.go index c5f2023..c763caa 100644 --- a/example_test.go +++ b/example_test.go @@ -69,3 +69,19 @@ func ExampleErrorf() { // Output: github.com/pkg/errors/example_test.go:67: whoops: foo } + +func ExampleError_Stacktrace() { + type Stacktrace interface { + Stacktrace() []errors.Frame + } + + err, ok := errors.Cause(fn()).(Stacktrace) + if !ok { + panic("oops, err does not implement Stacktrace") + } + + st := err.Stacktrace() + fmt.Printf("%+v", st[0:2]) // top two framces + + // Output: [github.com/pkg/errors/example_test.go:33 github.com/pkg/errors/example_test.go:78] +} diff --git a/stack.go b/stack.go new file mode 100644 index 0000000..398ec8d --- /dev/null +++ b/stack.go @@ -0,0 +1,141 @@ +package errors + +import ( + "fmt" + "io" + "path" + "runtime" + "strings" +) + +// Frame repesents an activation record. +type Frame uintptr + +// pc returns the program counter for this frame; +// multiple frames may have the same PC value. +func (f Frame) pc() uintptr { return uintptr(f) - 1 } + +// file returns the full path to the file that contains the +// function for this Frame's pc. +func (f Frame) file() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + file, _ := fn.FileLine(f.pc()) + return file +} + +// line returns the line number of source code of the +// function for this Frame's pc. +func (f Frame) line() int { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return 0 + } + _, line := fn.FileLine(f.pc()) + return line +} + +// Format formats the frame according to the fmt.Formatter interface. +// +// %s source file +// %d source line +// %n function name +// %v equivalent to %s:%d +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+s path of source file relative to the compile time GOPATH +func (f Frame) Format(s fmt.State, verb rune) { + switch verb { + case 's': + switch { + case s.Flag('+'): + pc := f.pc() + fn := runtime.FuncForPC(pc) + io.WriteString(s, trimGOPATH(fn, pc)) + default: + io.WriteString(s, path.Base(f.file())) + } + case 'd': + fmt.Fprintf(s, "%d", f.line()) + case 'n': + name := runtime.FuncForPC(f.pc()).Name() + i := strings.LastIndex(name, ".") + io.WriteString(s, name[i+1:]) + case 'v': + f.Format(s, 's') + io.WriteString(s, ":") + f.Format(s, 'd') + } +} + +// stack represents a stack of program counters. +type stack []uintptr + +// Deprecated: use Stacktrace() +func (s *stack) Stack() []uintptr { return *s } + +// Deprecated: use Stacktrace()[0] +func (s *stack) Location() (string, int) { + frame := s.Stacktrace()[0] + return fmt.Sprintf("%+s", frame), frame.line() +} + +func (s *stack) Stacktrace() []Frame { + f := make([]Frame, len(*s)) + for i := 0; i < len(f); i++ { + f[i] = Frame((*s)[i]) + } + return f +} + +func callers() *stack { + const depth = 32 + var pcs [depth]uintptr + n := runtime.Callers(3, pcs[:]) + var st stack = pcs[0:n] + return &st +} + +func trimGOPATH(fn *runtime.Func, pc uintptr) string { + // Here we want to get the source file path relative to the compile time + // GOPATH. As of Go 1.6.x there is no direct way to know the compiled + // GOPATH at runtime, but we can infer the number of path segments in the + // GOPATH. We note that fn.Name() returns the function name qualified by + // the import path, which does not include the GOPATH. Thus we can trim + // segments from the beginning of the file path until the number of path + // separators remaining is one more than the number of path separators in + // the function name. For example, given: + // + // GOPATH /home/user + // file /home/user/src/pkg/sub/file.go + // fn.Name() pkg/sub.Type.Method + // + // We want to produce: + // + // pkg/sub/file.go + // + // From this we can easily see that fn.Name() has one less path separator + // than our desired output. We count separators from the end of the file + // path until it finds two more than in the function name and then move + // one character forward to preserve the initial path segment without a + // leading separator. + const sep = "/" + goal := strings.Count(fn.Name(), sep) + 2 + file, _ := fn.FileLine(pc) + i := len(file) + for n := 0; n < goal; n++ { + i = strings.LastIndex(file[:i], sep) + if i == -1 { + // not enough separators found, set i so that the slice expression + // below leaves file unmodified + i = -len(sep) + break + } + } + // get back to 0 or trim the leading separator + file = file[i+len(sep):] + return file +} diff --git a/stack_test.go b/stack_test.go new file mode 100644 index 0000000..b4c82e7 --- /dev/null +++ b/stack_test.go @@ -0,0 +1,128 @@ +package errors + +import ( + "fmt" + "runtime" + "testing" +) + +var line, _, _, _ = runtime.Caller(0) + +func TestFrameLine(t *testing.T) { + var tests = []struct { + Frame + want int + }{{ + Frame(line), + 9, + }, { + func() Frame { + var line, _, _, _ = runtime.Caller(0) + return Frame(line) + }(), + 20, + }, { + func() Frame { + var line, _, _, _ = runtime.Caller(1) + return Frame(line) + }(), + 28, + }, { + Frame(0), // invalid PC + 0, + }} + + for _, tt := range tests { + got := tt.Frame.line() + want := tt.want + if want != got { + t.Errorf("Frame(%v): want: %v, got: %v", uintptr(tt.Frame), want, got) + } + } +} + +func TestStackLocation(t *testing.T) { + st := func() *stack { + var pcs [32]uintptr + n := runtime.Callers(1, pcs[:]) + var st stack = pcs[0:n] + return &st + }() + file, line := st.Location() + wfile, wline := "github.com/pkg/errors/stack_test.go", 47 + if file != wfile || line != wline { + t.Errorf("stack.Location(): want %q %d, got %q %d", wfile, wline, file, line) + } +} + +func TestFrameFormat(t *testing.T) { + var tests = []struct { + Frame + format string + want string + }{{ + Frame(line), + "%s", + "stack_test.go", + }, { + Frame(line), + "%+s", + "github.com/pkg/errors/stack_test.go", + }, { + Frame(0), + "%s", + "unknown", + }, { + Frame(line), + "%d", + "9", + }, { + Frame(0), + "%d", + "0", + }, { + Frame(line), + "%n", + "init", + }, { + Frame(0), + "%n", + "", + }, { + Frame(line), + "%v", + "stack_test.go:9", + }, { + Frame(0), + "%v", + "unknown:0", + }} + + for _, tt := range tests { + got := fmt.Sprintf(tt.format, tt.Frame) + want := tt.want + if want != got { + t.Errorf("%v %q: want: %q, got: %q", tt.Frame, tt.format, want, got) + } + } +} + +func TestTrimGOPATH(t *testing.T) { + var tests = []struct { + Frame + want string + }{{ + Frame(line), + "github.com/pkg/errors/stack_test.go", + }} + + for _, tt := range tests { + pc := tt.Frame.pc() + fn := runtime.FuncForPC(pc) + got := trimGOPATH(fn, pc) + want := tt.want + if want != got { + t.Errorf("%v: want %q, got %q", tt.Frame, want, got) + } + } +} From a4b4f758c568faafabe9cc9db134f6cd509c3c1c Mon Sep 17 00:00:00 2001 From: Dave Cheney Date: Tue, 7 Jun 2016 14:00:33 +1000 Subject: [PATCH 2/2] Responding to review feedback. - Handle possible nil in `runtime.FuncForPC` - Refactor trimGOPATH to work on strings, not funcs and pc's - Fix incorrect handling of `%n` for methods. --- stack.go | 17 +++++++++----- stack_test.go | 61 ++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/stack.go b/stack.go index 398ec8d..fd54243 100644 --- a/stack.go +++ b/stack.go @@ -47,6 +47,7 @@ func (f Frame) line() int { // Format accepts flags that alter the printing of some verbs, as follows: // // %+s path of source file relative to the compile time GOPATH +// %+v equivalent to %+s:%d func (f Frame) Format(s fmt.State, verb rune) { switch verb { case 's': @@ -54,7 +55,12 @@ func (f Frame) Format(s fmt.State, verb rune) { case s.Flag('+'): pc := f.pc() fn := runtime.FuncForPC(pc) - io.WriteString(s, trimGOPATH(fn, pc)) + if fn == nil { + io.WriteString(s, "unknown") + } else { + file, _ := fn.FileLine(pc) + io.WriteString(s, trimGOPATH(fn.Name(), file)) + } default: io.WriteString(s, path.Base(f.file())) } @@ -62,7 +68,9 @@ func (f Frame) Format(s fmt.State, verb rune) { fmt.Fprintf(s, "%d", f.line()) case 'n': name := runtime.FuncForPC(f.pc()).Name() - i := strings.LastIndex(name, ".") + i := strings.LastIndex(name, "/") + name = name[i+1:] + i = strings.Index(name, ".") io.WriteString(s, name[i+1:]) case 'v': f.Format(s, 's') @@ -99,7 +107,7 @@ func callers() *stack { return &st } -func trimGOPATH(fn *runtime.Func, pc uintptr) string { +func trimGOPATH(name, file string) string { // Here we want to get the source file path relative to the compile time // GOPATH. As of Go 1.6.x there is no direct way to know the compiled // GOPATH at runtime, but we can infer the number of path segments in the @@ -123,8 +131,7 @@ func trimGOPATH(fn *runtime.Func, pc uintptr) string { // one character forward to preserve the initial path segment without a // leading separator. const sep = "/" - goal := strings.Count(fn.Name(), sep) + 2 - file, _ := fn.FileLine(pc) + goal := strings.Count(name, sep) + 2 i := len(file) for n := 0; n < goal; n++ { i = strings.LastIndex(file[:i], sep) diff --git a/stack_test.go b/stack_test.go index b4c82e7..c9a4d4c 100644 --- a/stack_test.go +++ b/stack_test.go @@ -6,25 +6,25 @@ import ( "testing" ) -var line, _, _, _ = runtime.Caller(0) +var initpc, _, _, _ = runtime.Caller(0) func TestFrameLine(t *testing.T) { var tests = []struct { Frame want int }{{ - Frame(line), + Frame(initpc), 9, }, { func() Frame { - var line, _, _, _ = runtime.Caller(0) - return Frame(line) + var pc, _, _, _ = runtime.Caller(0) + return Frame(pc) }(), 20, }, { func() Frame { - var line, _, _, _ = runtime.Caller(1) - return Frame(line) + var pc, _, _, _ = runtime.Caller(1) + return Frame(pc) }(), 28, }, { @@ -55,17 +55,29 @@ func TestStackLocation(t *testing.T) { } } +type X struct{} + +func (x X) val() Frame { + var pc, _, _, _ = runtime.Caller(0) + return Frame(pc) +} + +func (x *X) ptr() Frame { + var pc, _, _, _ = runtime.Caller(0) + return Frame(pc) +} + func TestFrameFormat(t *testing.T) { var tests = []struct { Frame format string want string }{{ - Frame(line), + Frame(initpc), "%s", "stack_test.go", }, { - Frame(line), + Frame(initpc), "%+s", "github.com/pkg/errors/stack_test.go", }, { @@ -73,7 +85,11 @@ func TestFrameFormat(t *testing.T) { "%s", "unknown", }, { - Frame(line), + Frame(0), + "%+s", + "unknown", + }, { + Frame(initpc), "%d", "9", }, { @@ -81,17 +97,35 @@ func TestFrameFormat(t *testing.T) { "%d", "0", }, { - Frame(line), + Frame(initpc), "%n", "init", + }, { + func() Frame { + var x X + return x.ptr() + }(), + "%n", + "(*X).ptr", + }, { + func() Frame { + var x X + return x.val() + }(), + "%n", + "X.val", }, { Frame(0), "%n", "", }, { - Frame(line), + Frame(initpc), "%v", "stack_test.go:9", + }, { + Frame(initpc), + "%+v", + "github.com/pkg/errors/stack_test.go:9", }, { Frame(0), "%v", @@ -112,14 +146,15 @@ func TestTrimGOPATH(t *testing.T) { Frame want string }{{ - Frame(line), + Frame(initpc), "github.com/pkg/errors/stack_test.go", }} for _, tt := range tests { pc := tt.Frame.pc() fn := runtime.FuncForPC(pc) - got := trimGOPATH(fn, pc) + file, _ := fn.FileLine(pc) + got := trimGOPATH(fn.Name(), file) want := tt.want if want != got { t.Errorf("%v: want %q, got %q", tt.Frame, want, got)