From 384db1579fd8be4f74f67477e5db543690a117b3 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 9 Sep 2024 00:31:03 +0200 Subject: [PATCH] Revert "Add ability to skip frames (#852)" This reverts commit d95747f906d86bace950fc8eb5a7605a1e1dbc64. # Conflicts: # CHANGELOG.md # stacktrace.go --- client.go | 23 +++++++++-------------- stacktrace.go | 36 +++++++++++++++--------------------- stacktrace_test.go | 2 +- 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/client.go b/client.go index dd5e98262..b5b11b31d 100644 --- a/client.go +++ b/client.go @@ -416,13 +416,9 @@ func (client *Client) Options() ClientOptions { return client.options } -type EventOptions struct { - SkipFrames int -} - // CaptureMessage captures an arbitrary message. -func (client *Client) CaptureMessage(message string, hint *EventHint, scope EventModifier, opts ...EventOptions) *EventID { - event := client.EventFromMessage(message, LevelInfo, opts...) +func (client *Client) CaptureMessage(message string, hint *EventHint, scope EventModifier) *EventID { + event := client.EventFromMessage(message, LevelInfo) return client.CaptureEvent(event, hint, scope) } @@ -444,7 +440,7 @@ func (client *Client) CaptureCheckIn(checkIn *CheckIn, monitorConfig *MonitorCon // CaptureEvent captures an event on the currently active client if any. // -// The event must already be assembled. Typically, code would instead use +// The event must already be assembled. Typically code would instead use // the utility methods like CaptureException. The return value is the // event ID. In case Sentry is disabled or event was dropped, the return value will be nil. func (client *Client) CaptureEvent(event *Event, hint *EventHint, scope EventModifier) *EventID { @@ -453,7 +449,7 @@ func (client *Client) CaptureEvent(event *Event, hint *EventHint, scope EventMod // Recover captures a panic. // Returns EventID if successfully, or nil if there's no error to recover from. -func (client *Client) Recover(err interface{}, hint *EventHint, scope EventModifier, opts ...EventOptions) *EventID { +func (client *Client) Recover(err interface{}, hint *EventHint, scope EventModifier) *EventID { if err == nil { err = recover() } @@ -463,7 +459,7 @@ func (client *Client) Recover(err interface{}, hint *EventHint, scope EventModif // is store the Context in the EventHint and there nil means the Context is // not available. // nolint: staticcheck - return client.RecoverWithContext(nil, err, hint, scope, opts...) + return client.RecoverWithContext(nil, err, hint, scope) } // RecoverWithContext captures a panic and passes relevant context object. @@ -473,7 +469,6 @@ func (client *Client) RecoverWithContext( err interface{}, hint *EventHint, scope EventModifier, - opts ...EventOptions, ) *EventID { if err == nil { err = recover() @@ -496,9 +491,9 @@ func (client *Client) RecoverWithContext( case error: event = client.EventFromException(err, LevelFatal) case string: - event = client.EventFromMessage(err, LevelFatal, opts...) + event = client.EventFromMessage(err, LevelFatal) default: - event = client.EventFromMessage(fmt.Sprintf("%#v", err), LevelFatal, opts...) + event = client.EventFromMessage(fmt.Sprintf("%#v", err), LevelFatal) } return client.CaptureEvent(event, hint, scope) } @@ -519,7 +514,7 @@ func (client *Client) Flush(timeout time.Duration) bool { } // EventFromMessage creates an event from the given message string. -func (client *Client) EventFromMessage(message string, level Level, opts ...EventOptions) *Event { +func (client *Client) EventFromMessage(message string, level Level) *Event { if message == "" { err := usageError{fmt.Errorf("%s called with empty message", callerFunctionName())} return client.EventFromException(err, level) @@ -530,7 +525,7 @@ func (client *Client) EventFromMessage(message string, level Level, opts ...Even if client.options.AttachStacktrace { event.Threads = []Thread{{ - Stacktrace: NewStacktrace(opts...), + Stacktrace: NewStacktrace(), Crashed: false, Current: true, }} diff --git a/stacktrace.go b/stacktrace.go index 06218ecfd..3b372bcff 100644 --- a/stacktrace.go +++ b/stacktrace.go @@ -23,7 +23,7 @@ type Stacktrace struct { } // NewStacktrace creates a stacktrace using runtime.Callers. -func NewStacktrace(opts ...EventOptions) *Stacktrace { +func NewStacktrace() *Stacktrace { pcs := make([]uintptr, 100) n := runtime.Callers(1, pcs) @@ -31,13 +31,8 @@ func NewStacktrace(opts ...EventOptions) *Stacktrace { return nil } - skipFrames := 0 - if len(opts) > 0 { - skipFrames = opts[0].SkipFrames - } - runtimeFrames := extractFrames(pcs[:n]) - frames := createFrames(runtimeFrames, skipFrames) + frames := createFrames(runtimeFrames) stacktrace := Stacktrace{ Frames: frames, @@ -67,7 +62,7 @@ func ExtractStacktrace(err error) *Stacktrace { } runtimeFrames := extractFrames(pcs) - frames := createFrames(runtimeFrames, 0) + frames := createFrames(runtimeFrames) stacktrace := Stacktrace{ Frames: frames, @@ -275,25 +270,30 @@ func extractFrames(pcs []uintptr) []runtime.Frame { for { callerFrame, more := callersFrames.Next() - // Prepend the frame - frames = append([]runtime.Frame{callerFrame}, frames...) + frames = append(frames, callerFrame) if !more { break } } + // TODO don't append and reverse, put in the right place from the start. + // reverse + for i, j := 0, len(frames)-1; i < j; i, j = i+1, j-1 { + frames[i], frames[j] = frames[j], frames[i] + } + return frames } // createFrames creates Frame objects while filtering out frames that are not // meant to be reported to Sentry, those are frames internal to the SDK or Go. -func createFrames(frames []runtime.Frame, skip int) []Frame { - if len(frames) == 0 || skip >= len(frames) { +func createFrames(frames []runtime.Frame) []Frame { + if len(frames) == 0 { return nil } - var result []Frame + result := make([]Frame, 0, len(frames)) for _, frame := range frames { function := frame.Function @@ -307,12 +307,6 @@ func createFrames(frames []runtime.Frame, skip int) []Frame { } } - if skip >= len(result) { - return []Frame{} - } - - result = result[:len(result)-skip] - // Fix issues grouping errors with the new fully qualified function names // introduced from Go 1.21 result = cleanupFunctionNamePrefix(result) @@ -342,12 +336,12 @@ func shouldSkipFrame(module string) bool { var goRoot = strings.ReplaceAll(build.Default.GOROOT, "\\", "/") func setInAppFrame(frame *Frame) { - frame.InApp = true - if strings.HasPrefix(frame.AbsPath, goRoot) || strings.Contains(frame.Module, "vendor") || strings.Contains(frame.Module, "third_party") { frame.InApp = false + } else { + frame.InApp = true } } diff --git a/stacktrace_test.go b/stacktrace_test.go index 663f86bc6..96d682801 100644 --- a/stacktrace_test.go +++ b/stacktrace_test.go @@ -139,7 +139,7 @@ func TestCreateFrames(t *testing.T) { } for _, tt := range tests { t.Run("", func(t *testing.T) { - got := createFrames(tt.in, 0) + got := createFrames(tt.in) if diff := cmp.Diff(tt.out, got); diff != "" { t.Errorf("filterFrames() mismatch (-want +got):\n%s", diff) }