Unreachable mail servers: send retries in Scheduled, real action retries, account health
Sending: a send the server could not take (unreachable, timeout, 4xx) is no
longer failed at once. It waits in Scheduled ("retrying") and is tried again
after 1, 2, 5, 10, 30 min, then hourly, for 24 hours — on the server, so it
works with the browser closed. A permanent refusal (5xx, wrong password) is
"failed" at once, with the server's reason. GET /outbox lists both;
POST /outbox/{id}/retry (Retry now) and /cancel (Edit, Delete) act on them,
and they are left out of Drafts. Mail delivered but not filed in Sent now
counts as sent, and a conditional claim keeps two jobs from sending one
message twice. The app says when a send failed and when it finally went.
Actions: arq retries a job only on Retry, never on a plain exception, so the
"retry x5, then revert" for flags and moves never happened. Flag, move,
archive, restore, delete and empty-folder jobs now raise Retry while the
server is unreachable (~24 hours of growing gaps), skip a change a later one
replaced, stop at the first connection failure instead of 20 s per message,
and delete/empty keep the rows until the server has the messages gone.
Account health: 3 failed connections in a row mark an account unreachable
(amber in the sidebar and Settings, cleared on the next success); 3 refused
sign-ins stop its syncing as auth_failed until the password is entered
again, so the provider does not lock the account. Sync loops back off 30 s
doubling to 10 min and log one line per expected failure; a refused
connection fails at once with its reason (the connect task is kept), and a
silently dead IDLE connection is noticed within ~11 min instead of ~29.
Also: the empty reading pane is clipped to its width when dragged very
narrow; tests no longer leave live accounts and outbox rows in the database.
Migration 0021: outbox attempts / next_attempt_at / retry_until, accounts
sync_error / sync_error_since.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+106
-50
@@ -1,9 +1,11 @@
|
||||
"""Outbound user actions: flag changes and moves (SPEC §6.3).
|
||||
|
||||
The API writes the change to the DB first, then enqueues a job here to push it to
|
||||
the mail server. On provider failure the job retries (arq, ×5 exponential) and, on
|
||||
final failure, reverts the DB change and emits ``flags.changed`` with
|
||||
``reverted=true`` so the UI can roll back.
|
||||
the mail server. While the server cannot be reached the job is retried (arq
|
||||
``Retry``) with growing gaps for about 24 hours (``ACTION_MAX_TRIES``); a flag
|
||||
change that still cannot be pushed is then reverted in the DB and announced with
|
||||
``flags.changed`` / ``reverted=true`` so the UI rolls back. A plain exception does
|
||||
NOT make arq retry — only ``Retry`` does.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -15,6 +17,7 @@ from collections.abc import Iterable
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
|
||||
from arq import Retry
|
||||
from sqlalchemy import delete, func, select, update
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
@@ -22,6 +25,7 @@ from app.config import get_settings
|
||||
from app.db import session_scope
|
||||
from app.events import get_event_bus
|
||||
from app.models import Account, Draft, Mailbox, Message, Thread
|
||||
from app.sync.health import is_temporary
|
||||
from app.sync.provider_factory import build_provider
|
||||
from app.threading_.threader import denormalize_thread
|
||||
|
||||
@@ -30,7 +34,26 @@ logger = logging.getLogger(__name__)
|
||||
# User-settable IMAP flags (SPEC §8.2 is:read/starred/answered). \Deleted/\Draft
|
||||
# are managed by move/expunge and drafts, not this surface.
|
||||
ALLOWED_FLAGS = frozenset({"\\Seen", "\\Flagged", "\\Answered"})
|
||||
_MAX_TRIES = 5
|
||||
# Gaps between tries while the server is unreachable: ~24 hours in all.
|
||||
_RETRY_DEFERS_S = (30, 60, 120, 300, 600, 1800, 3600, 7200, 14400, 28800, 28800)
|
||||
ACTION_MAX_TRIES = len(_RETRY_DEFERS_S) + 1
|
||||
|
||||
|
||||
def _retry_later(ctx: dict[str, Any], exc: BaseException) -> Retry | None:
|
||||
"""A ``Retry`` for a temporary failure while tries remain, else None (give up)."""
|
||||
if not is_temporary(exc):
|
||||
return None
|
||||
attempt = int(ctx.get("job_try", 1))
|
||||
if attempt >= ACTION_MAX_TRIES:
|
||||
return None
|
||||
logger.info(
|
||||
"server unreachable (try %d), retrying in %ds: %s",
|
||||
attempt,
|
||||
_RETRY_DEFERS_S[attempt - 1],
|
||||
exc,
|
||||
)
|
||||
return Retry(defer=_RETRY_DEFERS_S[attempt - 1])
|
||||
|
||||
|
||||
# Folder names to create when a target special-use folder is missing on the server.
|
||||
_DEFAULT_FOLDER_NAME = {"archive": "Archive", "trash": "Trash", "junk": "Spam"}
|
||||
@@ -71,6 +94,8 @@ async def push_flags_job(
|
||||
message = await session.get(Message, mid)
|
||||
if message is None:
|
||||
return
|
||||
if _superseded(message.flags, add, remove):
|
||||
return # changed again since: that later change's own job pushes it
|
||||
account = await session.get(Account, message.account_id)
|
||||
mailbox = await session.get(Mailbox, message.mailbox_id)
|
||||
if account is None or mailbox is None:
|
||||
@@ -80,21 +105,29 @@ async def push_flags_job(
|
||||
provider = build_provider(account)
|
||||
try:
|
||||
await provider.push_flags(path, uid, frozenset(add), frozenset(remove))
|
||||
except Exception:
|
||||
if ctx.get("job_try", 1) >= _MAX_TRIES:
|
||||
await _revert_flags(mid, user_id, add, remove)
|
||||
return
|
||||
raise # arq retries with backoff
|
||||
except Exception as exc:
|
||||
retry = _retry_later(ctx, exc)
|
||||
if retry is not None:
|
||||
raise retry from exc
|
||||
logger.warning("flag change for message %s could not be pushed: %s", mid, exc)
|
||||
await _revert_flags(mid, user_id, add, remove)
|
||||
finally:
|
||||
await provider.close()
|
||||
|
||||
|
||||
def _superseded(flags: list[str], add: list[str], remove: list[str]) -> bool:
|
||||
"""The message's flags no longer show this change: a later one replaced it
|
||||
(e.g. read, then unread again while the server was unreachable)."""
|
||||
now = set(flags)
|
||||
return not set(add) <= now or bool(set(remove) & now)
|
||||
|
||||
|
||||
async def _revert_flags(
|
||||
message_id: uuid.UUID, user_id: uuid.UUID, add: list[str], remove: list[str]
|
||||
) -> None:
|
||||
async with session_scope() as session:
|
||||
message = await session.get(Message, message_id)
|
||||
if message is None:
|
||||
if message is None or _superseded(message.flags, add, remove):
|
||||
return
|
||||
# Undo the change: remove what we added, add back what we removed.
|
||||
message.flags = sorted((set(message.flags) - set(add)) | set(remove))
|
||||
@@ -138,11 +171,12 @@ async def move_job(
|
||||
provider = build_provider(account)
|
||||
try:
|
||||
new_uid = await provider.move(uid, src_path, dst_path)
|
||||
except Exception:
|
||||
if ctx.get("job_try", 1) >= _MAX_TRIES:
|
||||
logger.warning("move failed for message %s; leaving optimistic state", mid)
|
||||
return
|
||||
raise
|
||||
except Exception as exc:
|
||||
retry = _retry_later(ctx, exc)
|
||||
if retry is not None:
|
||||
raise retry from exc
|
||||
logger.warning("move failed for message %s; leaving optimistic state: %s", mid, exc)
|
||||
return
|
||||
finally:
|
||||
await provider.close()
|
||||
|
||||
@@ -193,28 +227,39 @@ async def move_thread_to_folder_job(ctx: dict[str, Any], thread_id: str, special
|
||||
# (message_id, account_id, target_path, new_uid) applied after the network moves.
|
||||
moves: list[tuple[uuid.UUID, uuid.UUID, str, int]] = []
|
||||
to_create: list[tuple[uuid.UUID, str, int]] = [] # account_id, path, uidvalidity
|
||||
unreachable: BaseException | None = None # set: retry the moves that did not happen
|
||||
|
||||
for account_id in account_ids:
|
||||
provider = build_provider(accounts[account_id])
|
||||
try:
|
||||
if account_id in target_id_path:
|
||||
target_id, target_path = target_id_path[account_id]
|
||||
else:
|
||||
target_id, target_path = None, _DEFAULT_FOLDER_NAME.get(special_use, special_use)
|
||||
await provider.create_mailbox(target_path)
|
||||
remote = await provider.list_mailboxes()
|
||||
uidv = next((r.uidvalidity for r in remote if r.path == target_path), 0)
|
||||
to_create.append((account_id, target_path, uidv))
|
||||
for mid, m_account, m_mailbox, m_uid in snapshot:
|
||||
if m_account != account_id or m_mailbox == target_id:
|
||||
continue
|
||||
src_path = path_by_id.get(m_mailbox, "INBOX")
|
||||
try:
|
||||
new_uid = await provider.move(m_uid, src_path, target_path)
|
||||
except Exception:
|
||||
logger.exception("move failed for message %s -> %s", mid, target_path)
|
||||
continue
|
||||
moves.append((mid, account_id, target_path, new_uid))
|
||||
try:
|
||||
if account_id in target_id_path:
|
||||
target_id, target_path = target_id_path[account_id]
|
||||
else:
|
||||
target_id, target_path = (
|
||||
None,
|
||||
_DEFAULT_FOLDER_NAME.get(special_use, special_use),
|
||||
)
|
||||
await provider.create_mailbox(target_path)
|
||||
remote = await provider.list_mailboxes()
|
||||
uidv = next((r.uidvalidity for r in remote if r.path == target_path), 0)
|
||||
to_create.append((account_id, target_path, uidv))
|
||||
for mid, m_account, m_mailbox, m_uid in snapshot:
|
||||
if m_account != account_id or m_mailbox == target_id:
|
||||
continue
|
||||
src_path = path_by_id.get(m_mailbox, "INBOX")
|
||||
try:
|
||||
new_uid = await provider.move(m_uid, src_path, target_path)
|
||||
except Exception as exc:
|
||||
if is_temporary(exc):
|
||||
raise # the server is gone: stop this account, retry it later
|
||||
logger.exception("move failed for message %s -> %s", mid, target_path)
|
||||
continue
|
||||
moves.append((mid, account_id, target_path, new_uid))
|
||||
except Exception as exc:
|
||||
if not is_temporary(exc):
|
||||
raise
|
||||
unreachable = exc
|
||||
finally:
|
||||
await provider.close()
|
||||
|
||||
@@ -246,6 +291,10 @@ async def move_thread_to_folder_job(ctx: dict[str, Any], thread_id: str, special
|
||||
await get_event_bus().publish(
|
||||
"thread.updated", {"user_id": str(user_id), "thread_id": str(tid)}
|
||||
)
|
||||
# What moved is recorded; the rest goes again once the server is back (the
|
||||
# re-run skips messages already in the target folder).
|
||||
if unreachable is not None and (retry := _retry_later(ctx, unreachable)) is not None:
|
||||
raise retry from unreachable
|
||||
|
||||
|
||||
async def restore_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
@@ -270,9 +319,7 @@ async def restore_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
await session.scalars(select(Mailbox).where(Mailbox.account_id.in_(account_ids)))
|
||||
)
|
||||
path_by_id = {mb.id: mb.path for mb in mailboxes}
|
||||
inbox_by_account = {
|
||||
mb.account_id: mb.id for mb in mailboxes if mb.special_use == "inbox"
|
||||
}
|
||||
inbox_by_account = {mb.account_id: mb.id for mb in mailboxes if mb.special_use == "inbox"}
|
||||
# (message_id, account_id, source_path, target_id) — resolve the target now.
|
||||
plan: list[tuple[uuid.UUID, uuid.UUID, str, uuid.UUID, int]] = []
|
||||
for m in messages:
|
||||
@@ -287,6 +334,7 @@ async def restore_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
|
||||
# (message_id, target_id, new_uid) applied after the network moves.
|
||||
moves: list[tuple[uuid.UUID, uuid.UUID, int]] = []
|
||||
unreachable: BaseException | None = None
|
||||
for account_id in account_ids:
|
||||
provider = build_provider(accounts[account_id])
|
||||
try:
|
||||
@@ -296,7 +344,10 @@ async def restore_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
target_path = path_by_id[target_id]
|
||||
try:
|
||||
new_uid = await provider.move(uid, src_path, target_path)
|
||||
except Exception:
|
||||
except Exception as exc:
|
||||
if is_temporary(exc):
|
||||
unreachable = exc # the server is gone: stop this account
|
||||
break
|
||||
logger.exception("restore move failed for message %s -> %s", mid, target_path)
|
||||
continue
|
||||
moves.append((mid, target_id, new_uid))
|
||||
@@ -314,6 +365,8 @@ async def restore_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
await get_event_bus().publish(
|
||||
"thread.updated", {"user_id": str(user_id), "thread_id": str(tid)}
|
||||
)
|
||||
if unreachable is not None and (retry := _retry_later(ctx, unreachable)) is not None:
|
||||
raise retry from unreachable
|
||||
|
||||
|
||||
async def delete_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
@@ -352,8 +405,12 @@ async def delete_thread_job(ctx: dict[str, Any], thread_id: str) -> None:
|
||||
continue
|
||||
try:
|
||||
await provider.delete_messages(path, uids)
|
||||
except Exception:
|
||||
logger.exception("expunge failed for %s in %s", uids, path)
|
||||
except Exception as exc:
|
||||
# Keep the rows while the server is away: deleted here but
|
||||
# not there, the next sync would bring the messages back.
|
||||
if (retry := _retry_later(ctx, exc)) is not None:
|
||||
raise retry from exc
|
||||
logger.warning("expunge failed for %s in %s: %s", uids, path, exc)
|
||||
finally:
|
||||
await provider.close()
|
||||
|
||||
@@ -389,9 +446,7 @@ async def empty_folder_job(ctx: dict[str, Any], user_id: str, special_use: str)
|
||||
async with session_scope() as session:
|
||||
mailboxes = list(
|
||||
await session.scalars(
|
||||
select(Mailbox).where(
|
||||
Mailbox.user_id == uid, Mailbox.special_use == special_use
|
||||
)
|
||||
select(Mailbox).where(Mailbox.user_id == uid, Mailbox.special_use == special_use)
|
||||
)
|
||||
)
|
||||
if not mailboxes:
|
||||
@@ -423,8 +478,11 @@ async def empty_folder_job(ctx: dict[str, Any], user_id: str, special_use: str)
|
||||
continue
|
||||
try:
|
||||
await provider.delete_messages(path, uids)
|
||||
except Exception:
|
||||
logger.exception("empty folder: expunge failed for %s in %s", uids, path)
|
||||
except Exception as exc:
|
||||
# As for a thread: keep the rows until the server has them gone.
|
||||
if (retry := _retry_later(ctx, exc)) is not None:
|
||||
raise retry from exc
|
||||
logger.warning("empty folder: expunge failed for %s in %s: %s", uids, path, exc)
|
||||
finally:
|
||||
await provider.close()
|
||||
|
||||
@@ -520,9 +578,9 @@ async def autopurge_trash_spam(ctx: dict[str, Any]) -> None:
|
||||
user_ids = {r.user_id for r in stale}
|
||||
async with session_scope() as session:
|
||||
await session.execute(
|
||||
update(Draft).where(Draft.reply_to_message_id.in_(msg_ids)).values(
|
||||
reply_to_message_id=None
|
||||
)
|
||||
update(Draft)
|
||||
.where(Draft.reply_to_message_id.in_(msg_ids))
|
||||
.values(reply_to_message_id=None)
|
||||
)
|
||||
await session.execute(delete(Message).where(Message.id.in_(msg_ids)))
|
||||
await session.flush()
|
||||
@@ -539,6 +597,4 @@ async def autopurge_trash_spam(ctx: dict[str, Any]) -> None:
|
||||
bus = get_event_bus()
|
||||
for user_id in user_ids:
|
||||
await bus.publish("message.deleted", {"user_id": str(user_id)})
|
||||
logger.info(
|
||||
"autopurge: removed %d Trash/Spam message(s) older than %dd", len(msg_ids), days
|
||||
)
|
||||
logger.info("autopurge: removed %d Trash/Spam message(s) older than %dd", len(msg_ids), days)
|
||||
|
||||
Reference in New Issue
Block a user