use SetException in logrus

getsentry · Mar 26, 2024 · c03711a · c03711a
1 parent b5a63ef
commit c03711a
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 237 deletions.
diff --git 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
@@ -341,15 +341,16 @@ 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
 	}
 
 	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
@@ -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
@@ -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)
-			}
-		})
-	}
-}