From 141637df435abb1da3cb63276a911676987a114d Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Tue, 1 Aug 2023 10:14:02 +0200 Subject: [PATCH] when creating a mailbox subscription, don't just try to insert a record into the database and handle bstore.ErrUnique, the transaction will have been marked as botched behaviour around failing DB calls that change data (insert/update) was changed in bstore quite some time ago. the tx state in bstore would become inconsistent when one or more (possibly unique) indexes had been modified, but then an ErrUnique would occur for the next index. bstore doesn't know how to roll back the partial changes during a transaction, so it marks the tx as botched and refuses further operations. so, we cannot just try to insert, wait for a possible ErrUnique, but then still try to continue with the transaction. instead, we check if the record exists and only insert it if we couldn't find it. found while working on webmail. --- http/import.go | 4 ++-- imapserver/server.go | 5 +++-- store/account.go | 8 +++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/http/import.go b/http/import.go index a6d7880..b3be525 100644 --- a/http/import.go +++ b/http/import.go @@ -450,8 +450,8 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store err = tx.Insert(&mb) ximportcheckf(err, "inserting mailbox in database") - err = tx.Insert(&store.Subscription{Name: p}) - if err != nil && !errors.Is(err, bstore.ErrUnique) { + if tx.Get(&store.Subscription{Name: p}) != nil { + err := tx.Insert(&store.Subscription{Name: p}) ximportcheckf(err, "subscribing to imported mailbox") } changes = append(changes, store.ChangeAddMailbox{Name: p, Flags: []string{`\Subscribed`}}) diff --git a/imapserver/server.go b/imapserver/server.go index 690a3cf..290d9b9 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -2470,8 +2470,9 @@ func (c *conn) cmdRename(tag, cmd string, p *parser) { } err = tx.Insert(&mb) xcheckf(err, "creating parent mailbox") - err = tx.Insert(&store.Subscription{Name: parent}) - if err != nil && !errors.Is(err, bstore.ErrUnique) { + + if tx.Get(&store.Subscription{Name: parent}) != nil { + err := tx.Insert(&store.Subscription{Name: parent}) xcheckf(err, "creating subscription") } changes = append(changes, store.ChangeAddMailbox{Name: parent, Flags: []string{`\Subscribed`}}) diff --git a/store/account.go b/store/account.go index ce025fc..22a759c 100644 --- a/store/account.go +++ b/store/account.go @@ -970,9 +970,11 @@ func (a *Account) MailboxEnsure(tx *bstore.Tx, name string, subscribe bool) (mb change := ChangeAddMailbox{Name: p} if subscribe { - err := tx.Insert(&Subscription{p}) - if err != nil && !errors.Is(err, bstore.ErrUnique) { - return Mailbox{}, nil, fmt.Errorf("subscribing to mailbox: %v", err) + if tx.Get(&Subscription{p}) != nil { + err := tx.Insert(&Subscription{p}) + if err != nil { + return Mailbox{}, nil, fmt.Errorf("subscribing to mailbox: %v", err) + } } change.Flags = []string{`\Subscribed`} }