diff --git a/backend/app/outbox/jobs.py b/backend/app/outbox/jobs.py index 407e904..ee0995d 100644 --- a/backend/app/outbox/jobs.py +++ b/backend/app/outbox/jobs.py @@ -20,6 +20,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.db import session_scope from app.events import get_event_bus +from app.indexer import blobstore from app.indexer.indexer import persist_message from app.models import Account, Draft, KnownRecipient, Mailbox, Message, Outbox from app.outbox.attachments import load_attachments @@ -28,6 +29,7 @@ from app.outbox.service import delete_mirrored_draft_row from app.providers.base import RemoteMessage from app.queue import enqueue_job from app.search.factory import get_search_backend +from app.sync.actions import _retry_later from app.sync.health import is_temporary from app.sync.provider_factory import build_provider @@ -167,6 +169,37 @@ async def _send_failed( ) +async def file_sent_copy_job(ctx: dict[str, Any], account_id: str, user_id: str, sha: str) -> None: + """File a sent message's copy in Sent, after the send itself succeeded but + the IMAP server could not be reached. Retried (arq Retry) for ~24 hours.""" + uid = uuid.UUID(user_id) + async with session_scope() as session: + account = await session.get(Account, uuid.UUID(account_id)) + if account is None: + return + sent_path = await session.scalar( + select(Mailbox.path).where( + Mailbox.account_id == account.id, Mailbox.special_use == "sent" + ) + ) + if not sent_path: + return + raw = blobstore.load(uid, sha) + provider = build_provider(account) + try: + appended_uid = await provider.append(sent_path, raw, frozenset({"\\Seen"})) + except Exception as exc: + retry = _retry_later(ctx, exc) + if retry is not None: + raise retry from exc + logger.error("could not file a sent message in Sent (blob %s): %s", sha, exc) + return + finally: + await provider.close() + await _persist_sent_copy(uid, account, raw, appended_uid) + logger.info("filed a sent message in Sent for account %s", account_id) + + async def send_job(ctx: dict[str, Any], outbox_id: str) -> None: """Send a queued message unless it was cancelled during the undo window. @@ -229,16 +262,22 @@ async def send_job(ctx: dict[str, Any], outbox_id: str) -> None: except Exception as exc: # noqa: BLE001 - retry or record; the draft is kept await _send_failed(oid, user_id, subject, exc) return - # Delivered. Filing it is housekeeping: if the IMAP server is not there - # right now, the send still succeeded (the Sent poll files it later). - try: - if sent_path and not gmail: + # Delivered. If filing the copy in Sent fails (the IMAP server is not + # there right now), the send still succeeded — and the copy must not be + # lost: it is kept and filed later by file_sent_copy_job. + if sent_path and not gmail: + try: appended_uid = await provider.append(sent_path, sent_raw, frozenset({"\\Seen"})) - # Remove the mirrored Drafts-folder copy now that it has been sent. - if drafts_path and draft_imap_uid: + except Exception as exc: # noqa: BLE001 + logger.warning("outbox %s sent; filing it in Sent failed, will retry: %s", oid, exc) + sha = blobstore.store(user_id, sent_raw) + await enqueue_job("file_sent_copy_job", str(account_obj.id), str(user_id), sha) + # Remove the mirrored Drafts-folder copy now that it has been sent. + if drafts_path and draft_imap_uid: + try: await provider.delete_messages(drafts_path, [draft_imap_uid]) - except Exception as exc: # noqa: BLE001 - logger.warning("outbox %s sent, but filing it on the server failed: %s", oid, exc) + except Exception as exc: # noqa: BLE001 + logger.warning("outbox %s sent; removing its Drafts copy failed: %s", oid, exc) finally: await provider.close() diff --git a/backend/app/providers/imap_smtp/provider.py b/backend/app/providers/imap_smtp/provider.py index be37bd8..525e95f 100644 --- a/backend/app/providers/imap_smtp/provider.py +++ b/backend/app/providers/imap_smtp/provider.py @@ -11,6 +11,7 @@ from __future__ import annotations import asyncio import contextlib import re +import ssl from collections.abc import AsyncIterator from dataclasses import dataclass from datetime import date @@ -67,6 +68,11 @@ class _KeepConnectTask: def create_client( # type: ignore[no-untyped-def] self, host, port, loop, conn_lost_cb=None, ssl_context=None ) -> None: + # This replaces IMAP4_SSL.create_client too (it comes first in the MRO), + # so do its one job here: without a context the connection is plain + # TCP to port 993, and the server just waits for a TLS handshake. + if ssl_context is None and isinstance(self, aioimaplib.IMAP4_SSL): + ssl_context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH) local_loop = loop if loop is not None else asyncio.get_running_loop() self.protocol = aioimaplib.IMAP4ClientProtocol(local_loop, conn_lost_cb) self.connect_task = local_loop.create_task( diff --git a/backend/tests/outbox/test_outbox.py b/backend/tests/outbox/test_outbox.py index 6831a1d..5d9b6c5 100644 --- a/backend/tests/outbox/test_outbox.py +++ b/backend/tests/outbox/test_outbox.py @@ -11,7 +11,9 @@ from datetime import UTC, datetime, timedelta from email.policy import default as default_policy import httpx +import pytest import pytest_asyncio +from arq import Retry from httpx import ASGITransport from sqlalchemy import delete, select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine @@ -615,6 +617,24 @@ async def test_delivered_but_not_filed_counts_as_sent(ctx: Ctx, monkeypatch) -> await outbox_jobs.send_job({"job_try": 1}, str(oid)) assert (await _outbox(ctx, oid)).state == "sent" and len(fake.sent) == 1 + # The copy is not lost: kept, and a job files it once IMAP answers again. + args, _ = ctx.enqueued[-1] + assert args[0] == "file_sent_copy_job" + _, account_id, user_id, sha = args + with pytest.raises(Retry): # still unreachable: tried again later + await outbox_jobs.file_sent_copy_job({"job_try": 1}, account_id, user_id, sha) + fake.append_error = None + await outbox_jobs.file_sent_copy_job({"job_try": 2}, account_id, user_id, sha) + assert fake.appended == ["Sent"] + ctx.session.expunge_all() + async with session_scope() as verify: + sent_msg = await verify.scalar( + select(Message) + .join(Mailbox, Mailbox.id == Message.mailbox_id) + .where(Mailbox.account_id == ctx.account_id, Mailbox.special_use == "sent") + ) + assert sent_msg is not None and sent_msg.subject == "Re: Hi" + async def test_two_jobs_for_one_send_send_once(ctx: Ctx, monkeypatch) -> None: # type: ignore[no-untyped-def] oid = await _make_outbox(ctx, state="retrying") diff --git a/backend/tests/sync/test_account_health.py b/backend/tests/sync/test_account_health.py index 88150ea..bc47836 100644 --- a/backend/tests/sync/test_account_health.py +++ b/backend/tests/sync/test_account_health.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import uuid from collections.abc import AsyncIterator @@ -125,3 +126,27 @@ async def test_refused_sign_in_stops_syncing(account_id: uuid.UUID) -> None: await health.failed(ProviderAuthError("AUTHENTICATIONFAILED")) account = await _account(account_id) assert account.state == "auth_failed" and "AUTHENTICATIONFAILED" in account.state_detail + + +async def test_ssl_connections_really_use_tls(monkeypatch) -> None: # type: ignore[no-untyped-def] + """Regression (v1.3.26): our IMAP4_SSL subclass skipped aioimaplib's TLS + setup, so port 993 got plain TCP and every account timed out.""" + import asyncio + import ssl + + from app.providers.imap_smtp.provider import _IMAP4, _IMAP4_SSL + + seen: list[object] = [] + loop = asyncio.get_running_loop() + + async def fake_create_connection(factory, host, port, ssl=None): # type: ignore[no-untyped-def] + seen.append(ssl) + raise ConnectionRefusedError + + monkeypatch.setattr(loop, "create_connection", fake_create_connection) + for cls in (_IMAP4_SSL, _IMAP4): + client = cls(host="imap.example.com", port=993, timeout=1) + with contextlib.suppress(ConnectionRefusedError): + await client.connect_task + assert isinstance(seen[0], ssl.SSLContext) # SSL: a TLS context + assert seen[1] is None # plain / STARTTLS: none up front diff --git a/backend/workers/worker.py b/backend/workers/worker.py index ea55937..b59564e 100644 --- a/backend/workers/worker.py +++ b/backend/workers/worker.py @@ -15,7 +15,7 @@ from typing import Any from arq import func from app.outbox.draft_sync import push_draft_to_imap_job, remove_imap_draft_job -from app.outbox.jobs import send_job +from app.outbox.jobs import file_sent_copy_job, send_job from app.sync.actions import ( ACTION_MAX_TRIES, delete_thread_job, @@ -48,6 +48,7 @@ _RETRYING_ACTIONS: list[Any] = [ restore_thread_job, delete_thread_job, empty_folder_job, + file_sent_copy_job, ] INTERACTIVE_FUNCTIONS: list[Any] = [