diff --git a/docs/content/docs/development/troubleshooting.mdx b/docs/content/docs/development/troubleshooting.mdx index 38aa84387..6f14e24ab 100644 --- a/docs/content/docs/development/troubleshooting.mdx +++ b/docs/content/docs/development/troubleshooting.mdx @@ -80,7 +80,7 @@ Newer builds return the invite-only refusal with its own machine code, `registra | A mailbox stalls after about an hour | The worker is missing `BOX_GOOGLE_*` or `BOX_OUTLOOK_*`. The backend starts the OAuth flow but each worker refreshes the token. Set them and restart the worker | | An IMAP mailbox connects but no mail ever arrives | Check the sync card in the mailbox drawer for a folder count. Zero folders on a reachable server means the server refused `STATUS` for every folder; the worker log names each one it skipped. A connected mailbox whose inbox is genuinely empty is normal | | An IMAP mailbox stopped syncing and the log is quiet | Sessions dropped by the server, or by a firewall that removed the mapping without closing the connection, are re-dialed on the next pass, and every pass that cannot reach the server is retried on a widening interval up to five minutes. If a mailbox is still stuck, `make logs worker` shows the folder cursors; a mailbox held by the sync budget says so in its drawer instead | -| A folder is missing from the unibox | Up to `100` folders per mailbox are synced (**Instance settings > Limits**). Past that, the inbox and the special folders are kept and the rest follow the server's order, with a warning in the drawer naming how many were left out. Gmail's All Mail, Starred and Important are label views over other folders and are deliberately never synced | +| A folder is missing from the unibox | Up to `100` folders per mailbox are synced (**Instance settings > Limits**). Past that, the inbox and the special folders are kept and the rest follow the server's order, with a warning in the drawer naming how many were left out. Gmail's All Mail, Starred and Important are label views over other folders and are deliberately never synced. A folder can also be skipped when the mail server gives it the same internal id (`UIDVALIDITY`) as another folder, which happens on servers that derive that id from the creation time; the drawer says so, and renaming or recreating the folder gives it a new one | | Scheduled sends never fire | Delayed sends run through the in-process Postgres task poller (`TASKS_PROVIDER=local`), so the backend must be running | | Every send dead-letters with `permission denied` on `/data/blobs` | The `blobs` volume was created before the images owned that path, so it is still `root:root` while the services run as uid 1000. Fix it once with `docker compose -p warmbly exec -u root backend chown -R warmbly:warmbly /data/blobs`. The `blob_fs_root` health check reports it, and volumes created from current images are already correct | | `email account not found in worker` | The mailbox is assigned to a worker that no longer exists, usually because the worker was recreated and came back with a fresh UUID. The reconciler releases and re-places it on a live worker within its interval. Compose workers now keep their id in the `worker_state` volume (`WORKER_STATE_DIR`), so this stops recurring once that volume exists; removing the volume or unsetting `WORKER_STATE_DIR` reintroduces the churn. See [worker identity](/development/deployment-guide/#worker-identity) | diff --git a/internal/app/worker/wmail/imap_conn.go b/internal/app/worker/wmail/imap_conn.go index a7f8e11bd..a245cd41b 100644 --- a/internal/app/worker/wmail/imap_conn.go +++ b/internal/app/worker/wmail/imap_conn.go @@ -17,8 +17,10 @@ import ( type ImapConn interface { // Sync pass. Folders() ([]models.Mailbox, *errx.MailError) - // FolderOverflow is how many folders the last listing left out. + // FolderOverflow is how many folders the last listing left out for the + // cap, FolderConflicts how many it left out for a duplicate UIDVALIDITY. FolderOverflow() int + FolderConflicts() int // HasCondStore picks the incremental strategy: mod-sequences when the // server has CONDSTORE, UIDNEXT plus a periodic flag scan when it does not. HasCondStore() bool diff --git a/internal/app/worker/wmail/sync_imap_flags.go b/internal/app/worker/wmail/sync_imap_flags.go index 3a8eb6752..0ef97dcb5 100644 --- a/internal/app/worker/wmail/sync_imap_flags.go +++ b/internal/app/worker/wmail/sync_imap_flags.go @@ -16,12 +16,21 @@ import ( // condition is static until someone reorganizes their mail, and a warning a // minute would bury every real error in the drawer. func (w *WMail) reportFolderOverflow() { - left := w.SmtpImapData.ImapClient.FolderOverflow() - if left <= 0 || w.SmtpImapData.overflowReported { + if w.SmtpImapData.overflowReported { + return + } + client := w.SmtpImapData.ImapClient + over, conflicts := client.FolderOverflow(), client.FolderConflicts() + if over <= 0 && conflicts <= 0 { return } w.SmtpImapData.overflowReported = true - w.CaptureError(errx.ErrMailFoldersOverflow(left)) + if over > 0 { + w.CaptureError(errx.ErrMailFoldersOverflow(over)) + } + if conflicts > 0 { + w.CaptureError(errx.ErrMailFoldersConflict(conflicts)) + } } // imapScanFlags mirrors read state and flag changes on a server without diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index ece63c318..0d23e60b0 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -20,11 +20,12 @@ import ( // rather than silently pass. type fakeImapConn struct { ImapConn - folders []models.Mailbox - changed []goimap.UID - fetches int - released int - overflow int + folders []models.Mailbox + changed []goimap.UID + fetches int + released int + overflow int + conflicts int // noCondStore drives the UIDNEXT path instead of the mod-sequence one. noCondStore bool flags map[uint32]imap.FlagState @@ -33,7 +34,8 @@ type fakeImapConn struct { func (c *fakeImapConn) Folders() ([]models.Mailbox, *errx.MailError) { return c.folders, nil } -func (c *fakeImapConn) FolderOverflow() int { return c.overflow } +func (c *fakeImapConn) FolderOverflow() int { return c.overflow } +func (c *fakeImapConn) FolderConflicts() int { return c.conflicts } // condStore defaults to true: most of these tests exercise the mod-sequence // path, and the UIDNEXT path has its own tests. @@ -284,6 +286,7 @@ type backfillImapConn struct { func (c *backfillImapConn) Folders() ([]models.Mailbox, *errx.MailError) { return c.folders, nil } func (c *backfillImapConn) FolderOverflow() int { return 0 } +func (c *backfillImapConn) FolderConflicts() int { return 0 } func (c *backfillImapConn) HasCondStore() bool { return true } func (c *backfillImapConn) ReleaseMailbox() {} diff --git a/internal/client/smtpimap/imap/client.go b/internal/client/smtpimap/imap/client.go index d2a37738d..cee27d4c4 100644 --- a/internal/client/smtpimap/imap/client.go +++ b/internal/client/smtpimap/imap/client.go @@ -90,8 +90,10 @@ type Client struct { // incremental sync. condStore atomic.Bool - // folderOverflow is what the last Folders call had to leave out. - folderOverflow atomic.Int32 + // folderOverflow is what the last Folders call had to leave out for the + // cap, folderConflicts what it left out for a duplicate UIDVALIDITY. + folderOverflow atomic.Int32 + folderConflicts atomic.Int32 } // begin starts the idle clock for one command; call the result on exit. diff --git a/internal/client/smtpimap/imap/folders.go b/internal/client/smtpimap/imap/folders.go index 241f36ee3..8a6c78216 100644 --- a/internal/client/smtpimap/imap/folders.go +++ b/internal/client/smtpimap/imap/folders.go @@ -112,15 +112,54 @@ func (c *Client) foldersCapped(limit int) ([]models.Mailbox, *errx.MailError) { box.HighestModSeq = st.HighestModSeq resp = append(resp, box) } + + resp, conflicts := dedupeByUIDValidity(resp) + c.folderConflicts.Store(int32(conflicts)) return resp, nil } +// dedupeByUIDValidity keeps one folder per UIDVALIDITY. +// +// Everything downstream identifies a folder by that number, including the +// primary key of the stored folder row, but RFC 3501 only promises UIDs are +// stable within one folder: Dovecot and others derive UIDVALIDITY from the +// creation time, so a folder tree created in the same second shares one. +// Two folders under a single id would advance each other's cursor and delete +// each other's row, which loses mail. Dropping the later one leaves it +// unsynced (and says so) but leaves every other folder correct. Input is +// already ranked, so the inbox and the special folders win any collision. +func dedupeByUIDValidity(boxes []models.Mailbox) ([]models.Mailbox, int) { + seen := make(map[uint32]string, len(boxes)) + kept := boxes[:0] + conflicts := 0 + for _, box := range boxes { + if other, dup := seen[box.UIDValidity]; dup { + log.Warn(). + Str("folder", box.Name). + Str("conflicts_with", other). + Uint32("uid_validity", box.UIDValidity). + Msg("imap: two folders report the same UIDVALIDITY; the second is not synced") + conflicts++ + continue + } + seen[box.UIDValidity] = box.Name + kept = append(kept, box) + } + return kept, conflicts +} + // FolderOverflow is how many selectable folders the last Folders call left // out because the account has more than config.MaxEmailFolders. func (c *Client) FolderOverflow() int { return int(c.folderOverflow.Load()) } +// FolderConflicts is how many folders the last Folders call left out because +// another folder reported the same UIDVALIDITY. +func (c *Client) FolderConflicts() int { + return int(c.folderConflicts.Load()) +} + // rankFolders keeps INBOX first, then the special folders (sent, drafts, // junk, trash, archive), then the rest in the server's order, and cuts the // list at cap. The order within each rank is the server's, so the result is diff --git a/internal/client/smtpimap/imap/folders_test.go b/internal/client/smtpimap/imap/folders_test.go new file mode 100644 index 000000000..1007d5ee7 --- /dev/null +++ b/internal/client/smtpimap/imap/folders_test.go @@ -0,0 +1,188 @@ +package imap + +import ( + "testing" + + "github.com/warmbly/warmbly/internal/models" +) + +// Everything downstream identifies a folder by its UIDVALIDITY, and the +// database keys the folder row on it, but RFC 3501 only promises UIDs are +// stable within one folder: Dovecot and others derive UIDVALIDITY from the +// creation time, so a folder tree created in one second shares one. Two +// folders under a single id would advance each other's cursor and delete each +// other's row, so only one is followed. +func TestRankFoldersPutsInboxAndSpecialFoldersFirst(t *testing.T) { + all := []models.Mailbox{ + {Name: "Projects"}, + {Name: "Archive"}, + {Name: "INBOX"}, + {Name: "Notes"}, + {Name: "Sent", Attrs: []string{"\\Sent"}}, + } + kept, overflow := rankFolders(all, 3) + if overflow != 2 { + t.Fatalf("overflow = %d, want 2", overflow) + } + if kept[0].Name != "INBOX" { + t.Fatalf("kept[0] = %q, want INBOX to survive any cap", kept[0].Name) + } + for _, want := range []string{"Sent", "Archive"} { + found := false + for _, b := range kept { + if b.Name == want { + found = true + } + } + if !found { + t.Errorf("%q was cut in favour of a plain user folder", want) + } + } +} + +// The cap must not reorder a listing that fits, because the order it keeps is +// the one the folder rows are written in. +func TestRankFoldersKeepsEverythingUnderTheCap(t *testing.T) { + all := []models.Mailbox{{Name: "INBOX"}, {Name: "Work"}, {Name: "Sent"}} + kept, overflow := rankFolders(all, 10) + if overflow != 0 || len(kept) != 3 { + t.Fatalf("kept %d folders with overflow %d, want all 3 and no overflow", len(kept), overflow) + } +} + +// A container the server lists only to show hierarchy cannot be SELECTed, so +// following it would fail every pass. +func TestSelectableFolder(t *testing.T) { + for _, tc := range []struct { + attrs []string + want bool + }{ + {[]string{"\\HasChildren"}, true}, + {nil, true}, + {[]string{"\\Noselect"}, false}, + {[]string{"\\NonExistent"}, false}, + {[]string{"\\NoSelect", "\\HasChildren"}, false}, + } { + if got := selectableFolder(tc.attrs); got != tc.want { + t.Errorf("selectableFolder(%v) = %v, want %v", tc.attrs, got, tc.want) + } + } +} + +// A server that publishes no special-use attributes reports its folders in +// the mailbox owner's language, and an unmatched Sent folder means the +// customer's sent mail shows up in their inbox instead. +func TestCanonicalFolderLocalizedNames(t *testing.T) { + for _, tc := range []struct { + name string + want string + }{ + {"Gesendete Elemente", models.FolderSent}, + {"Éléments envoyés", models.FolderSent}, + {"Elementos enviados", models.FolderSent}, + {"Elküldött elemek", models.FolderSent}, + {"INBOX.Papierkorb", models.FolderTrash}, + {"Corbeille", models.FolderTrash}, + {"Posta indesiderata", models.FolderSpam}, + {"Skräppost", models.FolderSpam}, + {"Entwürfe", models.FolderDrafts}, + {"Archiwum", models.FolderArchive}, + // A user folder that merely mentions a role stays where it is. + {"Spam reports", models.FolderInbox}, + {"Sent to legal", models.FolderInbox}, + {"Clients/Acme", models.FolderInbox}, + } { + if got := CanonicalFolder(tc.name, nil); got != tc.want { + t.Errorf("CanonicalFolder(%q) = %q, want %q", tc.name, got, tc.want) + } + } +} + +// The spam guard decides whether a warmup action may MOVE a message out of a +// folder, so a folder that merely mentions spam must not qualify. +func TestIsSpamMailboxName(t *testing.T) { + for _, tc := range []struct { + name string + want bool + }{ + {"Spam", true}, + {"Junk", true}, + {"[Gmail]/Spam", true}, + {"INBOX.Junk E-Mail", true}, + {"Courrier indésirable", true}, + {"Spam reports", false}, + {"Junk drawer", false}, + {"INBOX", false}, + } { + if got := IsSpamMailboxName(tc.name); got != tc.want { + t.Errorf("IsSpamMailboxName(%q) = %v, want %v", tc.name, got, tc.want) + } + } +} + +// Gmail's label views duplicate every message under another UID; following +// one would re-file known mail as archive and swap the (mailbox, uid) pair +// the warmup actions address. +func TestIsVirtualFolder(t *testing.T) { + for _, tc := range []struct { + name string + attrs []string + want bool + }{ + {"[Gmail]/All Mail", []string{"\\All"}, true}, + {"[Gmail]/All Mail", []string{"\\HasNoChildren"}, true}, + {"[Google Mail]/Important", nil, true}, + {"[Gmail]/Starred", nil, true}, + {"[Gmail]/Sent Mail", []string{"\\Sent"}, false}, + {"[Gmail]/Bin", []string{"\\Trash"}, false}, + {"Important", nil, false}, + {"All Mail", nil, false}, + {"INBOX", nil, false}, + } { + if got := IsVirtualFolder(tc.name, tc.attrs); got != tc.want { + t.Errorf("IsVirtualFolder(%q, %v) = %v, want %v", tc.name, tc.attrs, got, tc.want) + } + } +} + +// Two folders sharing a UIDVALIDITY is a server doing something RFC 3501 does +// not forbid but everything downstream assumes away: the folder row is keyed +// on it. Following both would advance each other's cursor and delete each +// other's row, so one is dropped and reported. +func TestDedupeByUIDValidity(t *testing.T) { + kept, conflicts := dedupeByUIDValidity([]models.Mailbox{ + {Name: "INBOX", UIDValidity: 100}, + {Name: "Sent", UIDValidity: 200}, + // Created in the same second as Sent on a server that stamps + // UIDVALIDITY with the creation time. + {Name: "Projects", UIDValidity: 200}, + {Name: "Notes", UIDValidity: 300}, + }) + if conflicts != 1 { + t.Fatalf("conflicts = %d, want 1", conflicts) + } + if len(kept) != 3 { + t.Fatalf("kept %d folders, want 3", len(kept)) + } + // The ranked order puts the special folder first, so Sent is the one that + // survives and the plain user folder is the one dropped. + if kept[1].Name != "Sent" { + t.Errorf("kept[1] = %q, want Sent to win the collision", kept[1].Name) + } + seen := map[uint32]bool{} + for _, b := range kept { + if seen[b.UIDValidity] { + t.Fatalf("UIDVALIDITY %d survived twice", b.UIDValidity) + } + seen[b.UIDValidity] = true + } +} + +// The common case must not allocate a conflict or reorder anything. +func TestDedupeByUIDValidityLeavesADistinctListingAlone(t *testing.T) { + in := []models.Mailbox{{Name: "INBOX", UIDValidity: 1}, {Name: "Sent", UIDValidity: 2}} + kept, conflicts := dedupeByUIDValidity(in) + if conflicts != 0 || len(kept) != 2 || kept[0].Name != "INBOX" || kept[1].Name != "Sent" { + t.Fatalf("a listing with distinct ids was changed: %+v, conflicts %d", kept, conflicts) + } +} diff --git a/internal/errx/email.go b/internal/errx/email.go index aea5824fe..9a27040bf 100644 --- a/internal/errx/email.go +++ b/internal/errx/email.go @@ -126,6 +126,12 @@ var ( ErrMailFoldersOverflow = func(left int) *MailError { return MError(MailErrorWarning, MailErrorCodeFolderLimit, fmt.Sprintf("This mailbox has %d more folders than the %d Warmbly follows. The inbox, sent, drafts, spam and trash are always synced; the rest are taken in the server's order.", left, config.MaxEmailFolders), MailErrorResolveMethodNone) } + // ErrMailFoldersConflict is relayed when the mail server gives two + // folders the same UIDVALIDITY, which is the id everything downstream + // identifies a folder by. Only one of them can be followed. + ErrMailFoldersConflict = func(left int) *MailError { + return MError(MailErrorWarning, MailErrorCodeFolderLimit, fmt.Sprintf("%d folder(s) on this mailbox share an internal id with another folder, which their mail server should not do, so only one of each pair is synced. Renaming or recreating the folder usually gives it a new id.", left), MailErrorResolveMethodNone) + } ErrMailUpdateLimit = MError(MailErrorCritical, MailErrorCodeUpdateLimit, "Your inbox has received an unusually large number of updates. Please reactivate your inbox once the issue is resolved.", MailErrorResolveMethodReload) ErrMailGoogleAuth = MError(MailErrorCritical, MailErrorCodeGoogleAuth, "Cannot access your Gmail account. Please re-authorize your account to restore mailbox access.", MailErrorResolveMethodReload) ErrMailGooglePayment = MError(MailErrorCritical, MailErrorCodeGooglePayment, "Gmail access blocked due to unpaid invoices. Please resolve the payment with Google.", MailErrorResolveMethodReload)