fix: missing stack trace for parsing error in logrusentry (#677)

getsentry · Aug 7, 2023 · 5caf94e · 5caf94e
1 parent cda691d
commit 5caf94e
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 17 deletions.
diff --git a/logrus/logrusentry.go b/logrus/logrusentry.go
@@ -4,10 +4,12 @@ package sentrylogrus
 import (
 	"errors"
 	"net/http"
+	"reflect"
 	"time"
 
-	sentry "github.com/getsentry/sentry-go"
 	"github.com/sirupsen/logrus"
+
+	sentry "github.com/getsentry/sentry-go"
 )
 
 // These default log field keys are used to pass specific metadata in a way that
@@ -182,17 +184,22 @@ func (h *Hook) entryToEvent(l *logrus.Entry) *sentry.Event {
 }
 
 func (h *Hook) exceptions(err error) []sentry.Exception {
+	if err == nil {
+		return nil
+	}
+
 	if !h.hub.Client().Options().AttachStacktrace {
 		return []sentry.Exception{{
-			Type:  "error",
+			Type:  reflect.TypeOf(err).String(),
 			Value: err.Error(),
 		}}
 	}
+
 	excs := []sentry.Exception{}
 	var last *sentry.Exception
 	for ; err != nil; err = errors.Unwrap(err) {
 		exc := sentry.Exception{
-			Type:       "error",
+			Type:       reflect.TypeOf(err).String(),
 			Value:      err.Error(),
 			Stacktrace: sentry.ExtractStacktrace(err),
 		}
@@ -208,6 +215,13 @@ func (h *Hook) exceptions(err error) []sentry.Exception {
 		excs = append(excs, exc)
 		last = &excs[len(excs)-1]
 	}
+
+	// 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()
+	}
+
 	// reverse
 	for i, j := 0, len(excs)-1; i < j; i, j = i+1, j-1 {
 		excs[i], excs[j] = excs[j], excs[i]

diff --git a/logrus/logrusentry_test.go b/logrus/logrusentry_test.go
@@ -152,7 +152,7 @@ func Test_entryToEvent(t *testing.T) {
 				Level: "fatal",
 				Extra: map[string]interface{}{},
 				Exception: []sentry.Exception{
-					{Type: "error", Value: "things failed"},
+					{Type: "*errors.errorString", Value: "things failed", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
 				},
 			},
 		},
@@ -181,7 +181,7 @@ func Test_entryToEvent(t *testing.T) {
 				Level: "fatal",
 				Extra: map[string]interface{}{},
 				Exception: []sentry.Exception{
-					{Type: "error", Value: "failure", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
+					{Type: "*errors.withStack", Value: "failure", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
 				},
 			},
 		},
@@ -267,37 +267,43 @@ func Test_exceptions(t *testing.T) {
 		err   error
 		want  []sentry.Exception
 	}{
+		{
+			name:  "error is nil",
+			trace: true,
+			err:   nil,
+			want:  nil,
+		},
 		{
 			name:  "std error",
 			trace: true,
 			err:   errors.New("foo"),
 			want: []sentry.Exception{
-				{Type: "error", Value: "foo"},
+				{Type: "*errors.errorString", Value: "foo", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
 			},
 		},
 		{
-			name:  "wrapped, no stack",
+			name:  "wrapped error",
 			trace: true,
 			err:   fmt.Errorf("foo: %w", errors.New("bar")),
 			want: []sentry.Exception{
-				{Type: "error", Value: "bar"},
-				{Type: "error", Value: "foo: bar"},
+				{Type: "*errors.errorString", Value: "bar"},
+				{Type: "*fmt.wrapError", Value: "foo: bar", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
 			},
 		},
 		{
-			name:  "ignored stack",
+			name:  "missing stack for pkgerr",
 			trace: false,
 			err:   pkgerr.New("foo"),
 			want: []sentry.Exception{
-				{Type: "error", Value: "foo"},
+				{Type: "*errors.fundamental", Value: "foo"},
 			},
 		},
 		{
 			name:  "stack",
 			trace: true,
 			err:   pkgerr.New("foo"),
 			want: []sentry.Exception{
-				{Type: "error", Value: "foo", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
+				{Type: "*errors.fundamental", Value: "foo", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
 			},
 		},
 		{
@@ -311,11 +317,11 @@ func Test_exceptions(t *testing.T) {
 				return fmt.Errorf("wrapped: %w", err)
 			}(),
 			want: []sentry.Exception{
-				{Type: "error", Value: "original"},
-				{Type: "error", Value: "fmt: original"},
-				{Type: "error", Value: "wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
-				{Type: "error", Value: "wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
-				{Type: "error", Value: "wrapped: wrap: fmt: original"},
+				{Type: "*errors.errorString", Value: "original"},
+				{Type: "*fmt.wrapError", Value: "fmt: original"},
+				{Type: "*errors.withStack", Value: "wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
+				{Type: "*errors.withStack", Value: "wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
+				{Type: "*fmt.wrapError", Value: "wrapped: wrap: fmt: original", Stacktrace: &sentry.Stacktrace{Frames: []sentry.Frame{}}},
 			},
 		},
 	}