From 33dfc5ce099af0ffedfffabebf599708f7aefd42 Mon Sep 17 00:00:00 2001
From: Michal Horejsek <horejsek@protonmail.com>
Date: Wed, 2 Dec 2020 13:07:02 +0100
Subject: [PATCH] Use function to determine which functions to skip

---
 internal/cmd/restart.go   |  3 +++
 pkg/config/logs.go        |  2 ++
 pkg/sentry/report.go      | 50 +++++++++++++++++++++++++++++----------
 pkg/sentry/report_test.go | 23 ++++++++++++++++++
 4 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/internal/cmd/restart.go b/internal/cmd/restart.go
index abd3036e..efcdf672 100644
--- a/internal/cmd/restart.go
+++ b/internal/cmd/restart.go
@@ -26,6 +26,7 @@ import (
 
 	"github.com/ProtonMail/proton-bridge/internal/frontend"
 	"github.com/ProtonMail/proton-bridge/pkg/config"
+	"github.com/ProtonMail/proton-bridge/pkg/sentry"
 	"github.com/urfave/cli"
 )
 
@@ -92,6 +93,8 @@ type PanicHandler struct {
 
 // HandlePanic should be called in defer to ensure restart of app after error.
 func (ph *PanicHandler) HandlePanic() {
+	sentry.SkipDuringUnwind()
+
 	r := recover()
 	if r == nil {
 		return
diff --git a/pkg/config/logs.go b/pkg/config/logs.go
index e55db04e..2a53df59 100644
--- a/pkg/config/logs.go
+++ b/pkg/config/logs.go
@@ -57,6 +57,8 @@ var logCrashRgx = regexp.MustCompile("^v.*_crash_.*\\.log$") //nolint[gochecknog
 
 // HandlePanic reports the crash to sentry or local file when sentry fails.
 func HandlePanic(cfg *Config, output string) {
+	sentry.SkipDuringUnwind()
+
 	if !cfg.IsDevMode() {
 		apiCfg := cfg.GetAPIConfig()
 		if err := sentry.ReportSentryCrash(apiCfg.ClientID, apiCfg.AppVersion, apiCfg.UserAgent, errors.New(output)); err != nil {
diff --git a/pkg/sentry/report.go b/pkg/sentry/report.go
index f447fda6..4c8db6c5 100644
--- a/pkg/sentry/report.go
+++ b/pkg/sentry/report.go
@@ -19,9 +19,7 @@ package sentry
 
 import (
 	"errors"
-	"regexp"
 	"runtime"
-	"strings"
 	"time"
 
 	"github.com/getsentry/sentry-go"
@@ -29,13 +27,14 @@ import (
 )
 
 var (
-	isPanicHandlerRegexp = regexp.MustCompile(`^ReportSentryCrash|^(\(\*PanicHandler\)\.)?HandlePanic`) //nolint[gochecknoglobals]
+	skippedFunctions = []string{} //nolint[gochecknoglobals]
 )
 
 // ReportSentryCrash reports a sentry crash.
-func ReportSentryCrash(clientID, appVersion, userAgent string, reportErr error) (err error) {
+func ReportSentryCrash(clientID, appVersion, userAgent string, reportErr error) error {
+	SkipDuringUnwind()
 	if reportErr == nil {
-		return
+		return nil
 	}
 
 	tags := map[string]string{
@@ -48,6 +47,7 @@ func ReportSentryCrash(clientID, appVersion, userAgent string, reportErr error)
 
 	var reportID string
 	sentry.WithScope(func(scope *sentry.Scope) {
+		SkipDuringUnwind()
 		scope.SetTags(tags)
 		eventID := sentry.CaptureException(reportErr)
 		if eventID != nil {
@@ -61,7 +61,24 @@ func ReportSentryCrash(clientID, appVersion, userAgent string, reportErr error)
 	}
 
 	log.WithField("error", reportErr).WithField("id", reportID).Warn("Sentry error reported")
-	return
+	return nil
+}
+
+// SkipDuringUnwind removes caller from the traceback.
+func SkipDuringUnwind() {
+	pcs := make([]uintptr, 2)
+	n := runtime.Callers(2, pcs)
+	if n == 0 {
+		return
+	}
+
+	frames := runtime.CallersFrames(pcs)
+	frame, _ := frames.Next()
+	if isFunctionFilteredOut(frame.Function) {
+		return
+	}
+
+	skippedFunctions = append(skippedFunctions, frame.Function)
 }
 
 // EnhanceSentryEvent swaps type with value and removes panic handlers from the stacktrace.
@@ -77,13 +94,22 @@ func EnhanceSentryEvent(event *sentry.Event, hint *sentry.EventHint) *sentry.Eve
 }
 
 func filterOutPanicHandlers(frames []sentry.Frame) []sentry.Frame {
-	idx := 0
+	newFrames := []sentry.Frame{}
 	for _, frame := range frames {
-		if strings.HasPrefix(frame.Module, "github.com/ProtonMail/proton-bridge") &&
-			isPanicHandlerRegexp.MatchString(frame.Function) {
-			break
+		// Sentry splits runtime.Frame.Function into Module and Function.
+		function := frame.Module + "." + frame.Function
+		if !isFunctionFilteredOut(function) {
+			newFrames = append(newFrames, frame)
 		}
-		idx++
 	}
-	return frames[:idx]
+	return newFrames
+}
+
+func isFunctionFilteredOut(function string) bool {
+	for _, skipFunction := range skippedFunctions {
+		if function == skipFunction {
+			return true
+		}
+	}
+	return false
 }
diff --git a/pkg/sentry/report_test.go b/pkg/sentry/report_test.go
index a473e063..95d13213 100644
--- a/pkg/sentry/report_test.go
+++ b/pkg/sentry/report_test.go
@@ -25,7 +25,30 @@ import (
 	"github.com/getsentry/sentry-go"
 )
 
+func TestSkipDuringUnwind(t *testing.T) {
+	// More calls in one function adds it only once.
+	SkipDuringUnwind()
+	SkipDuringUnwind()
+	func() {
+		SkipDuringUnwind()
+		SkipDuringUnwind()
+	}()
+
+	wantSkippedFunctions := []string{
+		"github.com/ProtonMail/proton-bridge/pkg/sentry.TestSkipDuringUnwind",
+		"github.com/ProtonMail/proton-bridge/pkg/sentry.TestSkipDuringUnwind.func1",
+	}
+	r.Equal(t, wantSkippedFunctions, skippedFunctions)
+}
+
 func TestFilterOutPanicHandlers(t *testing.T) {
+	skippedFunctions = []string{
+		"github.com/ProtonMail/proton-bridge/pkg/config.(*PanicHandler).HandlePanic",
+		"github.com/ProtonMail/proton-bridge/pkg/config.HandlePanic",
+		"github.com/ProtonMail/proton-bridge/pkg/sentry.ReportSentryCrash",
+		"github.com/ProtonMail/proton-bridge/pkg/sentry.ReportSentryCrash.func1",
+	}
+
 	frames := []sentry.Frame{
 		{Module: "github.com/ProtonMail/proton-bridge/internal/cmd", Function: "main"},
 		{Module: "github.com/urfave/cli", Function: "(*App).Run"},