feat(errors): per-account error modal with persisted error list
Accounts finishing done_with_errors showed only a count and a single
last_error. This adds a modal listing every concrete error of the
account's most recent run.
- migration 0005: account_errors table (kind folder|message|account,
folder, message_ref, error, created_at; ON DELETE CASCADE; indexed)
- store: AddAccountError / ClearAccountErrors / ListAccountErrors
- copy: OnError callback captures per-message error text (previously
only counted), with a "UID N: subject" reference
- orchestrator: clear errors at run start; persist folder/message/
account errors; cap 500 rows/account/run with a suppressed-note row
- api: GET /api/tasks/{id}/accounts/{accountId}/errors
- web: AccountErrorsModal, clickable ERRORS count, api + styles
Verified: migration applies on Postgres 18; store add/list/clear and
cascade tests pass against real pg; backend build/vet/test green; web
tsc+vite build and oxlint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9Eq4JtWjyNTv5qat3B3mM
This commit is contained in:
@@ -226,6 +226,31 @@ func (s *Server) handleProbeAccountFolders(w http.ResponseWriter, r *http.Reques
|
||||
})
|
||||
}
|
||||
|
||||
// handleListAccountErrors returns the individual errors recorded for an account
|
||||
// during its most recent run, for the per-account error modal.
|
||||
func (s *Server) handleListAccountErrors(w http.ResponseWriter, r *http.Request) {
|
||||
taskID, err := pathID(r, "id")
|
||||
if err != nil {
|
||||
http.Error(w, "bad id", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
accID, err := pathID(r, "accountId")
|
||||
if err != nil {
|
||||
http.Error(w, "bad account id", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
if _, ok := s.findAccount(r, taskID, accID); !ok {
|
||||
http.Error(w, "account not found", http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
errs, err := s.store.ListAccountErrors(r.Context(), accID)
|
||||
if err != nil {
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, errs)
|
||||
}
|
||||
|
||||
// handleSetAccountFolderMapping persists one account's rename map + excluded set.
|
||||
func (s *Server) handleSetAccountFolderMapping(w http.ResponseWriter, r *http.Request) {
|
||||
taskID, err := pathID(r, "id")
|
||||
|
||||
@@ -24,6 +24,7 @@ func (s *Server) Router() http.Handler {
|
||||
api.HandleFunc("PUT /api/tasks/{id}/folder-mapping", s.handleSetFolderMapping)
|
||||
api.HandleFunc("PUT /api/tasks/{id}/schedule", s.handleSetSchedule)
|
||||
api.HandleFunc("GET /api/tasks/{id}/runs", s.handleListRuns)
|
||||
api.HandleFunc("GET /api/tasks/{id}/accounts/{accountId}/errors", s.handleListAccountErrors)
|
||||
api.HandleFunc("DELETE /api/tasks/{id}/accounts/{accountId}", s.handleDeleteAccount)
|
||||
api.HandleFunc("POST /api/tasks/{id}/import", s.handleImportCSV)
|
||||
api.HandleFunc("POST /api/tasks/{id}/test", s.handleTestAccounts)
|
||||
|
||||
+29
-1
@@ -27,6 +27,11 @@ type CopyDeps struct {
|
||||
// folder's messages have been examined so far — so the UI shows movement
|
||||
// while dedup decisions are made, before bodies start copying.
|
||||
OnScan func(scanned, total int64)
|
||||
// OnError is called for each message-level error, with a message reference
|
||||
// ("UID N: subject", empty when the envelope is unavailable) and the error
|
||||
// text — so the orchestrator can persist individual errors for the
|
||||
// per-account error modal. Folder-level errors are reported by the caller.
|
||||
OnError func(ref, msg string)
|
||||
}
|
||||
|
||||
// CopyResult summarizes the outcome of one CopyFolder run.
|
||||
@@ -44,6 +49,15 @@ type CopyResult struct {
|
||||
// checked between windows.
|
||||
const metaScanBatch = 1000
|
||||
|
||||
// msgRef builds a human-readable reference for a message error: its UID plus
|
||||
// subject when known, e.g. "UID 42: Invoice". Falls back to just the UID.
|
||||
func msgRef(uid imap.UID, subject string) string {
|
||||
if subject == "" {
|
||||
return fmt.Sprintf("UID %d", uid)
|
||||
}
|
||||
return fmt.Sprintf("UID %d: %s", uid, subject)
|
||||
}
|
||||
|
||||
// metaBatches tiles 1..total into contiguous, non-overlapping windows of at
|
||||
// most batchSize, covering every sequence number exactly once.
|
||||
func metaBatches(total, batchSize uint32) []imap.SeqRange {
|
||||
@@ -95,11 +109,17 @@ func CopyFolder(ctx context.Context, src, dst *imapclient.Client, srcFolder, dst
|
||||
type queued struct {
|
||||
uid imap.UID
|
||||
key string
|
||||
subject string
|
||||
flags []imap.Flag
|
||||
internalDate time.Time
|
||||
}
|
||||
var todo []queued
|
||||
var scanned int64
|
||||
reportErr := func(ref, msg string) {
|
||||
if deps.OnError != nil {
|
||||
deps.OnError(ref, msg)
|
||||
}
|
||||
}
|
||||
// Scan metadata in bounded windows instead of one FETCH 1:*, so each
|
||||
// command is short (the server stays responsive) and ctx is checked on
|
||||
// every window boundary — not just between messages of one giant command.
|
||||
@@ -122,20 +142,26 @@ func CopyFolder(ctx context.Context, src, dst *imapclient.Client, srcFolder, dst
|
||||
buf, err := msg.Collect()
|
||||
if err != nil {
|
||||
res.Errors++
|
||||
reportErr("", "read message metadata: "+err.Error())
|
||||
continue
|
||||
}
|
||||
scanned++
|
||||
key := MessageKey(buf.Envelope, buf.RFC822Size)
|
||||
subject := ""
|
||||
if buf.Envelope != nil {
|
||||
subject = buf.Envelope.Subject
|
||||
}
|
||||
already, err := deps.IsMigrated(key)
|
||||
if err != nil {
|
||||
res.Errors++
|
||||
reportErr(msgRef(buf.UID, subject), "dedup lookup: "+err.Error())
|
||||
} else if already {
|
||||
res.Skipped++
|
||||
if deps.OnProgress != nil {
|
||||
deps.OnProgress(res.Copied, res.Skipped)
|
||||
}
|
||||
} else {
|
||||
todo = append(todo, queued{uid: buf.UID, key: key, flags: buf.Flags, internalDate: buf.InternalDate})
|
||||
todo = append(todo, queued{uid: buf.UID, key: key, subject: subject, flags: buf.Flags, internalDate: buf.InternalDate})
|
||||
}
|
||||
if deps.OnScan != nil {
|
||||
deps.OnScan(scanned, total)
|
||||
@@ -153,10 +179,12 @@ func CopyFolder(ctx context.Context, src, dst *imapclient.Client, srcFolder, dst
|
||||
}
|
||||
if err := streamOne(src, dst, dstFolder, q.uid, q.flags, q.internalDate); err != nil {
|
||||
res.Errors++
|
||||
reportErr(msgRef(q.uid, q.subject), "copy message: "+err.Error())
|
||||
continue
|
||||
}
|
||||
if err := deps.MarkMigrated(dstFolder, q.key); err != nil {
|
||||
res.Errors++
|
||||
reportErr(msgRef(q.uid, q.subject), "mark migrated: "+err.Error())
|
||||
continue
|
||||
}
|
||||
res.Copied++
|
||||
|
||||
@@ -16,6 +16,12 @@ import (
|
||||
var ErrNotTested = errors.New("accounts not fully tested")
|
||||
var ErrAlreadyRunning = errors.New("task already running")
|
||||
|
||||
// maxAccountErrors caps how many individual error rows one account records per
|
||||
// run, so a corrupt mailbox producing thousands of failures can't bloat the
|
||||
// account_errors table. The cap'th row is a synthetic "further errors
|
||||
// suppressed" note.
|
||||
const maxAccountErrors = 500
|
||||
|
||||
// folderPlan is one source folder scheduled for copy and its destination name.
|
||||
type folderPlan struct {
|
||||
src, dst string
|
||||
@@ -264,6 +270,7 @@ func (o *Orchestrator) runAccount(ctx context.Context, task store.Task, runID in
|
||||
_ = o.store.SetAccountStatus(ctx, a.ID, "running")
|
||||
_ = o.store.SetAccountError(ctx, a.ID, "") // clear any error from a previous run
|
||||
_ = o.store.ResetAccountCounters(ctx, a.ID) // start from zero; IncAccountCounters is additive
|
||||
_ = o.store.ClearAccountErrors(ctx, a.ID) // drop last run's per-error rows
|
||||
|
||||
// Per-account cancellable context: IMAP work uses actx (so CancelAccount
|
||||
// stops it); DB writes keep the parent ctx so status/counters persist even
|
||||
@@ -277,28 +284,28 @@ func (o *Orchestrator) runAccount(ctx context.Context, task store.Task, runID in
|
||||
|
||||
srcPass, err := crypto.Decrypt(o.encKey, a.SrcPassEnc)
|
||||
if err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "src", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "src", err)
|
||||
}
|
||||
dstPass, err := crypto.Decrypt(o.encKey, a.DstPassEnc)
|
||||
if err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "dst", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "dst", err)
|
||||
}
|
||||
|
||||
src, err := imapx.Connect(actx, srcEP)
|
||||
if err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "src", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "src", err)
|
||||
}
|
||||
defer func() { _ = src.Logout().Wait() }()
|
||||
if err := src.Login(a.SrcLogin, string(srcPass)).Wait(); err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "src", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "src", err)
|
||||
}
|
||||
dst, err := imapx.Connect(actx, dstEP)
|
||||
if err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "dst", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "dst", err)
|
||||
}
|
||||
defer func() { _ = dst.Logout().Wait() }()
|
||||
if err := dst.Login(a.DstLogin, string(dstPass)).Wait(); err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "dst", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "dst", err)
|
||||
}
|
||||
|
||||
// On cancel, close the connections so any in-flight network read (a slow
|
||||
@@ -311,7 +318,7 @@ func (o *Orchestrator) runAccount(ctx context.Context, task store.Task, runID in
|
||||
|
||||
folders, err := imapx.ListFolders(src)
|
||||
if err != nil {
|
||||
return o.accountFailed(ctx, task.ID, a, srcEP, dstEP, "src", err)
|
||||
return o.accountFailed(ctx, task.ID, runID, a, srcEP, dstEP, "src", err)
|
||||
}
|
||||
|
||||
// Planning pass: decide folders from the account's own config, then EXAMINE
|
||||
@@ -341,9 +348,24 @@ func (o *Orchestrator) runAccount(ctx context.Context, task store.Task, runID in
|
||||
var curFolder string
|
||||
var curTotal int64
|
||||
var lastEmit, lastScanEmit time.Time
|
||||
// Persist individual errors for the per-account error modal, capped so a
|
||||
// corrupt mailbox can't write unbounded rows. Runs on this goroutine, so
|
||||
// the counter is race-free (same reasoning as the progress vars above).
|
||||
var persistedErrs int
|
||||
addErr := func(kind, folder, ref, msg string) {
|
||||
persistedErrs++
|
||||
switch {
|
||||
case persistedErrs < maxAccountErrors:
|
||||
_ = o.store.AddAccountError(ctx, a.ID, runID, kind, folder, ref, msg)
|
||||
case persistedErrs == maxAccountErrors:
|
||||
_ = o.store.AddAccountError(ctx, a.ID, runID, "account", "", "",
|
||||
"too many errors — further errors suppressed")
|
||||
}
|
||||
}
|
||||
deps := imapx.CopyDeps{
|
||||
IsMigrated: func(k string) (bool, error) { return o.store.IsMigrated(ctx, a.ID, k) },
|
||||
MarkMigrated: func(folder, k string) error { return o.store.MarkMigrated(ctx, a.ID, folder, k) },
|
||||
OnError: func(ref, msg string) { addErr("message", curFolder, ref, msg) },
|
||||
OnProgress: func(c, s int) {
|
||||
now := time.Now()
|
||||
done := c + s
|
||||
@@ -393,6 +415,7 @@ func (o *Orchestrator) runAccount(ctx context.Context, task store.Task, runID in
|
||||
slog.Warn("folder copy error", "account", a.ID, "src_login", a.SrcLogin, "folder", fp.src, "err", err)
|
||||
folderErr = 1
|
||||
_ = o.store.SetAccountError(ctx, a.ID, "folder \""+fp.src+"\": "+err.Error())
|
||||
addErr("folder", fp.src, "", err.Error())
|
||||
o.hub.Publish(wshub.Event{Type: "error", TaskID: task.ID, Data: map[string]any{
|
||||
"account_id": a.ID, "src_login": a.SrcLogin, "folder": fp.src, "error": err.Error(),
|
||||
}})
|
||||
@@ -428,7 +451,7 @@ func (o *Orchestrator) runAccount(ctx context.Context, task store.Task, runID in
|
||||
return copied, skipped, errs
|
||||
}
|
||||
|
||||
func (o *Orchestrator) accountFailed(ctx context.Context, taskID int64, a store.Account, srcEP, dstEP imapx.Endpoint, side string, err error) (int64, int64, int64) {
|
||||
func (o *Orchestrator) accountFailed(ctx context.Context, taskID, runID int64, a store.Account, srcEP, dstEP imapx.Endpoint, side string, err error) (int64, int64, int64) {
|
||||
// A cancellation surfacing as an error is a cancel, not a failure.
|
||||
if errors.Is(err, context.Canceled) {
|
||||
_ = o.store.SetAccountStatus(ctx, a.ID, "cancelled")
|
||||
@@ -442,7 +465,9 @@ func (o *Orchestrator) accountFailed(ctx context.Context, taskID int64, a store.
|
||||
}
|
||||
slog.Error("account failed", "account", a.ID, "side", side, "login", login, "host", host, "port", port, "err", err)
|
||||
_ = o.store.SetAccountStatus(ctx, a.ID, "error")
|
||||
_ = o.store.SetAccountError(ctx, a.ID, side+" "+login+"@"+host+": "+err.Error())
|
||||
failMsg := side + " " + login + "@" + host + ": " + err.Error()
|
||||
_ = o.store.SetAccountError(ctx, a.ID, failMsg)
|
||||
_ = o.store.AddAccountError(ctx, a.ID, runID, "account", "", "", failMsg)
|
||||
o.hub.Publish(wshub.Event{Type: "error", TaskID: taskID,
|
||||
Data: map[string]any{"account_id": a.ID, "side": side, "login": login, "host": host, "port": port, "error": err.Error()}})
|
||||
return 0, 0, 1
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"time"
|
||||
)
|
||||
|
||||
// AccountError is one concrete error recorded during an account's run —
|
||||
// folder-level, message-level, or account-level (connect/login).
|
||||
type AccountError struct {
|
||||
ID int64 `json:"id"`
|
||||
AccountID int64 `json:"account_id"`
|
||||
RunID int64 `json:"run_id"`
|
||||
Kind string `json:"kind"` // folder | message | account
|
||||
Folder string `json:"folder"`
|
||||
MessageRef string `json:"message_ref"`
|
||||
Error string `json:"error"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
}
|
||||
|
||||
// AddAccountError appends one error row for an account's current run.
|
||||
func (s *Store) AddAccountError(ctx context.Context, accountID, runID int64, kind, folder, ref, msg string) error {
|
||||
_, err := s.Pool.Exec(ctx,
|
||||
`INSERT INTO account_errors (account_id, run_id, kind, folder, message_ref, error)
|
||||
VALUES ($1,$2,$3,$4,$5,$6)`,
|
||||
accountID, runID, kind, folder, ref, msg)
|
||||
return err
|
||||
}
|
||||
|
||||
// ClearAccountErrors removes an account's errors at the start of a run, so the
|
||||
// list reflects only the current run (mirrors ResetAccountCounters).
|
||||
func (s *Store) ClearAccountErrors(ctx context.Context, accountID int64) error {
|
||||
_, err := s.Pool.Exec(ctx, `DELETE FROM account_errors WHERE account_id=$1`, accountID)
|
||||
return err
|
||||
}
|
||||
|
||||
// ListAccountErrors returns an account's errors in insertion order, for the
|
||||
// per-account error modal.
|
||||
func (s *Store) ListAccountErrors(ctx context.Context, accountID int64) ([]AccountError, error) {
|
||||
rows, err := s.Pool.Query(ctx,
|
||||
`SELECT id, account_id, run_id, kind, folder, message_ref, error, created_at
|
||||
FROM account_errors WHERE account_id=$1 ORDER BY id`, accountID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
out := []AccountError{}
|
||||
for rows.Next() {
|
||||
var e AccountError
|
||||
var runID *int64
|
||||
if err := rows.Scan(&e.ID, &e.AccountID, &runID, &e.Kind, &e.Folder,
|
||||
&e.MessageRef, &e.Error, &e.CreatedAt); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if runID != nil {
|
||||
e.RunID = *runID
|
||||
}
|
||||
out = append(out, e)
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
@@ -0,0 +1,75 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// AddAccountError persists individual errors; ListAccountErrors returns them in
|
||||
// insertion order; ClearAccountErrors wipes them for the next run.
|
||||
func TestAccountErrorsAddListClear(t *testing.T) {
|
||||
s := testStore(t)
|
||||
ctx := context.Background()
|
||||
epSrc, _ := s.CreateEndpoint(ctx, Endpoint{RoleLabel: "src", Host: "a", Port: 993, TLSMode: "ssl"})
|
||||
epDst, _ := s.CreateEndpoint(ctx, Endpoint{RoleLabel: "dst", Host: "b", Port: 993, TLSMode: "ssl"})
|
||||
taskID, _ := s.CreateTask(ctx, Task{Name: "t", SrcEndpointID: epSrc, DstEndpointID: epDst})
|
||||
runID, _ := s.CreateRun(ctx, taskID, "manual")
|
||||
accID, _ := s.CreateAccount(ctx, Account{TaskID: taskID, SrcLogin: "u", SrcPassEnc: "x", DstLogin: "u2", DstPassEnc: "y"})
|
||||
|
||||
if err := s.AddAccountError(ctx, accID, runID, "folder", "INBOX", "", "examine failed"); err != nil {
|
||||
t.Fatalf("add folder: %v", err)
|
||||
}
|
||||
if err := s.AddAccountError(ctx, accID, runID, "message", "INBOX", "UID 42: hi", "append rejected"); err != nil {
|
||||
t.Fatalf("add message: %v", err)
|
||||
}
|
||||
|
||||
errs, err := s.ListAccountErrors(ctx, accID)
|
||||
if err != nil {
|
||||
t.Fatalf("list: %v", err)
|
||||
}
|
||||
if len(errs) != 2 {
|
||||
t.Fatalf("len=%d want 2", len(errs))
|
||||
}
|
||||
if errs[0].Kind != "folder" || errs[0].Folder != "INBOX" || errs[0].Error != "examine failed" {
|
||||
t.Fatalf("errs[0]=%+v", errs[0])
|
||||
}
|
||||
if errs[1].Kind != "message" || errs[1].MessageRef != "UID 42: hi" || errs[1].Error != "append rejected" {
|
||||
t.Fatalf("errs[1]=%+v", errs[1])
|
||||
}
|
||||
if errs[0].ID >= errs[1].ID {
|
||||
t.Fatalf("expected insertion order by id: %d then %d", errs[0].ID, errs[1].ID)
|
||||
}
|
||||
|
||||
if err := s.ClearAccountErrors(ctx, accID); err != nil {
|
||||
t.Fatalf("clear: %v", err)
|
||||
}
|
||||
errs, err = s.ListAccountErrors(ctx, accID)
|
||||
if err != nil {
|
||||
t.Fatalf("list after clear: %v", err)
|
||||
}
|
||||
if len(errs) != 0 {
|
||||
t.Fatalf("after clear len=%d want 0", len(errs))
|
||||
}
|
||||
}
|
||||
|
||||
// Deleting an account cascades its errors (ON DELETE CASCADE).
|
||||
func TestAccountErrorsCascadeOnAccountDelete(t *testing.T) {
|
||||
s := testStore(t)
|
||||
ctx := context.Background()
|
||||
epSrc, _ := s.CreateEndpoint(ctx, Endpoint{RoleLabel: "src", Host: "a", Port: 993, TLSMode: "ssl"})
|
||||
epDst, _ := s.CreateEndpoint(ctx, Endpoint{RoleLabel: "dst", Host: "b", Port: 993, TLSMode: "ssl"})
|
||||
taskID, _ := s.CreateTask(ctx, Task{Name: "t", SrcEndpointID: epSrc, DstEndpointID: epDst})
|
||||
accID, _ := s.CreateAccount(ctx, Account{TaskID: taskID, SrcLogin: "u", SrcPassEnc: "x", DstLogin: "u2", DstPassEnc: "y"})
|
||||
_ = s.AddAccountError(ctx, accID, 0, "account", "", "", "login failed")
|
||||
|
||||
if err := s.DeleteAccount(ctx, accID); err != nil {
|
||||
t.Fatalf("delete: %v", err)
|
||||
}
|
||||
var n int
|
||||
if err := s.Pool.QueryRow(ctx, `SELECT count(*) FROM account_errors WHERE account_id=$1`, accID).Scan(&n); err != nil {
|
||||
t.Fatalf("count: %v", err)
|
||||
}
|
||||
if n != 0 {
|
||||
t.Fatalf("account_errors not cascaded: %d rows", n)
|
||||
}
|
||||
}
|
||||
@@ -17,7 +17,7 @@ func testStore(t *testing.T) *Store {
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
s.Pool.Exec(context.Background(),
|
||||
`TRUNCATE endpoints, tasks, accounts, runs, migrated_messages RESTART IDENTITY CASCADE`)
|
||||
`TRUNCATE endpoints, tasks, accounts, runs, migrated_messages, account_errors RESTART IDENTITY CASCADE`)
|
||||
s.Pool.Close()
|
||||
})
|
||||
return s
|
||||
|
||||
Reference in New Issue
Block a user