From 2c9477d65c997f406652316aeceaf2d951c6f575 Mon Sep 17 00:00:00 2001 From: Jakub Date: Tue, 7 Mar 2023 13:48:34 +0100 Subject: [PATCH] fix(GODT-2442): WIP: bad events just aborts polls, feedback processed in separete channel. --- internal/bridge/user.go | 52 ++++++++++++++++++++++++++++------ internal/bridge/user_events.go | 47 ++++++------------------------ internal/user/events.go | 8 +++--- internal/user/user.go | 20 ++++++------- 4 files changed, 66 insertions(+), 61 deletions(-) diff --git a/internal/bridge/user.go b/internal/bridge/user.go index bb42fba9..6f826e59 100644 --- a/internal/bridge/user.go +++ b/internal/bridge/user.go @@ -24,6 +24,7 @@ import ( "runtime" "github.com/ProtonMail/gluon/imap" + "github.com/ProtonMail/gluon/reporter" "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/proton-bridge/v3/internal/async" "github.com/ProtonMail/proton-bridge/v3/internal/events" @@ -299,17 +300,52 @@ func (bridge *Bridge) SetAddressMode(ctx context.Context, userID string, mode va } // SendBadEventUserFeedback passes the feedback to the given user. -func (bridge *Bridge) SendBadEventUserFeedback(ctx context.Context, userID string, doResync bool) error { - logrus.WithField("userID", userID).WithField("doResycn", doResync).Info("Passing bad event feedback to user") +func (bridge *Bridge) SendBadEventUserFeedback(_ context.Context, userID string, doResync bool) error { + logrus.WithField("userID", userID).WithField("doResync", doResync).Info("Passing bad event feedback to user") - user, ok := bridge.users[userID] - if !ok { - return ErrNoSuchUser - } + return safe.LockRet(func() error { + ctx := context.Background() - user.SendBadEventFeedback(doResync) + user, ok := bridge.users[userID] + if !ok { + if rerr := bridge.reporter.ReportMessageWithContext( + "Failed to handle event: feedback failed: no such user", + reporter.Context{"user_id": userID}, + ); rerr != nil { + logrus.WithError(rerr).Error("Failed to report feedback failure") + } - return nil + // Cannot recover from this + panic(ErrNoSuchUser) + } + + if doResync { + if rerr := bridge.reporter.ReportMessageWithContext( + "Failed to handle event: feedback resync", + reporter.Context{"user_id": userID}, + ); rerr != nil { + logrus.WithError(rerr).Error("Failed to report feedback failure") + } + + user.BadEventFeedbackResync(ctx) + + if err := bridge.addIMAPUser(ctx, user); err != nil { + return fmt.Errorf("failed to add IMAP user: %w", err) + } + + return nil + } + + if rerr := bridge.reporter.ReportMessageWithContext( + "Failed to handle event: feedback logout", + reporter.Context{"user_id": userID}, + ); rerr != nil { + logrus.WithError(rerr).Error("Failed to report feedback failure") + } + + bridge.logoutUser(ctx, user, true, false) + return nil + }, bridge.usersLock) } func (bridge *Bridge) loginUser(ctx context.Context, client *proton.Client, authUID, authRef string, keyPass []byte) (string, error) { diff --git a/internal/bridge/user_events.go b/internal/bridge/user_events.go index da28fb24..db133080 100644 --- a/internal/bridge/user_events.go +++ b/internal/bridge/user_events.go @@ -140,57 +140,28 @@ func (bridge *Bridge) handleUserDeauth(ctx context.Context, user *user.User) { }, bridge.usersLock) } -func (bridge *Bridge) handleUserBadEvent(ctx context.Context, user *user.User, event events.UserBadEvent) { - go safe.Lock(func() { - reportContext := reporter.Context{ +func (bridge *Bridge) handleUserBadEvent(_ context.Context, user *user.User, event events.UserBadEvent) { + safe.Lock(func() { + if rerr := bridge.reporter.ReportMessageWithContext("Failed to handle event", reporter.Context{ "user_id": user.ID(), "old_event_id": event.OldEventID, "new_event_id": event.NewEventID, "event_info": event.EventInfo, "error": event.Error, "error_type": fmt.Sprintf("%T", internal.ErrCause(event.Error)), + }); rerr != nil { + logrus.WithError(rerr).Error("Failed to report failed event handling") } - // blockEventsIMAPandSMTP() + user.BadEventAbort() - if doResyc, err := bridge.getBadEventUserFeedback(user.ID()); err != nil || !doResyc { - if rerr := bridge.reporter.ReportMessageWithContext("Failed to handle event: logout", reportContext); rerr != nil { - logrus.WithError(rerr).Error("Failed to report failed event handling") - } - - bridge.logoutUser(ctx, user, true, false) - return - } - - if rerr := bridge.reporter.ReportMessageWithContext("Failed to handle event: repair", reportContext); rerr != nil { - logrus.WithError(rerr).Error("Failed to report event handling failure") - } - - if syncErr := user.SyncEvent(ctx); syncErr != nil { - reportContext["error"] = syncErr - reportContext["error_type"] = fmt.Sprintf("%T", internal.ErrCause(syncErr)) - if rerr := bridge.reporter.ReportMessageWithContext("Failed to handle event: repair failed: logging out", reportContext); rerr != nil { - logrus.WithError(rerr).Error("Failed to report repair failure") - } - - bridge.logoutUser(ctx, user, true, false) - return + // Disable IMAP user + if err := bridge.removeIMAPUser(context.Background(), user, false); err != nil { + logrus.WithError(err).Error("Failed to remove IMAP user") } }, bridge.usersLock) } -func (bridge *Bridge) getBadEventUserFeedback(userID string) (doResyc bool, err error) { - user, ok := bridge.users[userID] - if !ok { - return false, ErrNoSuchUser - } - - user.LockEvents() - defer user.UnlockEvents() - - return user.GetBadEventFeedback(), nil -} - func (bridge *Bridge) handleUncategorizedErrorEvent(event events.UncategorizedEventError) { if rerr := bridge.reporter.ReportMessageWithContext("Failed to handle due to uncategorized error", reporter.Context{ "error_type": fmt.Sprintf("%T", internal.ErrCause(event.Error)), diff --git a/internal/user/events.go b/internal/user/events.go index b2b66ffa..0dcebd37 100644 --- a/internal/user/events.go +++ b/internal/user/events.go @@ -86,16 +86,16 @@ func (user *User) handleRefreshEvent(ctx context.Context, refresh proton.Refresh l.WithError(err).Error("Failed to report refresh to sentry") } - return user.SyncEvent(ctx) -} - -func (user *User) SyncEvent(ctx context.Context) error { // Abort the event stream defer user.pollAbort.Abort() // Re-sync messages after the user, address and label refresh. defer user.goSync() + return user.syncUserAddressesAndLabels(ctx) +} + +func (user *User) syncUserAddressesAndLabels(ctx context.Context) error { return safe.LockRet(func() error { // Fetch latest user info. apiUser, err := user.client.GetUser(ctx) diff --git a/internal/user/user.go b/internal/user/user.go index 9543fed5..9b519e27 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -90,9 +90,6 @@ type User struct { syncWorkers int showAllMail uint32 - - // doResyncFeedback must be non-blocking for first attempt (i.e. buffered) - doResyncFeedback chan bool } // New returns a new user. @@ -151,8 +148,6 @@ func New( syncWorkers: syncWorkers, showAllMail: b32(showAllMail), - - doResyncFeedback: make(chan bool, 1), } // Initialize the user's update channels for its current address mode. @@ -304,14 +299,17 @@ func (user *User) SetAddressMode(_ context.Context, mode vault.AddressMode) erro }, user.eventLock, user.apiAddrsLock, user.updateChLock) } -// SendBadEventFeedback sends user feedback whether should do message re-sync. -func (user *User) SendBadEventFeedback(doResync bool) { - user.doResyncFeedback <- doResync +// BadEventAbort stops user to communicate. The resolution is either logout or resync. +func (user *User) BadEventAbort() { + user.syncAbort.Abort() + user.pollAbort.Abort() } -// GetBadEventFeedback read the user feedback whether should do message re-sync. -func (user *User) GetBadEventFeedback() bool { - return <-user.doResyncFeedback +// BadEventFeedbackResync sends user feedback whether should do message re-sync. +func (user *User) BadEventFeedbackResync(ctx context.Context) { + if err := user.syncUserAddressesAndLabels(ctx); err != nil { + user.log.WithError(err).Error("Bad event resync failed") + } } // SetShowAllMail sets whether to show the All Mail mailbox.