fix(tui): surface fetchFolders errors#1267
Open
mvanhorn wants to merge 2 commits into
Open
Conversation
Closes floatpane#1251 GetCachedEmailBody is a read operation but updated LastAccessedAt and rewrote the cache file on every hit, violating the read-doesn't-mutate boundary called out in floatpane#1251. main.go calls GetCachedEmailBody twice per email view (in UpdatePreviewMsg and ViewEmailMsg) and both call sites already chase the cached read with SaveEmailBody, which is the function responsible for stamping LastAccessedAt and persisting. Removing the duplicate work here also removes one disk write per email view. Drops the two lines flagged in the issue (config/cache.go:505-506) and adds a one-line note above the function pointing readers at SaveEmailBody as the access-time owner.
… dropping fetchFoldersCmd spawns one goroutine per account, each calling fetcher.FetchFolders. The previous implementation discarded any error returned by FetchFolders -- on broken IMAP login, expired OAuth, or unreachable server, the affected account silently dropped out of the merged folder list and the user got no signal. The reporter (issue floatpane#1125) flagged this as a UX bug: the user sees their folder list miss entries with no explanation. Collect per-account errors into a sibling map and pass them through FoldersFetchedMsg so the TUI can decide what to do with them. The consumer at main.go:510 now surfaces non-empty errors as a transient overlay (4s, then auto-restore) using the same PluginNotifyMsg pattern the rest of the TUI uses for transient status: save the previous model, swap to a Status overlay, fire RestoreViewMsg via tea.Tick. Account display name lookup prefers Email -> Name -> ID so the error string shows something meaningful even on partially configured accounts. Errors are sorted alphabetically so the overlay is deterministic across restarts. The disjoint-by-construction property (an account either populates FoldersByAccount or Errors, never both) is preserved by the existing return-after-error in the goroutine. Closes floatpane#1125. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
floatpanebot
previously requested changes
May 10, 2026
Member
floatpanebot
left a comment
There was a problem hiding this comment.
Hi @mvanhorn! Please fix the following issues with your PR:
- Title: Is too long (78 characters). The PR title must be strictly under 40 characters.
- Body: Missing the
## What?or## Why?headings required by the PR template.
Formatting issues have been resolved. Thank you!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Per-account
fetcher.FetchFolderserrors are now collected intoFoldersFetchedMsg.Errorsand surfaced as a transient overlay in theTUI (4s, then auto-restore via the existing
PluginNotifyMsgpattern).Closes #1125.
Why?
fetchFoldersCmdspawns one goroutine per account, each callingfetcher.FetchFolders. The previous code returned immediately on errand the goroutine vanished without leaving a trace. If an account's
IMAP login was broken, OAuth had expired, or the server was
unreachable, the affected account silently dropped out of the merged
folder list and the user got no signal -- their folder list quietly
missed entries. The reporter described this exactly: "user sees the
folder list silently miss those folders and never gets a notification."
How
tui/messages.goErrors map[string]errorfield onFoldersFetchedMsg.main.go(producer at fetchFoldersCmd)main.go(consumer at FoldersFetchedMsg case)Errorsis non-empty, surface a transient overlay listing affected accounts and their errors.The producer change is a 7-line localized capture: same goroutine, same
mutex, same return-after-error path -- the only difference is that the
err is stored before returning instead of being thrown away.
The consumer surfaces errors using the same pattern the codebase already
uses for
PluginNotifyMsg: savem.previousModel, swapm.currenttoa
tui.NewStatus(...)overlay, firetea.Tick(4*time.Second, ...)thatreturns
RestoreViewMsg. After 4 seconds the user is back to wherethey were. No new TUI primitives, no new message types beyond the field
on the existing message.
Account display name lookup prefers
Email->Name->IDsopartially configured accounts still show something meaningful in the
overlay. Errors are sorted alphabetically so the rendered list is
deterministic across restarts (the previous, per-iteration map order
would jitter).
The disjoint property (an account is in either
FoldersByAccountorErrors, never both) is preserved by the existingreturnafterrecording the error.
Verification
I did not add automated tests for the TUI surface itself -- the existing
test suite has no tests for
main.go's message handlers (verified viafind . -name '*_test.go'), so introducing the first one felt likescope creep for a fix PR. Happy to add a coverage test for the
disjoint-by-construction property in
fetchFoldersCmdif you'd like;just point me at the testing pattern you'd want.