refactor: make sentry report its own package

pkg/config/logs.go

@@ -19,6 +19,7 @@ package config
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -31,6 +32,7 @@ import (
 	"time"
 
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
+	"github.com/ProtonMail/proton-bridge/pkg/sentry"
 	"github.com/sirupsen/logrus"
 )
 
@@ -57,11 +59,10 @@ 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) {
 	if !cfg.IsDevMode() {
-		// TODO: Is it okay to just create a throwaway client like this?
-		c := pmapi.NewClientManager(cfg.GetAPIConfig()).GetAnonymousClient()
-		defer c.Logout()
-
-		if err := c.ReportSentryCrash(fmt.Errorf(output)); err != nil {
+		apiCfg := cfg.GetAPIConfig()
+		clientID := apiCfg.ClientID
+		appVersion := apiCfg.AppVersion
+		if err := sentry.ReportSentryCrash(clientID, appVersion, pmapi.CurrentUserAgent, errors.New(output)); err != nil {
 			log.Error("Sentry crash report failed: ", err)
 		}
 	}

pkg/pmapi/auth.go

@@ -494,7 +494,10 @@ func (c *client) DeleteAuth() (err error) {
 	return
 }
 
-// TODO: Need a method like IsConnected() to be able to detect whether a client is logged in or not.
+// IsConnected returns whether the client is authorized to access the API.
+func (c *client) IsConnected() bool {
+	return c.uid != "" && c.accessToken != ""
+}
 
 // ClearData clears sensitive data from the client.
 func (c *client) ClearData() {

pkg/pmapi/auth_test.go

@@ -336,12 +336,7 @@ func TestClient_Logout(t *testing.T) {
 	c.Logout()
 
 	r.Eventually(t, func() bool {
-		// TODO: Use a method like IsConnected() which returns whether the client was logged out or not.
-		return c.accessToken == "" &&
-			c.uid == "" &&
-			c.kr == nil &&
-			c.addresses == nil &&
-			c.user == nil
+		return c.IsConnected() == false && c.kr == nil && c.addresses == nil && c.user == nil
 	}, 10*time.Second, 10*time.Millisecond)
 }
 

pkg/pmapi/client.go

@@ -98,6 +98,7 @@ type Client interface {
 	Auth2FA(twoFactorCode string, auth *Auth) (*Auth2FA, error)
 	Logout()
 	DeleteAuth() error
+	IsConnected() bool
 	ClearData()
 
 	CurrentUser() (*User, error)
@@ -131,7 +132,6 @@ type Client interface {
 
 	ReportBugWithEmailClient(os, osVersion, title, description, username, email, emailClient string) error
 	SendSimpleMetric(category, action, label string) error
-	ReportSentryCrash(reportErr error) (err error)
 
 	GetMailSettings() (MailSettings, error)
 	GetContactEmailByEmail(string, int, int) ([]ContactEmail, error)

pkg/pmapi/mocks/mocks.go

@@ -418,6 +418,20 @@ func (mr *MockClientMockRecorder) Import(arg0 interface{}) *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Import", reflect.TypeOf((*MockClient)(nil).Import), arg0)
 }
 
+// IsConnected mocks base method
+func (m *MockClient) IsConnected() bool {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "IsConnected")
+	ret0, _ := ret[0].(bool)
+	return ret0
+}
+
+// IsConnected indicates an expected call of IsConnected
+func (mr *MockClientMockRecorder) IsConnected() *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsConnected", reflect.TypeOf((*MockClient)(nil).IsConnected))
+}
+
 // KeyRingForAddressID mocks base method
 func (m *MockClient) KeyRingForAddressID(arg0 string) *crypto.KeyRing {
 	m.ctrl.T.Helper()
@@ -531,20 +545,6 @@ func (mr *MockClientMockRecorder) ReportBugWithEmailClient(arg0, arg1, arg2, arg
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReportBugWithEmailClient", reflect.TypeOf((*MockClient)(nil).ReportBugWithEmailClient), arg0, arg1, arg2, arg3, arg4, arg5, arg6)
 }
 
-// ReportSentryCrash mocks base method
-func (m *MockClient) ReportSentryCrash(arg0 error) error {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "ReportSentryCrash", arg0)
-	ret0, _ := ret[0].(error)
-	return ret0
-}
-
-// ReportSentryCrash indicates an expected call of ReportSentryCrash
-func (mr *MockClientMockRecorder) ReportSentryCrash(arg0 interface{}) *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReportSentryCrash", reflect.TypeOf((*MockClient)(nil).ReportSentryCrash), arg0)
-}
-
 // SendMessage mocks base method
 func (m *MockClient) SendMessage(arg0 string, arg1 *pmapi.SendMessageReq) (*pmapi.Message, *pmapi.Message, error) {
 	m.ctrl.T.Helper()

pkg/sentry/report.go

@@ -15,7 +15,7 @@
 // You should have received a copy of the GNU General Public License
 // along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
 
-package pmapi
+package sentry
 
 import (
 	"fmt"
@@ -26,19 +26,20 @@ import (
 	"strings"
 
 	"github.com/getsentry/raven-go"
+	log "github.com/sirupsen/logrus"
 )
 
 const fileParseError = "[file parse error]"
 
 var isGoroutine = regexp.MustCompile("^goroutine [[:digit:]]+.*") //nolint[gochecknoglobals]
 
-// SentryThreads implements standard sentry thread report.
-type SentryThreads struct {
+// Threads implements standard sentry thread report.
+type Threads struct {
 	Values []Thread `json:"values"`
 }
 
 // Class specifier.
-func (s *SentryThreads) Class() string { return "threads" }
+func (s *Threads) Class() string { return "threads" }
 
 // Thread wraps a single stacktrace.
 type Thread struct {
@@ -49,7 +50,7 @@ type Thread struct {
 }
 
 // TraceAllRoutines traces all goroutines and saves them to the current object.
-func (s *SentryThreads) TraceAllRoutines() {
+func (s *Threads) TraceAllRoutines() {
 	s.Values = []Thread{}
 	goroutines := &strings.Builder{}
 	_ = pprof.Lookup("goroutine").WriteTo(goroutines, 2)
@@ -109,7 +110,7 @@ func (s *SentryThreads) TraceAllRoutines() {
 	s.Values = append(s.Values, thread)
 }
 
-func findPanicSender(s *SentryThreads, err error) string {
+func findPanicSender(s *Threads, err error) string {
 	out := "error nil"
 	if err != nil {
 		out = err.Error()
@@ -146,28 +147,31 @@ func findPanicSender(s *SentryThreads, err error) string {
 }
 
 // ReportSentryCrash reports a sentry crash with stacktrace from all goroutines.
-func (c *client) ReportSentryCrash(reportErr error) (err error) {
+func ReportSentryCrash(clientID, appVersion, userAgent string, reportErr error) (err error) {
 	if reportErr == nil {
 		return
 	}
+
 	tags := map[string]string{
 		"OS":        runtime.GOOS,
-		"Client":    c.cm.config.ClientID,
-		"Version":   c.cm.config.AppVersion,
-		"UserAgent": CurrentUserAgent,
-		"UserID":    c.userID,
+		"Client":    clientID,
+		"Version":   appVersion,
+		"UserAgent": userAgent,
+		"UserID":    "",
 	}
 
-	threads := &SentryThreads{}
+	threads := &Threads{}
 	threads.TraceAllRoutines()
 	errorWithFile := findPanicSender(threads, reportErr)
 	packet := raven.NewPacket(errorWithFile, threads)
 
 	eventID, ch := raven.Capture(packet, tags)
+
 	if err = <-ch; err == nil {
-		c.log.Warn("Reported error with id: ", eventID)
+		log.WithField("errorID", eventID).Warn("Reported sentry error")
 	} else {
-		c.log.Errorf("Can not report `%s` due to `%s`", reportErr.Error(), err.Error())
+		log.WithField("error", reportErr).WithError(err).Error("Failed to report sentry error")
 	}
+
 	return err
 }

pkg/sentry/report_test.go

@@ -15,7 +15,7 @@
 // You should have received a copy of the GNU General Public License
 // along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
 
-package pmapi
+package sentry
 
 import (
 	"errors"
@@ -25,14 +25,17 @@ import (
 )
 
 func TestSentryCrashReport(t *testing.T) {
-	cm := NewClientManager(testClientConfig)
-	c := cm.GetClient("bridgetest")
-	if err := c.ReportSentryCrash(errors.New("Testing crash report - api proxy; goroutines with threads, find origin")); err != nil {
+	if err := ReportSentryCrash(
+		"clientID",
+		"appVersion",
+		"useragent",
+		errors.New("Testing crash report - api proxy; goroutines with threads, find origin"),
+	); err != nil {
 		t.Fatal("Expected no error while report, but have", err)
 	}
 }
 
-func (s *SentryThreads) TraceAllRoutinesTest() {
+func (s *Threads) TraceAllRoutinesTest() {
 	s.Values = []Thread{
 		{
 			ID:      0,

test/fakeapi/auth.go

@@ -154,6 +154,10 @@ func (api *FakePMAPI) Logout() {
 	api.ClearData()
 }
 
+func (api *FakePMAPI) IsConnected() bool {
+	return api.uid != "" && api.lastToken != ""
+}
+
 func (api *FakePMAPI) DeleteAuth() error {
 	if err := api.checkAndRecordCall(DELETE, "/auth", nil); err != nil {
 		return err