From 0a48e4f4d99e4a85129713d5f82a048bc2972528 Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 10:58:41 -0500 Subject: [PATCH 1/7] Add separate Unwrapper and Tracer functions --- client.go | 13 ++++- errors/go.mod | 2 + go.mod | 2 + rollbar.go | 41 ++++++++++++- rollbar_test.go | 151 +++++++++++++++++++++++++++++++++++------------- transforms.go | 38 ++---------- 6 files changed, 168 insertions(+), 79 deletions(-) diff --git a/client.go b/client.go index 57458b0..98e4cec 100644 --- a/client.go +++ b/client.go @@ -9,9 +9,9 @@ import ( "io" "net/http" "os" + "reflect" "regexp" "runtime" - "reflect" ) // A Client can be used to interact with Rollbar via the configured Transport. @@ -163,11 +163,15 @@ func (c *Client) SetTransform(transform func(map[string]interface{})) { c.configuration.transform = transform } +func (c *Client) SetUnwrapper(unwrapper UnwrapperFunc) { + c.configuration.unwrapper = unwrapper +} + // SetStackTracer sets the stackTracer function which is called to extract the stack // trace from enhanced error types. Return nil if no trace information is available. // Return true if the error type can be handled and false otherwise. // This feature can be used to add support for custom error type stack trace extraction. -func (c *Client) SetStackTracer(stackTracer func(err error) ([]runtime.Frame, bool)) { +func (c *Client) SetStackTracer(stackTracer StackTracerFunc) { c.configuration.stackTracer = stackTracer } @@ -605,7 +609,8 @@ type configuration struct { scrubFields *regexp.Regexp checkIgnore func(string) bool transform func(map[string]interface{}) - stackTracer func(error) ([]runtime.Frame, bool) + unwrapper UnwrapperFunc + stackTracer StackTracerFunc person Person captureIp captureIp } @@ -629,6 +634,8 @@ func createConfiguration(token, environment, codeVersion, serverHost, serverRoot fingerprint: false, checkIgnore: func(_s string) bool { return false }, transform: func(_d map[string]interface{}) {}, + unwrapper: DefaultUnwrapper, + stackTracer: DefaultStackTracer, person: Person{}, captureIp: CaptureIpFull, } diff --git a/errors/go.mod b/errors/go.mod index befc1a6..64f58c8 100644 --- a/errors/go.mod +++ b/errors/go.mod @@ -1,3 +1,5 @@ module github.com/rollbar/rollbar-go/errors +go 1.13 + require github.com/pkg/errors v0.8.1 diff --git a/go.mod b/go.mod index 075a9ce..ac50a32 100644 --- a/go.mod +++ b/go.mod @@ -1 +1,3 @@ module github.com/rollbar/rollbar-go + +go 1.13 diff --git a/rollbar.go b/rollbar.go index 8db60e8..f71401d 100644 --- a/rollbar.go +++ b/rollbar.go @@ -36,6 +36,35 @@ var ( nilErrTitle = "" ) +type UnwrapperFunc func(error) error +type StackTracerFunc func(error) ([]runtime.Frame, bool) + +func DefaultUnwrapper(err error) error { + type causer interface { + Cause() error + } + type wrapper interface { // matches the new Go 1.13 Unwrap() method, copied from xerrors + Unwrap() error + } + + if e, ok := err.(causer); ok { + return e.Cause() + } + if e, ok := err.(wrapper); ok { + return e.Unwrap() + } + + return nil +} + +func DefaultStackTracer(err error) ([]runtime.Frame, bool) { + if s, ok := err.(Stacker); ok { + return s.Stack(), true + } + + return nil, false +} + // SetEnabled sets whether or not the managed Client instance is enabled. // If this is true then this library works as normal. // If this is false then no calls will be made to the network. @@ -126,12 +155,16 @@ func SetTransform(transform func(map[string]interface{})) { std.SetTransform(transform) } +func SetUnwrapper(unwrapper UnwrapperFunc) { + std.SetUnwrapper(unwrapper) +} + // SetStackTracer sets the stackTracer function on the managed Client instance. // StackTracer is called to extract the stack trace from enhanced error types. // Return nil if no trace information is available. Return true if the error type // can be handled and false otherwise. // This feature can be used to add support for custom error type stack trace extraction. -func SetStackTracer(stackTracer func(err error) ([]runtime.Frame, bool)) { +func SetStackTracer(stackTracer StackTracerFunc) { std.SetStackTracer(stackTracer) } @@ -541,11 +574,15 @@ func LambdaWrapper(handlerFunc interface{}) interface{} { return std.LambdaWrapper(handlerFunc) } +type Stacker interface { + Stack() []runtime.Frame +} + // CauseStacker is an interface that errors can implement to create a trace_chain. // Callers are required to call runtime.Callers and build the runtime.Frame slice // on their own at the time the cause is wrapped. type CauseStacker interface { error Cause() error - Stack() []runtime.Frame + Stacker } diff --git a/rollbar_test.go b/rollbar_test.go index e2bc9fd..0e2ac01 100644 --- a/rollbar_test.go +++ b/rollbar_test.go @@ -45,7 +45,7 @@ func testErrorStackWithSkipGeneric2(s string) { func TestErrorClass(t *testing.T) { errors := map[string]error{ // generic error - "errors.errorString": fmt.Errorf("something is broken"), + "errors.errorString": fmt.Errorf("something is broken"), // custom error "rollbar.CustomError": &CustomError{"terrible mistakes were made"}, } @@ -394,6 +394,9 @@ type cs struct { stack []runtime.Frame } +var _ Stacker = cs{} +var _ CauseStacker = cs{} + func (cs cs) Cause() error { return cs.cause } @@ -402,58 +405,120 @@ func (cs cs) Stack() []runtime.Frame { return cs.stack } -func TestGetCauseOfStdErr(t *testing.T) { - if nil != getCause(fmt.Errorf("")) { - t.Error("cause should be nil for standard error") - } +type uw struct { + error + wrapped error } -func TestGetCauseOfCauseStacker(t *testing.T) { - cause := fmt.Errorf("cause") - effect := cs{fmt.Errorf("effect"), cause, nil} - if cause != getCause(effect) { - t.Error("effect should return cause") - } +func (uw uw) Unwrap() error { + return uw.wrapped } -func TestGetOrBuildStackOfStdErrWithoutParent(t *testing.T) { - err := cs{fmt.Errorf(""), nil, getCallersFrames(0)} - if nil == getOrBuildFrames(err, nil, 0, nil) { - t.Error("should build stack if parent is not a CauseStacker") - } +func TestDefaultUnwrapper(t *testing.T) { + t.Run("standard error", func(t *testing.T) { + if nil != DefaultUnwrapper(fmt.Errorf("")) { + t.Error("unwrapping a standard error should get nil") + } + }) + t.Run("unwrap", func(t *testing.T) { + wrapped := fmt.Errorf("wrapped") + parent := uw{fmt.Errorf("parent"), wrapped} + if wrapped != DefaultUnwrapper(parent) { + t.Error("parent should return wrapped") + } + }) + t.Run("CauseStacker", func(t *testing.T) { + cause := fmt.Errorf("cause") + effect := cs{fmt.Errorf("effect"), cause, nil} + if cause != DefaultUnwrapper(effect) { + t.Error("effect should return cause") + } + }) } -func TestGetOrBuildStackOfStdErrWithParent(t *testing.T) { - cause := fmt.Errorf("cause") - effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} - if 0 != len(getOrBuildFrames(cause, effect, 0, nil)) { - t.Error("should return empty stack of stadard error if parent is CauseStacker") - } +func TestDefaultStackTracer(t *testing.T) { + t.Run("standard error", func(t *testing.T) { + trace, ok := DefaultStackTracer(fmt.Errorf("standard error")) + if trace != nil { + t.Error("standard errors should not return a trace") + } + if ok { + t.Errorf("standard errors should not be handled") + } + }) + t.Run("Stacker", func(t *testing.T) { + trace := getCallersFrames(0) + err := cs{fmt.Errorf("cause"), nil, trace} + extractedTrace, ok := DefaultStackTracer(err) + if extractedTrace == nil { + t.Error("Stackers should return a trace") + } else if extractedTrace[0] != trace[0] { + t.Error("the trace from the error must be extracted") + } + if !ok { + t.Error("Stackers should be handled") + } + }) } -func TestGetOrBuildStackOfCauseStackerWithoutParent(t *testing.T) { - cause := fmt.Errorf("cause") - effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} - if len(effect.Stack()) == 0 { - t.Fatal("stack should not be empty") - } - if effect.Stack()[0] != getOrBuildFrames(effect, nil, 0, nil)[0] { - t.Error("should use stack from effect") - } -} +func TestGetOrBuildFrames(t *testing.T) { + // These tests all use the default stack tracer. The logic this is testing doesn't really + // depend on how the stack trace is extracted. -func TestGetOrBuildStackOfCauseStackerWithParent(t *testing.T) { - cause := fmt.Errorf("cause") - effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} - effect2 := cs{fmt.Errorf("effect2"), effect, getCallersFrames(0)} - if effect2.Stack()[0] != getOrBuildFrames(effect2, effect, 0, nil)[0] { - t.Error("should use stack from effect2") - } + t.Run("standard error without parent", func(t *testing.T) { + err := fmt.Errorf("") + trace := getOrBuildFrames(err, nil, 0, DefaultStackTracer) + if nil == trace { + t.Error("should build a new stack trace if error has no stack and parent is nil") + } + }) + t.Run("standard error with traceable parent", func(t *testing.T) { + cause := fmt.Errorf("cause") + effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} + if 0 != len(getOrBuildFrames(cause, effect, 0, DefaultStackTracer)) { + t.Error("should return empty stack of standard error if parent is traceable") + } + }) + t.Run("standard error with non-traceable parent", func(t *testing.T) { + child := fmt.Errorf("child") + parent := uw{fmt.Errorf("parent"), child} + trace := getOrBuildFrames(child, parent, 0, DefaultStackTracer) + if nil == trace { + t.Error("should build a new stack trace if parent is not traceable") + } + }) + t.Run("traceable error without parent", func(t *testing.T) { + cause := fmt.Errorf("cause") + effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} + if effect.Stack()[0] != getOrBuildFrames(effect, nil, 0, DefaultStackTracer)[0] { + t.Error("should use stack trace from effect") + } + }) + t.Run("traceable error with traceable parent", func(t *testing.T) { + cause := fmt.Errorf("cause") + effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} + effect2 := cs{fmt.Errorf("effect2"), effect, getCallersFrames(0)} + if effect.Stack()[0] != getOrBuildFrames(effect, effect2, 0, DefaultStackTracer)[0] { + t.Error("should use stack from child, not parent") + } + }) + t.Run("traceable error with non-traceable parent", func(t *testing.T) { + cause := fmt.Errorf("cause") + effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} + effect2 := uw{fmt.Errorf("effect2"), effect} + if effect.Stack()[0] != getOrBuildFrames(effect, effect2, 0, DefaultStackTracer)[0] { + t.Error("should use stack from child") + } + }) } func TestErrorBodyWithoutChain(t *testing.T) { err := fmt.Errorf("ERR") - errorBody, fingerprint := errorBody(configuration{fingerprint: true}, err, 0) + errorBody, fingerprint := errorBody(configuration{ + fingerprint: true, + unwrapper: DefaultUnwrapper, + stackTracer: DefaultStackTracer, + }, err, 0) if nil != errorBody["trace"] { t.Error("should not have trace element") } @@ -476,7 +541,11 @@ func TestErrorBodyWithChain(t *testing.T) { cause := fmt.Errorf("cause") effect := cs{fmt.Errorf("effect1"), cause, getCallersFrames(0)} effect2 := cs{fmt.Errorf("effect2"), effect, getCallersFrames(0)} - errorBody, fingerprint := errorBody(configuration{fingerprint: true}, effect2, 0) + errorBody, fingerprint := errorBody(configuration{ + fingerprint: true, + unwrapper: DefaultUnwrapper, + stackTracer: DefaultStackTracer, + }, effect2, 0) if nil != errorBody["trace"] { t.Error("should not have trace element") } diff --git a/transforms.go b/transforms.go index d999073..f262cbd 100644 --- a/transforms.go +++ b/transforms.go @@ -215,7 +215,7 @@ func errorBody(configuration configuration, err error, skip int) (map[string]int fingerprint = fingerprint + stack.Fingerprint() } parent = err - err = getCause(err) + err = configuration.unwrapper(err) if err == nil { break } @@ -239,44 +239,16 @@ func buildTrace(err error, stack stack) map[string]interface{} { } } -func getCause(err error) error { - type causer interface { - Cause() error - } - - if cs, ok := err.(causer); ok { - return cs.Cause() - } - return nil -} - // getOrBuildFrames gets stack frames from errors that provide one of their own // otherwise, it builds a new stack trace. It returns the stack frames if the error // is of a compatible type. If the error is not, but the parent error is, it assumes // the parent error will be processed later and therefore returns nil. -func getOrBuildFrames(err error, parent error, skip int, - tracer func(error) ([]runtime.Frame, bool)) []runtime.Frame { - - switch x := err.(type) { - case CauseStacker: - return x.Stack() - default: - if tracer != nil { - if st, ok := tracer(err); ok && st != nil { - return st - } - } +func getOrBuildFrames(err error, parent error, skip int, tracer StackTracerFunc) []runtime.Frame { + if st, ok := tracer(err); ok && st != nil { + return st } - - switch parent.(type) { - case CauseStacker: + if _, ok := tracer(parent); ok { return nil - default: - if tracer != nil { - if _, ok := tracer(parent); ok { - return nil - } - } } return getCallersFrames(1 + skip) From 5940f2dd112dbc74a841a3464430966585c53d1b Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 11:55:23 -0500 Subject: [PATCH 2/7] Add some documentation --- go.mod | 2 +- rollbar.go | 44 +++++++++++++++++++++++++++++++---- rollbar_set_unwrapper_test.go | 25 ++++++++++++++++++++ rollbar_test.go | 4 ++-- 4 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 rollbar_set_unwrapper_test.go diff --git a/go.mod b/go.mod index ac50a32..11d8f12 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ -module github.com/rollbar/rollbar-go +module github.com/wheniwork/rollbar-go go 1.13 diff --git a/rollbar.go b/rollbar.go index f71401d..90d52c1 100644 --- a/rollbar.go +++ b/rollbar.go @@ -36,9 +36,31 @@ var ( nilErrTitle = "" ) +// An UnwrapperFunc is used to extract wrapped errors when building an error chain. It should return +// the wrapped error if available, or nil otherwise. +// +// The client will use DefaultUnwrapperFunc by default, and a user can override the default behavior +// by calling SetUnwrapperFunc. See the documentation for DefaultUnwrapperFunc and SetUnwrapperFunc +// for more details. type UnwrapperFunc func(error) error + +// A StackTracerFunc is used to extract stack traces when building an error chain. The first return +// value should be the extracted stack trace, if available. The second return value should be +// whether the function was able to extract a stack trace (even if the extracted stack trace was +// empty or nil). +// +// The client will use DefaultStackTracerFunc by default, and a user can override the default +// behavior by calling SetStackTracerFunc. See the documentation for DefaultStackTracerFunc and +// SetStackTracerFunc for more details. type StackTracerFunc func(error) ([]runtime.Frame, bool) +// DefaultUnwrapper is the default UnwrapperFunc used by rollbar-go clients. It can unwrap any +// error types with the Unwrap method specified in Go 1.13, or any error type implementing the +// legacy CauseStacker interface. +// +// It also implicitly supports errors from github.com/pkg/errors. However, users of pkg/errors may +// wish to also use the stack trace extraction features provided in the +// github.com/rollbar/rollbar-go/errors package. func DefaultUnwrapper(err error) error { type causer interface { Cause() error @@ -57,6 +79,11 @@ func DefaultUnwrapper(err error) error { return nil } +// DefaultStackTracer is the default StackTracerFunc used by rollbar-go clients. It can extract +// stack traces from error types implementing the Stacker interface (and by extension, the legacy +// CauseStacker interface). +// +// To support stack trace extraction for other types of errors, see SetStackTracer. func DefaultStackTracer(err error) ([]runtime.Frame, bool) { if s, ok := err.(Stacker); ok { return s.Stack(), true @@ -155,15 +182,22 @@ func SetTransform(transform func(map[string]interface{})) { std.SetTransform(transform) } +// SetUnwrapper sets the UnwrapperFunc used by the managed Client instance. The unwrapper function +// is used to extract wrapped errors from enhanced error types. This feature can be used to add +// support for custom error types that do not yet implement the Unwrap method specified in Go 1.13. +// See the documentation of UnwrapperFunc for more details. +// +// In order to preserve the default unwrapping behavior, callers of SetUnwrapper may wish to include +// a call to DefaultUnwrapperFunc in their custom unwrapper function. See the provided example. func SetUnwrapper(unwrapper UnwrapperFunc) { std.SetUnwrapper(unwrapper) } -// SetStackTracer sets the stackTracer function on the managed Client instance. -// StackTracer is called to extract the stack trace from enhanced error types. -// Return nil if no trace information is available. Return true if the error type -// can be handled and false otherwise. -// This feature can be used to add support for custom error type stack trace extraction. +// SetStackTracer sets the StackTracerFunc used by the managed Client instance. The stack tracer +// function is used to extract the stack trace from enhanced error types. This feature can be used +// to add support for custom error types that do not implement the Stacker interface. +// +// See the documentation of StackTracerFunc for more details. func SetStackTracer(stackTracer StackTracerFunc) { std.SetStackTracer(stackTracer) } diff --git a/rollbar_set_unwrapper_test.go b/rollbar_set_unwrapper_test.go new file mode 100644 index 0000000..6ef59c6 --- /dev/null +++ b/rollbar_set_unwrapper_test.go @@ -0,0 +1,25 @@ +package rollbar + +type ExampleError struct { + error + wrapped error +} + +func (e ExampleError) GetWrappedError() error { + return e.wrapped +} + +func ExampleSetUnwrapper() { + SetUnwrapper(func(err error) error { + // preserve the default behavior for other types of errors + if unwrapped := DefaultUnwrapper(err); unwrapped != nil { + return unwrapped + } + + if ex, ok := err.(ExampleError); ok { + return ex.GetWrappedError() + } + + return nil + }) +} diff --git a/rollbar_test.go b/rollbar_test.go index 0e2ac01..2221d4c 100644 --- a/rollbar_test.go +++ b/rollbar_test.go @@ -475,8 +475,8 @@ func TestGetOrBuildFrames(t *testing.T) { t.Run("standard error with traceable parent", func(t *testing.T) { cause := fmt.Errorf("cause") effect := cs{fmt.Errorf("effect"), cause, getCallersFrames(0)} - if 0 != len(getOrBuildFrames(cause, effect, 0, DefaultStackTracer)) { - t.Error("should return empty stack of standard error if parent is traceable") + if nil != getOrBuildFrames(cause, effect, 0, DefaultStackTracer) { + t.Error("should return nil if child is not traceable but parent is") } }) t.Run("standard error with non-traceable parent", func(t *testing.T) { From 73306a9e3db4024840ca1eacd1fcb3d5b0e0f318 Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 12:27:37 -0500 Subject: [PATCH 3/7] Add more docs --- client.go | 19 +++++++++++++++---- go.mod | 2 +- rollbar.go | 20 ++++++++++---------- rollbar_set_stack_tracer_test.go | 32 ++++++++++++++++++++++++++++++++ rollbar_set_unwrapper_test.go | 15 +++++++++------ 5 files changed, 67 insertions(+), 21 deletions(-) create mode 100644 rollbar_set_stack_tracer_test.go diff --git a/client.go b/client.go index 98e4cec..283fca6 100644 --- a/client.go +++ b/client.go @@ -163,14 +163,25 @@ func (c *Client) SetTransform(transform func(map[string]interface{})) { c.configuration.transform = transform } +// SetUnwrapper sets the UnwrapperFunc used by the client. The unwrapper function +// is used to extract wrapped errors from enhanced error types. This feature can be used to add +// support for custom error types that do not yet implement the Unwrap method specified in Go 1.13. +// See the documentation of UnwrapperFunc for more details. +// +// In order to preserve the default unwrapping behavior, callers of SetUnwrapper may wish to include +// a call to DefaultUnwrapper in their custom unwrapper function. See the example on the SetUnwrapper function. func (c *Client) SetUnwrapper(unwrapper UnwrapperFunc) { c.configuration.unwrapper = unwrapper } -// SetStackTracer sets the stackTracer function which is called to extract the stack -// trace from enhanced error types. Return nil if no trace information is available. -// Return true if the error type can be handled and false otherwise. -// This feature can be used to add support for custom error type stack trace extraction. +// SetStackTracer sets the StackTracerFunc used by the client. The stack tracer +// function is used to extract the stack trace from enhanced error types. This feature can be used +// to add support for custom error types that do not implement the Stacker interface. +// See the documentation of StackTracerFunc for more details. +// +// In order to preserve the default stack tracing behavior, callers of SetStackTracer may wish +// to include a call to DefaultStackTracer in their custom tracing function. See the example +// on the SetStackTracer function. func (c *Client) SetStackTracer(stackTracer StackTracerFunc) { c.configuration.stackTracer = stackTracer } diff --git a/go.mod b/go.mod index 11d8f12..ac50a32 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ -module github.com/wheniwork/rollbar-go +module github.com/rollbar/rollbar-go go 1.13 diff --git a/rollbar.go b/rollbar.go index 90d52c1..fbfdac0 100644 --- a/rollbar.go +++ b/rollbar.go @@ -39,9 +39,8 @@ var ( // An UnwrapperFunc is used to extract wrapped errors when building an error chain. It should return // the wrapped error if available, or nil otherwise. // -// The client will use DefaultUnwrapperFunc by default, and a user can override the default behavior -// by calling SetUnwrapperFunc. See the documentation for DefaultUnwrapperFunc and SetUnwrapperFunc -// for more details. +// The client will use DefaultUnwrapper by default, and a user can override the default behavior +// by calling SetUnwrapper. See SetUnwrapper for more details. type UnwrapperFunc func(error) error // A StackTracerFunc is used to extract stack traces when building an error chain. The first return @@ -49,9 +48,8 @@ type UnwrapperFunc func(error) error // whether the function was able to extract a stack trace (even if the extracted stack trace was // empty or nil). // -// The client will use DefaultStackTracerFunc by default, and a user can override the default -// behavior by calling SetStackTracerFunc. See the documentation for DefaultStackTracerFunc and -// SetStackTracerFunc for more details. +// The client will use DefaultStackTracer by default, and a user can override the default +// behavior by calling SetStackTracer. See SetStackTracer for more details. type StackTracerFunc func(error) ([]runtime.Frame, bool) // DefaultUnwrapper is the default UnwrapperFunc used by rollbar-go clients. It can unwrap any @@ -61,7 +59,7 @@ type StackTracerFunc func(error) ([]runtime.Frame, bool) // It also implicitly supports errors from github.com/pkg/errors. However, users of pkg/errors may // wish to also use the stack trace extraction features provided in the // github.com/rollbar/rollbar-go/errors package. -func DefaultUnwrapper(err error) error { +var DefaultUnwrapper UnwrapperFunc = func(err error) error { type causer interface { Cause() error } @@ -84,7 +82,7 @@ func DefaultUnwrapper(err error) error { // CauseStacker interface). // // To support stack trace extraction for other types of errors, see SetStackTracer. -func DefaultStackTracer(err error) ([]runtime.Frame, bool) { +var DefaultStackTracer StackTracerFunc = func(err error) ([]runtime.Frame, bool) { if s, ok := err.(Stacker); ok { return s.Stack(), true } @@ -188,7 +186,7 @@ func SetTransform(transform func(map[string]interface{})) { // See the documentation of UnwrapperFunc for more details. // // In order to preserve the default unwrapping behavior, callers of SetUnwrapper may wish to include -// a call to DefaultUnwrapperFunc in their custom unwrapper function. See the provided example. +// a call to DefaultUnwrapper in their custom unwrapper function. See the provided example. func SetUnwrapper(unwrapper UnwrapperFunc) { std.SetUnwrapper(unwrapper) } @@ -196,8 +194,10 @@ func SetUnwrapper(unwrapper UnwrapperFunc) { // SetStackTracer sets the StackTracerFunc used by the managed Client instance. The stack tracer // function is used to extract the stack trace from enhanced error types. This feature can be used // to add support for custom error types that do not implement the Stacker interface. -// // See the documentation of StackTracerFunc for more details. +// +// In order to preserve the default stack tracing behavior, callers of SetStackTracer may wish +// to include a call to DefaultStackTracer in their custom tracing function. See the provided example. func SetStackTracer(stackTracer StackTracerFunc) { std.SetStackTracer(stackTracer) } diff --git a/rollbar_set_stack_tracer_test.go b/rollbar_set_stack_tracer_test.go new file mode 100644 index 0000000..1c4a4fd --- /dev/null +++ b/rollbar_set_stack_tracer_test.go @@ -0,0 +1,32 @@ +package rollbar_test + +import ( + "runtime" + + "github.com/rollbar/rollbar-go" +) + +type CustomTraceError struct { + error + trace []runtime.Frame +} + +func (e CustomTraceError) GetTrace() []runtime.Frame { + return e.trace +} + +func ExampleSetStackTracer() { + rollbar.SetStackTracer(func(err error) ([]runtime.Frame, bool) { + // preserve the default behavior for other types of errors + if trace, ok := rollbar.DefaultStackTracer(err); ok { + return trace, ok + } + + if cerr, ok := err.(CustomTraceError); ok { + return cerr.GetTrace(), true + } + + return nil, false + }) +} + diff --git a/rollbar_set_unwrapper_test.go b/rollbar_set_unwrapper_test.go index 6ef59c6..1333f4c 100644 --- a/rollbar_set_unwrapper_test.go +++ b/rollbar_set_unwrapper_test.go @@ -1,25 +1,28 @@ -package rollbar +package rollbar_test -type ExampleError struct { +import "github.com/rollbar/rollbar-go" + +type CustomWrappingError struct { error wrapped error } -func (e ExampleError) GetWrappedError() error { +func (e CustomWrappingError) GetWrappedError() error { return e.wrapped } func ExampleSetUnwrapper() { - SetUnwrapper(func(err error) error { + rollbar.SetUnwrapper(func(err error) error { // preserve the default behavior for other types of errors - if unwrapped := DefaultUnwrapper(err); unwrapped != nil { + if unwrapped := rollbar.DefaultUnwrapper(err); unwrapped != nil { return unwrapped } - if ex, ok := err.(ExampleError); ok { + if ex, ok := err.(CustomWrappingError); ok { return ex.GetWrappedError() } return nil }) } + From 8b6f732365c2d04f9b080774347068e614905cfd Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 12:29:10 -0500 Subject: [PATCH 4/7] Rename example files --- ...ck_tracer_test.go => rollbar_example_set_stack_tracer_test.go | 1 - ...et_unwrapper_test.go => rollbar_example_set_unwrapper_test.go | 1 - 2 files changed, 2 deletions(-) rename rollbar_set_stack_tracer_test.go => rollbar_example_set_stack_tracer_test.go (99%) rename rollbar_set_unwrapper_test.go => rollbar_example_set_unwrapper_test.go (99%) diff --git a/rollbar_set_stack_tracer_test.go b/rollbar_example_set_stack_tracer_test.go similarity index 99% rename from rollbar_set_stack_tracer_test.go rename to rollbar_example_set_stack_tracer_test.go index 1c4a4fd..711e364 100644 --- a/rollbar_set_stack_tracer_test.go +++ b/rollbar_example_set_stack_tracer_test.go @@ -29,4 +29,3 @@ func ExampleSetStackTracer() { return nil, false }) } - diff --git a/rollbar_set_unwrapper_test.go b/rollbar_example_set_unwrapper_test.go similarity index 99% rename from rollbar_set_unwrapper_test.go rename to rollbar_example_set_unwrapper_test.go index 1333f4c..effb3e7 100644 --- a/rollbar_set_unwrapper_test.go +++ b/rollbar_example_set_unwrapper_test.go @@ -25,4 +25,3 @@ func ExampleSetUnwrapper() { return nil }) } - From e4193bae3a8a128b780c437f1d0cbd6d1a09d210 Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 12:53:14 -0500 Subject: [PATCH 5/7] Update package docs and interface docs --- doc.go | 30 ++++++++++++++++++------------ rollbar.go | 8 ++++++-- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/doc.go b/doc.go index 304d524..466d1f0 100644 --- a/doc.go +++ b/doc.go @@ -3,6 +3,8 @@ Package rollbar is a Golang Rollbar client that makes it easy to report errors t Basic Usage +This package is designed to be used via the functions exposed at the root of the `rollbar` package. These work by managing a single instance of the `Client` type that is configurable via the setter functions at the root of the package. + package main import ( @@ -12,10 +14,10 @@ Basic Usage func main() { rollbar.SetToken("MY_TOKEN") - rollbar.SetEnvironment("production") // defaults to "development" - rollbar.SetCodeVersion("v2") // optional Git hash/branch/tag (required for GitHub integration) - rollbar.SetServerHost("web.1") // optional override; defaults to hostname - rollbar.SetServerRoot("/") // local path of project (required for GitHub integration and non-project stacktrace collapsing) + rollbar.SetEnvironment("production") // defaults to "development" + rollbar.SetCodeVersion("v2") // optional Git hash/branch/tag (required for GitHub integration) + rollbar.SetServerHost("web.1") // optional override; defaults to hostname + rollbar.SetServerRoot("/") // local path of project (required for GitHub integration and non-project stacktrace collapsing) rollbar.Info("Message body goes here") rollbar.WrapAndWait(doSomething) @@ -26,13 +28,12 @@ Basic Usage timer.Reset(10) // this will panic } - -This package is designed to be used via the functions exposed at the root of the `rollbar` package. These work by managing a single instance of the `Client` type that is configurable via the setter functions at the root of the package. - If you wish for more fine grained control over the client or you wish to have multiple independent clients then you can create and manage your own instances of the `Client` type. We provide two implementations of the `Transport` interface, `AsyncTransport` and `SyncTransport`. These manage the communication with the network layer. The Async version uses a buffered channel to communicate with the Rollbar API in a separate go routine. The Sync version is fully synchronous. It is possible to create your own `Transport` and configure a Client to use your preferred implementation. +Handling Panics + Go does not provide a mechanism for handling all panics automatically, therefore we provide two functions `Wrap` and `WrapAndWait` to make working with panics easier. They both take a function and then report to Rollbar if that function panics. They use the recover mechanism to capture the panic, and therefore if you wish your process to have the normal behaviour on panic (i.e. to crash), you will need to re-panic the result of calling `Wrap`. For example, package main @@ -60,15 +61,20 @@ Go does not provide a mechanism for handling all panics automatically, therefore The above pattern of calling `Wrap(...)` and then `Wait(...)` can be combined via `WrapAndWait(...)`. When `WrapAndWait(...)` returns if there was a panic it has already been sent to the Rollbar API. The error is still returned by this function if there is one. +Tracing Errors + +Due to the nature of the `error` type in Go, it can be difficult to attribute errors to their original origin without doing some extra work. To account for this, we provide multiple ways of configuring the client to unwrap errors and extract stack traces. -Due to the nature of the `error` type in Go, it can be difficult to attribute errors to their original origin without doing some extra work. To account for this, we define the interface `CauseStacker`: +The client will automatically unwrap any error type which implements the `Unwrap() error` method specified in Go 1.13. (See https://golang.org/pkg/errors/ for details.) This behavior can be extended for other types of errors by calling `SetUnwrapper`. - type CauseStacker interface { - error - Cause() error +For stack traces, we provide the `Stacker` interface, which can be implemented on custom error types: + + type Stacker interface { Stack() []runtime.Frame } -One can implement this interface for custom Error types to be able to build up a chain of stack traces. In order to get the correct stack, callers are required to call runtime.Callers and build the runtime.Frame slice on their own at the time the cause is wrapped. This is the least intrusive mechanism for gathering this information due to the decisions made by the Go runtime to not track this information. +If you cannot implement the `Stacker` interface on your error type (which is common for third-party error libraries), you can provide a custom tracing function by calling `SetStackTracer`. + +See the documentation of `SetUnwrapper` and `SetStackTracer` for more information and examples. */ package rollbar diff --git a/rollbar.go b/rollbar.go index fbfdac0..1fa734a 100644 --- a/rollbar.go +++ b/rollbar.go @@ -608,13 +608,17 @@ func LambdaWrapper(handlerFunc interface{}) interface{} { return std.LambdaWrapper(handlerFunc) } +// Stacker is an interface that errors can implement to allow the extraction of stack traces. +// To generate a stack trace, users are required to call runtime.Callers and build the runtime.Frame slice +// at the time the error is created. type Stacker interface { Stack() []runtime.Frame } // CauseStacker is an interface that errors can implement to create a trace_chain. -// Callers are required to call runtime.Callers and build the runtime.Frame slice -// on their own at the time the cause is wrapped. +// +// Deprecated: For unwrapping, use the `Unwrap() error` method specified in Go 1.13. (See https://golang.org/pkg/errors/ for more information). +// For stack traces, use the `Stacker` interface directly. type CauseStacker interface { error Cause() error From dc767d806409f5ddae41a5432ed7a0cd17fb6d40 Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 12:56:13 -0500 Subject: [PATCH 6/7] Add a blurb about pkg/errors --- doc.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc.go b/doc.go index 466d1f0..fb681c0 100644 --- a/doc.go +++ b/doc.go @@ -76,5 +76,7 @@ For stack traces, we provide the `Stacker` interface, which can be implemented o If you cannot implement the `Stacker` interface on your error type (which is common for third-party error libraries), you can provide a custom tracing function by calling `SetStackTracer`. See the documentation of `SetUnwrapper` and `SetStackTracer` for more information and examples. + +Finally, users of github.com/pkg/errors can use the utilities provided in the `errors` sub-package. */ package rollbar From 6f4ecdfff094e307cf380ad93dd4924a7256dc5f Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Mon, 23 Sep 2019 13:16:45 -0500 Subject: [PATCH 7/7] Add tests for setting things --- rollbar_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/rollbar_test.go b/rollbar_test.go index 2221d4c..0892094 100644 --- a/rollbar_test.go +++ b/rollbar_test.go @@ -570,3 +570,62 @@ func TestErrorBodyWithChain(t *testing.T) { t.Error("fingerprint should be the fingerprints in chain concatenated together. got: ", fingerprint) } } + +func TestSetUnwrapper(t *testing.T) { + type myCustomError struct { + error + wrapped error + } + + client := NewAsync("example", "test", "0.0.0", "", "") + child := fmt.Errorf("child") + parent := myCustomError{fmt.Errorf("parent"), child} + + if client.configuration.unwrapper(parent) != nil { + t.Fatal("bad test; default unwrapper must not recognize the custom error type") + } + + client.SetUnwrapper(func(err error) error { + if e, ok := err.(myCustomError); ok { + return e.wrapped + } + + return nil + }) + + if client.configuration.unwrapper(parent) != child { + t.Error("error did not unwrap correctly") + } +} + +func TestSetStackTracer(t *testing.T) { + type myCustomError struct { + error + trace []runtime.Frame + } + + client := NewAsync("example", "test", "0.0.0", "", "") + err := myCustomError{fmt.Errorf("some error"), getCallersFrames(0)} + + if trace, ok := client.configuration.stackTracer(err); ok || trace != nil { + t.Fatal("bad test; default stack tracer must not recognize the custom error type") + } + + client.SetStackTracer(func(err error) (frames []runtime.Frame, b bool) { + if e, ok := err.(myCustomError); ok { + return e.trace, true + } + + return nil, false + }) + + trace, ok := client.configuration.stackTracer(err) + if !ok { + t.Error("error was not handled by custom stack tracer") + } + if trace == nil { + t.Errorf("custom tracer failed to extract trace") + } else if trace[0] != err.trace[0] { + t.Errorf("custom tracer got the wrong trace") + } +}