GODT-1036 Event loop Sentry reporting of failures and refresh

This commit is contained in:
Michal Horejsek
2021-02-10 10:36:37 +01:00
committed by Jakub Cuth
parent 7fc7083c76
commit 4e531d4524
11 changed files with 97 additions and 59 deletions

1
go.mod
View File

@ -54,6 +54,7 @@ require (
github.com/nsf/jsondiff v0.0.0-20200515183724-f29ed568f4ce github.com/nsf/jsondiff v0.0.0-20200515183724-f29ed568f4ce
github.com/olekukonko/tablewriter v0.0.4 // indirect github.com/olekukonko/tablewriter v0.0.4 // indirect
github.com/pkg/errors v0.9.1 github.com/pkg/errors v0.9.1
github.com/pkg/math v0.0.0-20141027224758-f2ed9e40e245
github.com/sirupsen/logrus v1.7.0 github.com/sirupsen/logrus v1.7.0
github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966
github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect

2
go.sum
View File

@ -223,6 +223,8 @@ github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTw
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/math v0.0.0-20141027224758-f2ed9e40e245 h1:gk/AF9SGRj+RafNCoDcS3RRscb8S4BVbvqODOgWA7/8=
github.com/pkg/math v0.0.0-20141027224758-f2ed9e40e245/go.mod h1:2dhPPj2Li3DXrSY2U2ADdZy2B7sjQsT57lqENx1+FSE=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNueLj0oo= github.com/russross/blackfriday v1.5.2 h1:HyvC0ARfnZBqnXwABFeSZHpKvJHJJfPz81GNueLj0oo=

View File

@ -76,19 +76,20 @@ const (
) )
type Base struct { type Base struct {
CrashHandler *crash.Handler SentryReporter *sentry.Reporter
Locations *locations.Locations CrashHandler *crash.Handler
Settings *settings.Settings Locations *locations.Locations
Lock *os.File Settings *settings.Settings
Cache *cache.Cache Lock *os.File
Listener listener.Listener Cache *cache.Cache
Creds *credentials.Store Listener listener.Listener
CM *pmapi.ClientManager Creds *credentials.Store
CookieJar *cookies.Jar CM *pmapi.ClientManager
Updater *updater.Updater CookieJar *cookies.Jar
Versioner *versioner.Versioner Updater *updater.Updater
TLS *tls.TLS Versioner *versioner.Versioner
Autostart *autostart.App TLS *tls.TLS
Autostart *autostart.App
Name string // the app's name Name string // the app's name
usage string // the app's usage description usage string // the app's usage description
@ -234,19 +235,20 @@ func New( // nolint[funlen]
} }
return &Base{ return &Base{
CrashHandler: crashHandler, SentryReporter: sentryReporter,
Locations: locations, CrashHandler: crashHandler,
Settings: settingsObj, Locations: locations,
Lock: lock, Settings: settingsObj,
Cache: cache, Lock: lock,
Listener: listener, Cache: cache,
Creds: credentials.NewStore(kc), Listener: listener,
CM: cm, Creds: credentials.NewStore(kc),
CookieJar: jar, CM: cm,
Updater: updater, CookieJar: jar,
Versioner: versioner, Updater: updater,
TLS: tls.New(settingsPath), Versioner: versioner,
Autostart: autostart, TLS: tls.New(settingsPath),
Autostart: autostart,
Name: appName, Name: appName,
usage: appUsage, usage: appUsage,

View File

@ -71,8 +71,7 @@ func run(b *base.Base, c *cli.Context) error { // nolint[funlen]
if err != nil { if err != nil {
logrus.WithError(err).Fatal("Failed to load TLS config") logrus.WithError(err).Fatal("Failed to load TLS config")
} }
bridge := bridge.New(b.Locations, b.Cache, b.Settings, b.SentryReporter, b.CrashHandler, b.Listener, b.CM, b.Creds, b.Updater, b.Versioner)
bridge := bridge.New(b.Locations, b.Cache, b.Settings, b.CrashHandler, b.Listener, b.CM, b.Creds, b.Updater, b.Versioner)
imapBackend := imap.NewIMAPBackend(b.CrashHandler, b.Listener, b.Cache, bridge) imapBackend := imap.NewIMAPBackend(b.CrashHandler, b.Listener, b.Cache, bridge)
smtpBackend := smtp.NewSMTPBackend(b.CrashHandler, b.Listener, b.Settings, bridge) smtpBackend := smtp.NewSMTPBackend(b.CrashHandler, b.Listener, b.Settings, bridge)

View File

@ -29,6 +29,7 @@ import (
"github.com/ProtonMail/proton-bridge/internal/updater" "github.com/ProtonMail/proton-bridge/internal/updater"
"github.com/ProtonMail/proton-bridge/internal/users" "github.com/ProtonMail/proton-bridge/internal/users"
"github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/ProtonMail/proton-bridge/pkg/pmapi"
"github.com/ProtonMail/proton-bridge/pkg/sentry"
"github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/ProtonMail/proton-bridge/pkg/listener"
logrus "github.com/sirupsen/logrus" logrus "github.com/sirupsen/logrus"
@ -56,6 +57,7 @@ func New(
locations Locator, locations Locator,
cache Cacher, cache Cacher,
s SettingsProvider, s SettingsProvider,
sentryReporter *sentry.Reporter,
panicHandler users.PanicHandler, panicHandler users.PanicHandler,
eventListener listener.Listener, eventListener listener.Listener,
clientManager users.ClientManager, clientManager users.ClientManager,
@ -69,7 +71,7 @@ func New(
clientManager.AllowProxy() clientManager.AllowProxy()
} }
storeFactory := newStoreFactory(cache, panicHandler, clientManager, eventListener) storeFactory := newStoreFactory(cache, sentryReporter, panicHandler, clientManager, eventListener)
u := users.New(locations, panicHandler, eventListener, clientManager, credStorer, storeFactory, true) u := users.New(locations, panicHandler, eventListener, clientManager, credStorer, storeFactory, true)
b := &Bridge{ b := &Bridge{
Users: u, Users: u,

View File

@ -23,37 +23,40 @@ import (
"github.com/ProtonMail/proton-bridge/internal/store" "github.com/ProtonMail/proton-bridge/internal/store"
"github.com/ProtonMail/proton-bridge/internal/users" "github.com/ProtonMail/proton-bridge/internal/users"
"github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/ProtonMail/proton-bridge/pkg/listener"
"github.com/ProtonMail/proton-bridge/pkg/sentry"
) )
type storeFactory struct { type storeFactory struct {
cache Cacher cache Cacher
panicHandler users.PanicHandler sentryReporter *sentry.Reporter
clientManager users.ClientManager panicHandler users.PanicHandler
eventListener listener.Listener clientManager users.ClientManager
storeCache *store.Cache eventListener listener.Listener
storeCache *store.Cache
} }
func newStoreFactory( func newStoreFactory(
cache Cacher, cache Cacher,
sentryReporter *sentry.Reporter,
panicHandler users.PanicHandler, panicHandler users.PanicHandler,
clientManager users.ClientManager, clientManager users.ClientManager,
eventListener listener.Listener, eventListener listener.Listener,
) *storeFactory { ) *storeFactory {
return &storeFactory{ return &storeFactory{
cache: cache, cache: cache,
panicHandler: panicHandler, sentryReporter: sentryReporter,
clientManager: clientManager, panicHandler: panicHandler,
eventListener: eventListener, clientManager: clientManager,
storeCache: store.NewCache(cache.GetIMAPCachePath()), eventListener: eventListener,
storeCache: store.NewCache(cache.GetIMAPCachePath()),
} }
} }
// New creates new store for given user. // New creates new store for given user.
func (f *storeFactory) New(user store.BridgeUser) (*store.Store, error) { func (f *storeFactory) New(user store.BridgeUser) (*store.Store, error) {
storePath := getUserStorePath(f.cache.GetDBDir(), user.ID()) storePath := getUserStorePath(f.cache.GetDBDir(), user.ID())
return store.New(f.panicHandler, user, f.clientManager, f.eventListener, storePath, f.storeCache) return store.New(f.sentryReporter, f.panicHandler, user, f.clientManager, f.eventListener, storePath, f.storeCache)
} }
// Remove removes all store files for given user. // Remove removes all store files for given user.

View File

@ -28,8 +28,13 @@ import (
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
) )
const pollInterval = 30 * time.Second const (
const pollIntervalSpread = 5 * time.Second pollInterval = 30 * time.Second
pollIntervalSpread = 5 * time.Second
// errMaxSentry defines after how many errors in a row to report it to sentry.
errMaxSentry = 20
)
type eventLoop struct { type eventLoop struct {
cache *Cache cache *Cache
@ -41,6 +46,7 @@ type eventLoop struct {
isRunning bool // The whole event loop is running. isRunning bool // The whole event loop is running.
pollCounter int pollCounter int
errCounter int
log *logrus.Entry log *logrus.Entry
@ -227,9 +233,18 @@ func (loop *eventLoop) processNextEvent() (more bool, err error) { // nolint[fun
_, errUnauthorized := errors.Cause(err).(*pmapi.ErrUnauthorized) _, errUnauthorized := errors.Cause(err).(*pmapi.ErrUnauthorized)
if err == nil {
loop.errCounter = 0
}
// All errors except Invalid Token (which is not possible to recover from) are ignored. // All errors except Invalid Token (which is not possible to recover from) are ignored.
if err != nil && !errUnauthorized && errors.Cause(err) != pmapi.ErrInvalidToken { if err != nil && !errUnauthorized && errors.Cause(err) != pmapi.ErrInvalidToken {
l.WithError(err).Error("Error skipped") l.WithError(err).WithField("errors", loop.errCounter).Error("Error skipped")
loop.errCounter++
if loop.errCounter == errMaxSentry {
if sentryErr := loop.store.sentryReporter.ReportMessage("Warning: event loop issues: " + err.Error() + ", " + loop.currentEventID); sentryErr != nil {
l.WithError(sentryErr).Error("Failed to report error to sentry")
}
}
err = nil err = nil
} }
}() }()
@ -283,6 +298,10 @@ func (loop *eventLoop) processEvent(event *pmapi.Event) (err error) {
eventLog.Info("Processing refresh event") eventLog.Info("Processing refresh event")
loop.store.triggerSync() loop.store.triggerSync()
if sentryErr := loop.store.sentryReporter.ReportMessage("Warning: refresh occurred, " + loop.currentEventID); sentryErr != nil {
loop.log.WithError(sentryErr).Error("Failed to report refresh to sentry")
}
return return
} }

View File

@ -26,6 +26,7 @@ import (
"github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/ProtonMail/proton-bridge/pkg/listener"
"github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/ProtonMail/proton-bridge/pkg/pmapi"
"github.com/ProtonMail/proton-bridge/pkg/sentry"
"github.com/hashicorp/go-multierror" "github.com/hashicorp/go-multierror"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
@ -95,10 +96,11 @@ var (
// Store is local user storage, which handles the synchronization between IMAP and PM API. // Store is local user storage, which handles the synchronization between IMAP and PM API.
type Store struct { type Store struct {
panicHandler PanicHandler sentryReporter *sentry.Reporter
eventLoop *eventLoop panicHandler PanicHandler
user BridgeUser eventLoop *eventLoop
clientManager ClientManager user BridgeUser
clientManager ClientManager
log *logrus.Entry log *logrus.Entry
@ -115,7 +117,8 @@ type Store struct {
} }
// New creates or opens a store for the given `user`. // New creates or opens a store for the given `user`.
func New( func New( // nolint[funlen]
sentryReporter *sentry.Reporter,
panicHandler PanicHandler, panicHandler PanicHandler,
user BridgeUser, user BridgeUser,
clientManager ClientManager, clientManager ClientManager,
@ -145,14 +148,15 @@ func New(
} }
store = &Store{ store = &Store{
panicHandler: panicHandler, sentryReporter: sentryReporter,
clientManager: clientManager, panicHandler: panicHandler,
user: user, clientManager: clientManager,
cache: cache, user: user,
filePath: path, cache: cache,
db: bdb, filePath: path,
lock: &sync.RWMutex{}, db: bdb,
log: l, lock: &sync.RWMutex{},
log: l,
} }
// Minimal increase is event pollInterval, doubles every failed retry up to 5 minutes. // Minimal increase is event pollInterval, doubles every failed retry up to 5 minutes.

View File

@ -125,6 +125,7 @@ func (mocks *mocksForStore) newStoreNoEvents(combinedMode bool, msgs ...*pmapi.M
var err error var err error
mocks.store, err = New( mocks.store, err = New(
nil, // Sentry reporter is not used under unit tests.
mocks.panicHandler, mocks.panicHandler,
mocks.user, mocks.user,
mocks.clientManager, mocks.clientManager,

View File

@ -31,6 +31,7 @@ import (
usersmocks "github.com/ProtonMail/proton-bridge/internal/users/mocks" usersmocks "github.com/ProtonMail/proton-bridge/internal/users/mocks"
"github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/ProtonMail/proton-bridge/pkg/pmapi"
pmapimocks "github.com/ProtonMail/proton-bridge/pkg/pmapi/mocks" pmapimocks "github.com/ProtonMail/proton-bridge/pkg/pmapi/mocks"
"github.com/ProtonMail/proton-bridge/pkg/sentry"
gomock "github.com/golang/mock/gomock" gomock "github.com/golang/mock/gomock"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -185,9 +186,10 @@ func initMocks(t *testing.T) mocks {
// Set up store factory. // Set up store factory.
m.storeMaker.EXPECT().New(gomock.Any()).DoAndReturn(func(user store.BridgeUser) (*store.Store, error) { m.storeMaker.EXPECT().New(gomock.Any()).DoAndReturn(func(user store.BridgeUser) (*store.Store, error) {
var sentryReporter *sentry.Reporter // Sentry reporter is not used under unit tests.
dbFile, err := ioutil.TempFile("", "bridge-store-db-*.db") dbFile, err := ioutil.TempFile("", "bridge-store-db-*.db")
require.NoError(t, err, "could not get temporary file for store db") require.NoError(t, err, "could not get temporary file for store db")
return store.New(m.PanicHandler, user, m.clientManager, m.eventListener, dbFile.Name(), m.storeCache) return store.New(sentryReporter, m.PanicHandler, user, m.clientManager, m.eventListener, dbFile.Name(), m.storeCache)
}).AnyTimes() }).AnyTimes()
m.storeMaker.EXPECT().Remove(gomock.Any()).AnyTimes() m.storeMaker.EXPECT().Remove(gomock.Any()).AnyTimes()

View File

@ -21,8 +21,10 @@ import (
"time" "time"
"github.com/ProtonMail/proton-bridge/internal/bridge" "github.com/ProtonMail/proton-bridge/internal/bridge"
"github.com/ProtonMail/proton-bridge/internal/constants"
"github.com/ProtonMail/proton-bridge/internal/users" "github.com/ProtonMail/proton-bridge/internal/users"
"github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/ProtonMail/proton-bridge/pkg/listener"
"github.com/ProtonMail/proton-bridge/pkg/sentry"
) )
// GetBridge returns bridge instance. // GetBridge returns bridge instance.
@ -67,8 +69,9 @@ func newBridgeInstance(
eventListener listener.Listener, eventListener listener.Listener,
clientManager users.ClientManager, clientManager users.ClientManager,
) *bridge.Bridge { ) *bridge.Bridge {
sentryReporter := sentry.NewReporter("bridge", constants.Version)
panicHandler := &panicHandler{t: t} panicHandler := &panicHandler{t: t}
updater := newFakeUpdater() updater := newFakeUpdater()
versioner := newFakeVersioner() versioner := newFakeVersioner()
return bridge.New(locations, cache, settings, panicHandler, eventListener, clientManager, credStore, updater, versioner) return bridge.New(locations, cache, settings, sentryReporter, panicHandler, eventListener, clientManager, credStore, updater, versioner)
} }