From c03711a4d75563cc476b83685fe174008266ba28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Ribi=C4=87?= Date: Tue, 26 Mar 2024 18:54:39 +0100 Subject: [PATCH] use SetException in logrus --- CHANGELOG.md | 4 + interfaces.go | 5 +- logrus/logrusentry.go | 68 +-------------- logrus/logrusentry_test.go | 168 ------------------------------------- 4 files changed, 8 insertions(+), 237 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45d27a4e6..5951037e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ - Add `Fiber` integration ([#795](https://github.com/getsentry/sentry-go/pull/795)) - Use `errors.Unwrap()` to create exception groups ([#792](https://github.com/getsentry/sentry-go/pull/792)) +### Fixes + +- Fix missing stack trace for parsing error in logrusentry ([#689](https://github.com/getsentry/sentry-go/pull/689)) + ## 0.27.0 The Sentry SDK team is happy to announce the immediate availability of Sentry Go SDK v0.27.0. diff --git a/interfaces.go b/interfaces.go index d57f753a8..689c11578 100644 --- a/interfaces.go +++ b/interfaces.go @@ -341,7 +341,8 @@ type Event struct { // SetException appends the unwrapped errors to the event's exception list. // // maxErrorDepth is the maximum depth of the error chain we will look -// into while unwrapping the errors. +// into while unwrapping the errors. If maxErrorDepth is -1, we will +// unwrap all errors in the chain. func (e *Event) SetException(exception error, maxErrorDepth int) { if exception == nil { return @@ -349,7 +350,7 @@ func (e *Event) SetException(exception error, maxErrorDepth int) { err := exception - for i := 0; err != nil && i < maxErrorDepth; i++ { + for i := 0; err != nil && (i < maxErrorDepth || maxErrorDepth == -1); i++ { // Add the current error to the exception slice with its details e.Exception = append(e.Exception, Exception{ Value: err.Error(), diff --git a/logrus/logrusentry.go b/logrus/logrusentry.go index c4f733a15..f9d921651 100644 --- a/logrus/logrusentry.go +++ b/logrus/logrusentry.go @@ -4,7 +4,6 @@ package sentrylogrus import ( "errors" "net/http" - "reflect" "time" "github.com/sirupsen/logrus" @@ -162,8 +161,7 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event { } if err, ok := s.Extra[logrus.ErrorKey].(error); ok { delete(s.Extra, logrus.ErrorKey) - ex := h.exceptions(err) - s.Exception = ex + s.SetException(err, -1) } key = h.key(FieldUser) if user, ok := s.Extra[key].(sentry.User); ok { @@ -189,70 +187,6 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event { return s } -func (h *Hook) exceptions(err error) []sentry.Exception { - if err == nil { - return nil - } - - if !h.hub.Client().Options().AttachStacktrace { - return []sentry.Exception{{ - Type: reflect.TypeOf(err).String(), - Value: err.Error(), - }} - } - - var excs []sentry.Exception - for err != nil { - // Add the current error to the exception slice with its details - excs = append(excs, sentry.Exception{ - Value: err.Error(), - Type: reflect.TypeOf(err).String(), - Stacktrace: sentry.ExtractStacktrace(err), - }) - - // Attempt to unwrap the error using the standard library's Unwrap method. - // If errors.Unwrap returns nil, it means either there is no error to unwrap, - // or the error does not implement the Unwrap method. - unwrappedErr := errors.Unwrap(err) - - if unwrappedErr == nil { - break - } - - err = unwrappedErr - } - - // Add a trace of the current stack to the most recent error in a chain if - // it doesn't have a stack trace yet. - if excs[0].Stacktrace == nil { - excs[0].Stacktrace = sentry.NewStacktrace() - } - - if len(excs) <= 1 { - return excs - } - - // reverse - for i, j := 0, len(excs)-1; i < j; i, j = i+1, j-1 { - excs[i], excs[j] = excs[j], excs[i] - } - - for i := range excs { - excs[i].Mechanism = &sentry.Mechanism{ - Data: map[string]any{ - "is_exception_group": true, - "exception_id": i, - }, - } - if i == 0 { - continue - } - excs[i].Mechanism.Data["parent_id"] = i - 1 - } - - return excs -} - // Flush waits until the underlying Sentry transport sends any buffered events, // blocking for at most the given timeout. It returns false if the timeout was // reached, in which case some events may not have been sent. diff --git a/logrus/logrusentry_test.go b/logrus/logrusentry_test.go index 0ff8ac583..1dbe6f47c 100644 --- a/logrus/logrusentry_test.go +++ b/logrus/logrusentry_test.go @@ -2,7 +2,6 @@ package sentrylogrus import ( "errors" - "fmt" "net/http" "net/http/httptest" "strings" @@ -267,170 +266,3 @@ func Test_entryToEvent(t *testing.T) { }) } } - -func Test_exceptions(t *testing.T) { - t.Parallel() - tests := map[string]struct { - trace bool - err error - want []sentry.Exception - }{ - "error is nil": { - trace: true, - err: nil, - want: nil, - }, - "std error": { - trace: true, - err: errors.New("foo"), - want: []sentry.Exception{ - { - Type: "*errors.errorString", - Value: "foo", - Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}, - }, - }, - }, - "wrapped error": { - trace: true, - err: fmt.Errorf("foo: %w", errors.New("bar")), - want: []sentry.Exception{ - { - Type: "*errors.errorString", - Value: "bar", - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 0, - "is_exception_group": true, - }, - }, - }, - { - Type: "*fmt.wrapError", - Value: "foo: bar", - Stacktrace: &sentry.Stacktrace{ - Frames: []sentry.Frame{}, - }, - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 1, - "is_exception_group": true, - "parent_id": 0, - }, - }, - }, - }, - }, - "missing stack for pkgerr": { - trace: false, - err: pkgerr.New("foo"), - want: []sentry.Exception{ - {Type: "*errors.fundamental", Value: "foo"}, - }, - }, - "stack": { - trace: true, - err: pkgerr.New("foo"), - want: []sentry.Exception{ - {Type: "*errors.fundamental", Value: "foo", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}}, - }, - }, - "multi-wrapped error": { - trace: true, - err: func() error { - err := errors.New("original") - err = fmt.Errorf("fmt: %w", err) - err = pkgerr.Wrap(err, "wrap") - err = pkgerr.WithStack(err) - return fmt.Errorf("wrapped: %w", err) - }(), - want: []sentry.Exception{ - { - Type: "*errors.errorString", - Value: "original", - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 0, - "is_exception_group": true, - }, - }, - }, - { - Type: "*fmt.wrapError", - Value: "fmt: original", - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 1, - "is_exception_group": true, - "parent_id": 0, - }, - }, - }, - { - Type: "*errors.withMessage", - Value: "wrap: fmt: original", - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 2, - "is_exception_group": true, - "parent_id": 1, - }, - }, - }, - { - Type: "*errors.withStack", - Value: "wrap: fmt: original", - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 3, - "is_exception_group": true, - "parent_id": 2, - }, - }, - Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}, - }, - { - Type: "*errors.withStack", - Value: "wrap: fmt: original", - Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}, - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 4, - "is_exception_group": true, - "parent_id": 3, - }, - }, - }, - { - Type: "*fmt.wrapError", - Value: "wrapped: wrap: fmt: original", - Stacktrace: &sentry.Stacktrace{ - Frames: []sentry.Frame{}, - }, - Mechanism: &sentry.Mechanism{ - Data: map[string]any{ - "exception_id": 5, - "is_exception_group": true, - "parent_id": 4, - }, - }, - }, - }, - }, - } - - for name, tt := range tests { - tt := tt - t.Run(name, func(t *testing.T) { - t.Parallel() - h, err := New(nil, sentry.ClientOptions{AttachStacktrace: tt.trace}) - if err != nil { - t.Fatal(err) - } - got := h.exceptions(tt.err) - if d := cmp.Diff(tt.want, got); d != "" { - t.Error(d) - } - }) - } -}