Skip to content

Commit

Permalink
feat: single frame per error
Browse files Browse the repository at this point in the history
Instead of storing the entire call-stack per error value we now only store a single frame per error value.

This works much better in my goasync library as the call-stack is of course not goroutine friendly but by saving a single frame from the location of the error wrapping, this helps generate a pseudo call-stack across goroutine boundaries.

Technically this is a backwards incompatible change but no one is really using this apart from myself and I can't be bothered with the whole v3 branch thing right now.
  • Loading branch information
brad-jones committed Sep 11, 2020
1 parent 74debc2 commit 05da744
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 82 deletions.
26 changes: 10 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,21 @@ import (
)

// Simple re-useable error types can be defined like this.
// This is essentiually the same as "errors.New()" but creates a `*goerr.Error`.
// This is essentially the same as "errors.New()" but creates a `*goerr.Error`.
var errFoo = goerr.New("expecting 123456789")

func crash1(abc string) error {
if err := crash2(abc + "456"); err != nil {
// Use Trace anywhere you would normally return an error
// Use Wrap anywhere you would normally return an error
// This will both store stackframe information and wrap the error
return goerr.Trace(err)
return goerr.Wrap(err)
}
return nil
}

func crash2(abc string) error {
if err := crash3(abc + "7810"); err != nil {
return goerr.Trace(err)
return goerr.Wrap(err)
}
return nil
}
Expand All @@ -49,7 +49,7 @@ func crash3(abc string) error {
// Additional context messages can be added to the trace.
// These messages should be human friendly and when prefixed
// to the existing error message should read like a sentence.
return goerr.Trace(errFoo, "crash3 received "+abc)
return goerr.Wrap(errFoo, "crash3 received "+abc)
}
return nil
}
Expand All @@ -67,17 +67,11 @@ Running the above will output something similar to:
crash3 received 1234567810: expecting 123456789
main.crash3:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:32
return goerr.Trace(errFoo, "crash3 received "+abc)
main.crash2:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:21
if err := crash3(abc + "7810"); err != nil {
main.crash1:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:12
if err := crash2(abc + "456"); err != nil {
main.main:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:38
if err := crash1("123"); err != nil {
runtime.main:C:/Users/brad.jones/scoop/apps/go/current/src/runtime/proc.go:204
fn()
runtime.goexit:C:/Users/brad.jones/scoop/apps/go/current/src/runtime/asm_amd64.s:1374
BYTE $0x90 // NOP
return goerr.Wrap(errFoo, "crash3 received "+abc)
main.crash2:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:22
return goerr.Wrap(err)
main.crash1:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:15
return goerr.Wrap(err)
```

_Also see further working examples under: <https://github.com/brad-jones/goerr/tree/v2/examples>_
45 changes: 27 additions & 18 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ the error was encountered.
This package borrows (literally in some cases) from other similar packages:
https://github.com/palantir/stacktrace
https://github.com/go-errors/errors
https://github.com/pkg/errors
Expand All @@ -28,7 +30,7 @@ https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully
The difference is that this package has been built on top of the now standard
error wrapping support that was introduced in Go v1.13.
We can use Trace like this:
We can use Wrap like this:
package main
Expand All @@ -42,16 +44,16 @@ We can use Trace like this:
func crash1(abc string) error {
if err := crash2(abc + "456"); err != nil {
// Use Trace anywhere you would normally return an error
// Use Wrap anywhere you would normally return an error
// This will both store stackframe information and wrap the error
return goerr.Trace(err)
return goerr.Wrap(err)
}
return nil
}
func crash2(abc string) error {
if err := crash3(abc + "7810"); err != nil {
return goerr.Trace(err)
return goerr.Wrap(err)
}
return nil
}
Expand All @@ -61,7 +63,7 @@ We can use Trace like this:
// Additional context messages can be added to the trace.
// These messages should be human friendly and when prefixed
// to the existing error message should read like a sentence.
return goerr.Trace(errFoo, "crash3 received "+abc)
return goerr.Wrap(errFoo, "crash3 received "+abc)
}
return nil
}
Expand All @@ -77,17 +79,21 @@ And see output similar to:
crash3 received 1234567810: expecting 123456789
main.crash3:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:32
return goerr.Trace(errFoo, "crash3 received "+abc)
main.crash2:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:21
if err := crash3(abc + "7810"); err != nil {
main.crash1:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:12
if err := crash2(abc + "456"); err != nil {
main.main:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:38
if err := crash1("123"); err != nil {
runtime.main:C:/Users/brad.jones/scoop/apps/go/current/src/runtime/proc.go:204
fn()
runtime.goexit:C:/Users/brad.jones/scoop/apps/go/current/src/runtime/asm_amd64.s:1374
BYTE $0x90 // NOP
return goerr.Wrap(errFoo, "crash3 received "+abc)
main.crash2:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:22
return goerr.Wrap(err)
main.crash1:C:/Users/brad.jones/Projects/Personal/goerr/examples/simple/main.go:15
return goerr.Wrap(err)
Intent
To borrow from "palantir/stacktrace" the intent is not that we capture the exact
state of the stack when an error happens, including every function call. For a
library that does that, see github.com/go-errors/errors.
The intent here is to attach relevant contextual information (messages, variables)
at strategic places along the call stack, keeping stack traces compact and
maximally useful.
Check and Handle
Expand All @@ -102,7 +108,7 @@ Take the same example from the proposal that has been refactored to use goerr.
func CopyFile(src, dst string) (err error) {
defer Handle(func(e error){
err = Trace(e, fmt.Sprintf("failed to copy %s to %s", src, dst))
err = Wrap(e, fmt.Sprintf("failed to copy %s to %s", src, dst))
})
r, err := os.Open(src); Check(err)
Expand All @@ -112,7 +118,7 @@ Take the same example from the proposal that has been refactored to use goerr.
defer Handle(func(e error){
w.Close()
os.Remove(dst)
panic(e) // re-panic to make above handler set the err
Check(e) // re-panic to make above handler set the err
})
_, err = io.Copy(w, r); Check(err)
Expand All @@ -132,6 +138,9 @@ shouldn't!
Panicing doesn't work across goroutines for a start and this
https://go101.org/article/panic-and-recover-more.html
Although there is https://github.com/brad-jones/goasync which makes use of this
package to handle errors across goroutines (including panics), reasonable well.
I think where this can be really useful is when you say have a function like this:
func DoSomeWork() (err error) {
Expand Down
14 changes: 2 additions & 12 deletions examples/check-handle/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,12 @@ func TestCheckHandle(t *testing.T) {
if assert.NoError(t, err) {
assert.Equal(t,
[]string{
"crash1 failed because: we couldn't open the file: open /tmp/not-found/a9e5b8c7-13f6-4acc-a0c8-978319cb738b: no such file or directory",
"we couldn't open the file: open /tmp/not-found/a9e5b8c7-13f6-4acc-a0c8-978319cb738b: no such file or directory",
"",
"{",
" \"Err\": 2,",
" \"Op\": \"open\",",
" \"Path\": \"/tmp/not-found/a9e5b8c7-13f6-4acc-a0c8-978319cb738b\"",
"}",
"",
"main.crash1:/main.go:23",
"main.crash1:/main.go:21",
"\tgoerr.Check(err, \"we couldn't open the file\")",
"main.main:/main.go:13",
"\tif err := crash1(); err != nil {",
"runtime.main:/src/runtime/proc.go:204",
"\tfn()",
"runtime.goexit:/src/runtime/asm_amd64.s:1374",
"\tBYTE\t$0x90\t// NOP",
"",
"",
},
Expand Down
8 changes: 1 addition & 7 deletions examples/custom/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,7 @@ func TestCustom(t *testing.T) {
"}",
"",
"main.crash1:/main.go:19",
"\treturn goerr.Trace(&httpError{StatusCode: 500})",
"main.main:/main.go:23",
"\tif err := crash1(); err != nil {",
"runtime.main:/src/runtime/proc.go:204",
"\tfn()",
"runtime.goexit:/src/runtime/asm_amd64.s:1374",
"\tBYTE\t$0x90\t// NOP",
"\treturn goerr.Wrap(&httpError{StatusCode: 500})",
"",
"",
},
Expand Down
16 changes: 5 additions & 11 deletions examples/simple/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,11 @@ func TestSimple(t *testing.T) {
"crash3 received 1234567810: expecting 123456789",
"",
"main.crash3:/main.go:32",
"\treturn goerr.Trace(errFoo, \"crash3 received \"+abc)",
"main.crash2:/main.go:21",
"\tif err := crash3(abc + \"7810\"); err != nil {",
"main.crash1:/main.go:12",
"\tif err := crash2(abc + \"456\"); err != nil {",
"main.main:/main.go:38",
"\tif err := crash1(\"123\"); err != nil {",
"runtime.main:/src/runtime/proc.go:204",
"\tfn()",
"runtime.goexit:/src/runtime/asm_amd64.s:1374",
"\tBYTE\t$0x90\t// NOP",
"\treturn goerr.Wrap(errFoo, \"crash3 received \"+abc)",
"main.crash2:/main.go:22",
"\treturn goerr.Wrap(err)",
"main.crash1:/main.go:15",
"\treturn goerr.Wrap(err)",
"",
"",
},
Expand Down
7 changes: 0 additions & 7 deletions goerr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ func TestCause(t *testing.T) {
assert.Equal(t, e1, goerr.Cause(e3))
}

func TestCauseGoErr(t *testing.T) {
e1 := goerr.New("abc")
e2 := goerr.Wrap(e1)
e3 := goerr.Wrap(e2)
assert.Equal(t, goerr.Unwrap(e2), goerr.Cause(e3))
}

func TestIs(t *testing.T) {
e1 := goerr.New("abc")
e2 := goerr.Wrap(e1)
Expand Down
21 changes: 12 additions & 9 deletions stacktrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,17 @@ func NewStackTrace(err error) *StackTrace {
}
}

// Reverse the frames so we create a call stack in the expected manner.
// ie: from the error cause to the root of the program
// https://github.com/golang/go/wiki/SliceTricks#reversing
for i := len(frames)/2 - 1; i >= 0; i-- {
opp := len(frames) - 1 - i
frames[i], frames[opp] = frames[opp], frames[i]
if len(frames) > 0 {
// Reverse the frames so we create a call stack in the expected manner.
// ie: from the error cause to the root of the program
// https://github.com/golang/go/wiki/SliceTricks#reversing
for i := len(frames)/2 - 1; i >= 0; i-- {
opp := len(frames) - 1 - i
frames[i], frames[opp] = frames[opp], frames[i]
}
st.Stack = frames
}

st.Stack = frames
return st
}

Expand Down Expand Up @@ -104,10 +106,11 @@ func marshalError(err error) map[string]interface{} {
jS := string(j)
if strings.HasPrefix(jS, "{") && jS != "{}" {
var out map[string]interface{}
json.Unmarshal(j, &out)
if err := json.Unmarshal(j, &out); err != nil {
return nil
}
return out
}
}

return nil
}
4 changes: 2 additions & 2 deletions stacktrace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ func TestStackTraceNewFromGoErr(t *testing.T) {
traced := goerr.Wrap(err)
st := goerr.NewStackTrace(traced)
assert.Equal(t, traced, st.Error)
assert.Equal(t, goerr.Unwrap(traced), st.Cause)
assert.Equal(t, err.Unwrap(), st.Cause)
assert.Equal(t, "abc", st.ErrorMsg)
assert.Equal(t, 3, len(st.Stack))
assert.Equal(t, 1, len(st.Stack))
assert.Nil(t, st.ErrorCtx)
}

Expand Down

0 comments on commit 05da744

Please sign in to comment.