From bf2fbe3085e4842364dadc33dae81d73e97b0986 Mon Sep 17 00:00:00 2001 From: Piotr Srebniak Date: Wed, 19 Dec 2018 23:04:50 +0100 Subject: [PATCH 1/3] implement error interface #391 --- error.go | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/error.go b/error.go index 574134857..edf613839 100644 --- a/error.go +++ b/error.go @@ -62,7 +62,7 @@ func (t *Tracer) NewError(err error) *Error { if err == nil { panic("NewError must be called with a non-nil error") } - e := t.newError() + e := t.newError(err) rand.Read(e.ID[:]) // ignore error, can't do anything about it initException(&e.exception, err) initStacktrace(e, err) @@ -80,7 +80,7 @@ func (t *Tracer) NewError(err error) *Error { // // If r.Message is empty, "[EMPTY]" will be used. func (t *Tracer) NewErrorLog(r ErrorLogRecord) *Error { - e := t.newError() + e := t.newError(r.Error) e.log = ErrorLogRecord{ Message: truncateString(r.Message), MessageFormat: truncateString(r.MessageFormat), @@ -100,7 +100,7 @@ func (t *Tracer) NewErrorLog(r ErrorLogRecord) *Error { } // newError returns a new Error associated with the Tracer. -func (t *Tracer) newError() *Error { +func (t *Tracer) newError(err error) *Error { e, _ := t.errorDataPool.Get().(*ErrorData) if e == nil { e = &ErrorData{ @@ -111,7 +111,10 @@ func (t *Tracer) newError() *Error { } } e.Timestamp = time.Now() - return &Error{ErrorData: e} + return &Error{ + ErrorData: e, + cause: err, + } } // Error describes an error occurring in the monitored service. @@ -119,6 +122,11 @@ type Error struct { // ErrorData holds the error data. This field is set to nil when // the error's Send method is called. *ErrorData + + // cause holds original error. + // It is accessible by Cause method + // https://godoc.org/github.com/pkg/errors#Cause + cause error } // ErrorData holds the details for an error, and is embedded inside Error. @@ -186,6 +194,22 @@ func (e *Error) SetTransaction(tx *Transaction) { tx.mu.RUnlock() } +// Cause returns original error assigned to Error +// https://godoc.org/github.com/pkg/errors#Cause +func (e *Error) Cause() error { + return e.cause +} + +// Error returns string message for error. +// if Error.Cause() returns nil, "[EMPTY]" will be used +func (e *Error) Error() string { + if e.cause != nil { + return e.cause.Error() + } + + return "[EMPTY]" +} + func (e *Error) setTransactionData(td *TransactionData) { e.TraceID = td.traceContext.Trace e.ParentID = td.traceContext.Span From b9f3c69257ad105e7b10304def78b9e9f2ff3c9b Mon Sep 17 00:00:00 2001 From: Piotr Srebniak Date: Mon, 24 Dec 2018 11:58:59 +0100 Subject: [PATCH 2/3] change captureError behaviour; add tests for Error --- error.go | 11 +++++++---- error_test.go | 35 ++++++++++++++++++++++++++++++++--- gocontext.go | 10 ++++++---- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/error.go b/error.go index edf613839..97124b56d 100644 --- a/error.go +++ b/error.go @@ -194,16 +194,19 @@ func (e *Error) SetTransaction(tx *Transaction) { tx.mu.RUnlock() } -// Cause returns original error assigned to Error +// Cause returns original error assigned to Error, nil if Error or Error.cause is nil. // https://godoc.org/github.com/pkg/errors#Cause func (e *Error) Cause() error { - return e.cause + if e != nil { + return e.cause + } + return nil } // Error returns string message for error. -// if Error.Cause() returns nil, "[EMPTY]" will be used +// if Error is nil or Error.Cause() returns nil, "[EMPTY]" will be used. func (e *Error) Error() string { - if e.cause != nil { + if e != nil && e.cause != nil { return e.cause.Error() } diff --git a/error_test.go b/error_test.go index b2ce7a28b..0c54e3889 100644 --- a/error_test.go +++ b/error_test.go @@ -102,12 +102,12 @@ func TestErrorAutoStackTraceReuse(t *testing.T) { func TestCaptureErrorNoTransaction(t *testing.T) { // When there's no transaction or span in the context, - // CaptureError returns nil as it has no tracer with + // CaptureError returns Error with nil ErrorData as it has no tracer with // which it can create the error. e := apm.CaptureError(context.Background(), errors.New("boom")) - assert.Nil(t, e) + assert.Nil(t, e.ErrorData) - // Send is a no-op on a nil Error. + // Send is a no-op on a Error with nil ErrorData. e.Send() } @@ -135,6 +135,35 @@ func TestErrorLogRecord(t *testing.T) { assert.Equal(t, "makeError", err0.Culprit) // based on exception stacktrace } +func TestErrorCauserInterface(t *testing.T) { + type Causer interface { + Cause() error + } + var e Causer = apm.CaptureError(context.Background(), errors.New("boom")) + assert.EqualError(t, e.Cause(), "boom") +} + +func TestErrorNilCauser(t *testing.T) { + var e *apm.Error + assert.Nil(t, e.Cause()) + + e = &apm.Error{} + assert.Nil(t, e.Cause()) +} + +func TestErrorErrorInterface(t *testing.T) { + var e error = apm.CaptureError(context.Background(), errors.New("boom")) + assert.EqualError(t, e, "boom") +} + +func TestErrorNilError(t *testing.T) { + var e *apm.Error + assert.EqualError(t, e, "[EMPTY]") + + e = &apm.Error{} + assert.EqualError(t, e, "[EMPTY]") +} + func makeError(msg string) error { return errors.New(msg) } diff --git a/gocontext.go b/gocontext.go index f4cbd6b5d..7dc485b36 100644 --- a/gocontext.go +++ b/gocontext.go @@ -64,13 +64,15 @@ func StartSpanOptions(ctx context.Context, name, spanType string, opts SpanOptio // set either from err, or from the caller. // // If there is no span or transaction in the context, CaptureError returns -// nil. As a convenience, if the provided error is nil, then CaptureError -// will also return nil. +// Error with nil ErrorData field. As a convenience, if the provided error is nil, +// then CaptureError will also return nil. func CaptureError(ctx context.Context, err error) *Error { if err == nil { return nil } - var e *Error + var e = &Error{ + cause: err, + } if span := SpanFromContext(ctx); span != nil { span.mu.RLock() if !span.ended() { @@ -86,7 +88,7 @@ func CaptureError(ctx context.Context, err error) *Error { } tx.mu.RUnlock() } - if e != nil { + if e.ErrorData != nil { e.Handled = true } return e From d743a05f7c5158c3e2885c6d5434c3ed5df77f73 Mon Sep 17 00:00:00 2001 From: Piotr Srebniak Date: Mon, 24 Dec 2018 12:09:32 +0100 Subject: [PATCH 3/3] fix Error.Cause doc --- error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/error.go b/error.go index 97124b56d..5cab1016f 100644 --- a/error.go +++ b/error.go @@ -204,7 +204,7 @@ func (e *Error) Cause() error { } // Error returns string message for error. -// if Error is nil or Error.Cause() returns nil, "[EMPTY]" will be used. +// if Error or Error.cause is nil, "[EMPTY]" will be used. func (e *Error) Error() string { if e != nil && e.cause != nil { return e.cause.Error()