From bb1d27a5be392344c3f86629c4bb1e1bad954062 Mon Sep 17 00:00:00 2001 From: Michal Horejsek Date: Thu, 3 Sep 2020 14:36:12 +0200 Subject: [PATCH] Do not ignore errors --- Changelog.md | 2 +- internal/imap/backend.go | 4 ++- internal/imap/backend_cache.go | 6 ++-- internal/imap/mailbox_messages.go | 56 +++++++++++++++++++++++-------- internal/smtp/user.go | 4 ++- internal/store/cache.go | 4 ++- internal/store/mailbox.go | 2 +- internal/store/mailbox_message.go | 3 ++ internal/store/user_mailbox.go | 4 ++- 9 files changed, 63 insertions(+), 22 deletions(-) diff --git a/Changelog.md b/Changelog.md index 2b67e9c6..8406194c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -35,7 +35,7 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) * Set first-start to false in bridge, not in frontend. * GODT-400 Refactor sendingInfo. * GODT-513 Update routes to API v4. - +* GODT-551 Do not ignore errors during message flagging. ### Fixed * GODT-454 Fix send on closed channel when receiving unencrypted send confirmation from GUI. diff --git a/internal/imap/backend.go b/internal/imap/backend.go index 0e300fdb..ab59b1c4 100644 --- a/internal/imap/backend.go +++ b/internal/imap/backend.go @@ -164,7 +164,9 @@ func (ib *imapBackend) Login(_ *imap.ConnInfo, username, password string) (goIMA if err := imapUser.user.CheckBridgeLogin(password); err != nil { log.WithError(err).Error("Could not check bridge password") - _ = imapUser.Logout() + if err := imapUser.Logout(); err != nil { + log.WithError(err).Warn("Could not logout user after unsuccessful login check") + } // Apple Mail sometimes generates a lot of requests very quickly. // It's therefore good to have a timeout after a bad login so that we can slow // those requests down a little bit. diff --git a/internal/imap/backend_cache.go b/internal/imap/backend_cache.go index ff4dd6fd..57582d77 100644 --- a/internal/imap/backend_cache.go +++ b/internal/imap/backend_cache.go @@ -80,7 +80,7 @@ func (ib *imapBackend) removeFromCache(userID, label, toRemove string) { func (ib *imapBackend) getCacheList(userID, label string) (list string) { if err := ib.loadIMAPCache(); err != nil { - log.Warn("Could not load cache: ", err) + log.WithError(err).Warn("Could not load cache") } ib.imapCacheLock.Lock() @@ -97,7 +97,9 @@ func (ib *imapBackend) getCacheList(userID, label string) (list string) { ib.imapCacheLock.Unlock() - _ = ib.saveIMAPCache() + if err := ib.saveIMAPCache(); err != nil { + log.WithError(err).Warn("Could not save cache") + } return } diff --git a/internal/imap/mailbox_messages.go b/internal/imap/mailbox_messages.go index d8a08cb8..e9e468f9 100644 --- a/internal/imap/mailbox_messages.go +++ b/internal/imap/mailbox_messages.go @@ -77,19 +77,29 @@ func (im *imapMailbox) setFlags(messageIDs, flags []string) error { } if seen { - _ = im.storeMailbox.MarkMessagesRead(messageIDs) + if err := im.storeMailbox.MarkMessagesRead(messageIDs); err != nil { + return err + } } else { - _ = im.storeMailbox.MarkMessagesUnread(messageIDs) + if err := im.storeMailbox.MarkMessagesUnread(messageIDs); err != nil { + return err + } } if flagged { - _ = im.storeMailbox.MarkMessagesStarred(messageIDs) + if err := im.storeMailbox.MarkMessagesStarred(messageIDs); err != nil { + return err + } } else { - _ = im.storeMailbox.MarkMessagesUnstarred(messageIDs) + if err := im.storeMailbox.MarkMessagesUnstarred(messageIDs); err != nil { + return err + } } if deleted { - _ = im.storeMailbox.DeleteMessages(messageIDs) + if err := im.storeMailbox.DeleteMessages(messageIDs); err != nil { + return err + } } spamMailbox, err := im.storeAddress.GetMailbox("Spam") @@ -97,9 +107,13 @@ func (im *imapMailbox) setFlags(messageIDs, flags []string) error { return err } if spam { - _ = spamMailbox.LabelMessages(messageIDs) + if err := spamMailbox.LabelMessages(messageIDs); err != nil { + return err + } } else { - _ = spamMailbox.UnlabelMessages(messageIDs) + if err := spamMailbox.UnlabelMessages(messageIDs); err != nil { + return err + } } return nil @@ -111,22 +125,32 @@ func (im *imapMailbox) addOrRemoveFlags(operation imap.FlagsOp, messageIDs, flag case imap.SeenFlag: switch operation { case imap.AddFlags: - _ = im.storeMailbox.MarkMessagesRead(messageIDs) + if err := im.storeMailbox.MarkMessagesRead(messageIDs); err != nil { + return err + } case imap.RemoveFlags: - _ = im.storeMailbox.MarkMessagesUnread(messageIDs) + if err := im.storeMailbox.MarkMessagesUnread(messageIDs); err != nil { + return err + } } case imap.FlaggedFlag: switch operation { case imap.AddFlags: - _ = im.storeMailbox.MarkMessagesStarred(messageIDs) + if err := im.storeMailbox.MarkMessagesStarred(messageIDs); err != nil { + return err + } case imap.RemoveFlags: - _ = im.storeMailbox.MarkMessagesUnstarred(messageIDs) + if err := im.storeMailbox.MarkMessagesUnstarred(messageIDs); err != nil { + return err + } } case imap.DeletedFlag: if operation == imap.RemoveFlags { break // Nothing to do, no message has the \Deleted flag. } - _ = im.storeMailbox.DeleteMessages(messageIDs) + if err := im.storeMailbox.DeleteMessages(messageIDs); err != nil { + return err + } case imap.AnsweredFlag, imap.DraftFlag, imap.RecentFlag: // Not supported. case message.AppleMailJunkFlag, message.ThunderbirdJunkFlag: @@ -140,9 +164,13 @@ func (im *imapMailbox) addOrRemoveFlags(operation imap.FlagsOp, messageIDs, flag // No label removal is necessary because Spam and Inbox are both exclusive labels so the backend // will automatically take care of label removal. case imap.AddFlags: - _ = storeMailbox.LabelMessages(messageIDs) + if err := storeMailbox.LabelMessages(messageIDs); err != nil { + return err + } case imap.RemoveFlags: - _ = storeMailbox.UnlabelMessages(messageIDs) + if err := storeMailbox.UnlabelMessages(messageIDs); err != nil { + return err + } } } } diff --git a/internal/smtp/user.go b/internal/smtp/user.go index a17f93de..ff905866 100644 --- a/internal/smtp/user.go +++ b/internal/smtp/user.go @@ -359,7 +359,9 @@ func (su *smtpUser) Send(from string, to []string, messageReader io.Reader) (err return errors.New("error decoding subject message " + message.Header.Get("Subject")) } if !su.continueSendingUnencryptedMail(subject) { - _ = su.client().DeleteMessages([]string{message.ID}) + if err := su.client().DeleteMessages([]string{message.ID}); err != nil { + log.WithError(err).Warn("Failed to delete canceled messages") + } return errors.New("sending was canceled by user") } } diff --git a/internal/store/cache.go b/internal/store/cache.go index 314044dd..948abf93 100644 --- a/internal/store/cache.go +++ b/internal/store/cache.go @@ -45,7 +45,9 @@ func (c *Cache) getEventID(userID string) string { c.lock.Lock() defer c.lock.Unlock() - _ = c.loadCache() + if err := c.loadCache(); err != nil { + log.WithError(err).Warn("Problem to load store cache") + } if c.cache == nil { c.cache = map[string]map[string]string{} diff --git a/internal/store/mailbox.go b/internal/store/mailbox.go index c4a834fb..82f7ba00 100644 --- a/internal/store/mailbox.go +++ b/internal/store/mailbox.go @@ -41,7 +41,7 @@ type Mailbox struct { } func newMailbox(storeAddress *Address, labelID, labelPrefix, labelName, color string) (mb *Mailbox, err error) { - _ = storeAddress.store.db.Update(func(tx *bolt.Tx) error { + err = storeAddress.store.db.Update(func(tx *bolt.Tx) error { mb, err = txNewMailbox(tx, storeAddress, labelID, labelPrefix, labelName, color) return err }) diff --git a/internal/store/mailbox_message.go b/internal/store/mailbox_message.go index 59cd46d5..e44be110 100644 --- a/internal/store/mailbox_message.go +++ b/internal/store/mailbox_message.go @@ -129,6 +129,9 @@ func (storeMailbox *Mailbox) MarkMessagesRead(apiIDs []string) error { ids = append(ids, apiID) } } + if len(ids) == 0 { + return nil + } return storeMailbox.client().MarkMessagesRead(ids) } diff --git a/internal/store/user_mailbox.go b/internal/store/user_mailbox.go index db8e9b9b..15a52767 100644 --- a/internal/store/user_mailbox.go +++ b/internal/store/user_mailbox.go @@ -218,7 +218,9 @@ func (store *Store) deleteMailboxEvent(labelID string) error { store.lock.Lock() defer store.lock.Unlock() - _ = store.removeMailboxCount(labelID) + if err := store.removeMailboxCount(labelID); err != nil { + log.WithError(err).Warn("Problem to remove mailbox counts while deleting mailbox") + } for _, a := range store.addresses { if err := a.deleteMailboxEvent(labelID); err != nil {