fix: drop empty emails, clear stale errors on success, harden IMAP extraction

Agent-Logs-Url: https://github.com/christianlouis/InboxConverge/sessions/d50e7f06-4c6f-4caa-b5dd-329068e8096c

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>

CHANGELOG.md

@@ -27,6 +27,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   sequence numbers, followed by a lightweight `FETCH (UID)` to resolve them
   to stable UIDs; all subsequent operations (`FETCH`, `STORE`) continue to
   use the UID form.
+- **IMAP: filter whitespace-only RFC822 responses** — the FETCH extraction
+  loop now checks `email_data.strip()` so that trivially-empty byte payloads
+  (e.g. `\r\n`) returned by servers like T-Online are rejected at the
+  extraction layer rather than being forwarded as empty emails.
+- **Tasks: drop completely empty emails with a warning** — after parsing
+  each fetched email, the processing loop now checks whether the message has
+  no subject, no From header, and no body.  Such emails are silently dropped
+  (logged as `WARNING`, written to the processing log, UID recorded as seen
+  so the message is not retried).  This handles cases where T-Online or
+  other servers emit genuinely empty RFC822 responses that may indicate a
+  server-side bug.
+- **Status page: clear stale IMAP errors after a successful run** — on a
+  successful processing run (`emails_failed == 0`), the account's
+  `last_error_message` and `last_error_at` fields are now cleared.
+  Previously a transient IMAP error would remain visible on the dashboard
+  even after subsequent pulls completed without problems.
 - **Dashboard: fix UTC hour-shift on "Last check" timestamps** — the dashboard
   page was using `new Date(iso)` which treats timezone-naive ISO strings from
   the backend as local time, shifting relative labels (e.g. "1h ago" instead

backend/app/services/mail_processor.py

@@ -396,14 +396,17 @@ class MailProcessor:
                         email_data = line
                         break
 
-                    if email_data:
+                    if email_data and email_data.strip():
                         emails.append(email_data)
                         new_uids.append(uid_str)
                         fetched_uids.append(uid)
                     else:
                         logger.warning(
                             f"No email data extracted for UID {uid_str} on "
-                            f"account {self.account.id}"
+                            f"account {self.account.id} "
+                            f"(raw bytes: {len(email_data) if email_data else 0}); "
+                            "this may indicate a server-side error producing empty "
+                            "IMAP RFC822 responses"
                         )
 
                 except Exception as e:

backend/app/workers/tasks.py

@@ -204,6 +204,7 @@ async def process_mail_account(account_id: int):
                     return
 
             successfully_forwarded_uids: list[str] = []
+            skipped_empty_uids: list[str] = []
 
             if len(emails) != len(new_uids):
                 logger.error(
@@ -238,6 +239,55 @@ async def process_mail_account(account_id: int):
                 forwarded_ok = False
                 error_msg: str | None = None
 
+                # ── Detect completely empty emails ───────────────────────────
+                # T-Online (and potentially other servers) may return empty
+                # RFC822 responses.  An email with no subject, no sender and no
+                # body provides no value and should not be forwarded.  We log a
+                # warning (it may indicate a server-side bug) and skip the
+                # message while still recording its UID so it is not retried.
+                if not email_subject and not email_from:
+                    body_has_content = False
+                    try:
+                        if msg.is_multipart():  # type: ignore[possibly-undefined]
+                            for part in msg.walk():
+                                payload = part.get_payload(decode=True)
+                                if isinstance(payload, bytes) and payload.strip():
+                                    body_has_content = True
+                                    break
+                        else:
+                            payload = msg.get_payload(decode=True)  # type: ignore[possibly-undefined]
+                            body_has_content = isinstance(payload, bytes) and bool(
+                                payload.strip()
+                            )
+                    except Exception:
+                        pass
+
+                    if not body_has_content:
+                        logger.warning(
+                            "Dropping completely empty email (uid=%s, account=%s, "
+                            "size=%d bytes) — no subject, sender, or body; "
+                            "this may indicate a server-side error.",
+                            uid,
+                            account.id,
+                            email_size_bytes,
+                        )
+                        skipped_empty_uids.append(uid)
+                        db.add(
+                            ProcessingLog(
+                                user_id=account.user_id,
+                                mail_account_id=account.id,
+                                processing_run_id=run.id,
+                                level="WARNING",
+                                message="Dropped empty email (no subject, sender, or body)",
+                                email_subject=None,
+                                email_from=None,
+                                email_size_bytes=email_size_bytes,
+                                success=False,
+                                error_details={"reason": "empty_email"},
+                            )
+                        )
+                        continue
+
                 try:
                     if use_gmail_api and gmail_service and gmail_cred:
                         # Inject via Gmail API (preferred)
@@ -331,6 +381,17 @@ async def process_mail_account(account_id: int):
                         )
                     )
 
+            # Also persist UIDs of dropped empty emails so they are not
+            # re-fetched and re-evaluated on the next run.
+            for uid in skipped_empty_uids:
+                if uid not in already_seen_uids:
+                    db.add(
+                        DownloadedMessageId(
+                            mail_account_id=account.id,
+                            message_uid=uid,
+                        )
+                    )
+
             # If Gmail API was used, persist any refreshed access token back to
             # the DB so the next run doesn't need an extra token-refresh call.
             if use_gmail_api and gmail_service and gmail_cred:
@@ -361,6 +422,8 @@ async def process_mail_account(account_id: int):
             if emails_failed == 0:
                 account.last_successful_check_at = datetime.now(timezone.utc)  # type: ignore[assignment]
                 account.status = AccountStatus.ACTIVE  # type: ignore[assignment]
+                account.last_error_message = None  # type: ignore[assignment]
+                account.last_error_at = None  # type: ignore[assignment]
             else:
                 account.status = AccountStatus.ERROR  # type: ignore[assignment]
                 account.last_error_at = datetime.now(timezone.utc)  # type: ignore[assignment]

backend/tests/unit/test_empty_email_handling.py

@@ -0,0 +1,225 @@
+"""
+Unit tests for empty-email detection in the task processing pipeline.
+
+Covers two layers:
+  1. tasks._empty_email_check_logic  – the inline body-inspection that
+     decides whether a parsed email should be dropped.
+  2. The "clear last_error_message on success" contract – verifying that a
+     successful run nulls out any previously stored error so the status page
+     no longer shows stale IMAP errors.
+"""
+
+import email as email_lib
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _parse(raw: bytes):
+    """Parse raw bytes into an email.Message object (same call used in tasks.py)."""
+    return email_lib.message_from_bytes(raw)
+
+
+def _body_has_content(msg) -> bool:
+    """
+    Mirror of the body-detection logic in tasks.py's email processing loop.
+
+    Returns True if the message has at least one non-empty / non-whitespace
+    text or binary payload.
+    """
+    if msg.is_multipart():
+        for part in msg.walk():
+            payload = part.get_payload(decode=True)
+            if isinstance(payload, bytes) and payload.strip():
+                return True
+        return False
+    else:
+        payload = msg.get_payload(decode=True)
+        return isinstance(payload, bytes) and bool(payload.strip())
+
+
+def _is_empty_email(raw: bytes) -> bool:
+    """
+    Replicates the complete empty-email gate from tasks.py:
+      - no subject AND no from AND no body  →  True (should be dropped)
+    """
+    msg = _parse(raw)
+    email_subject = (msg.get("Subject", "") or "").strip() or None
+    email_from = (msg.get("From", "") or "").strip() or None
+
+    if email_subject or email_from:
+        return False  # Has at least a header → not empty
+
+    return not _body_has_content(msg)
+
+
+# ---------------------------------------------------------------------------
+# Body-detection tests
+# ---------------------------------------------------------------------------
+
+
+class TestBodyHasContent:
+    def test_plain_text_body(self):
+        raw = b"From: a@b.com\r\nSubject: Hi\r\n\r\nHello world"
+        msg = _parse(raw)
+        assert _body_has_content(msg) is True
+
+    def test_empty_body(self):
+        raw = b"From: a@b.com\r\nSubject: Hi\r\n\r\n"
+        msg = _parse(raw)
+        assert _body_has_content(msg) is False
+
+    def test_crlf_only_body(self):
+        raw = b"From: a@b.com\r\nSubject: Hi\r\n\r\n\r\n"
+        msg = _parse(raw)
+        assert _body_has_content(msg) is False
+
+    def test_whitespace_only_body(self):
+        raw = b"From: a@b.com\r\nSubject: Hi\r\n\r\n   \t  "
+        msg = _parse(raw)
+        assert _body_has_content(msg) is False
+
+    def test_multipart_with_content(self):
+        raw = (
+            b"MIME-Version: 1.0\r\n"
+            b"Content-Type: multipart/mixed; boundary=X\r\n\r\n"
+            b"--X\r\n"
+            b"Content-Type: text/plain\r\n\r\n"
+            b"Hello from multipart\r\n"
+            b"--X--\r\n"
+        )
+        msg = _parse(raw)
+        assert _body_has_content(msg) is True
+
+    def test_multipart_all_empty_parts(self):
+        raw = (
+            b"MIME-Version: 1.0\r\n"
+            b"Content-Type: multipart/mixed; boundary=X\r\n\r\n"
+            b"--X\r\n"
+            b"Content-Type: text/plain\r\n\r\n"
+            b"\r\n"
+            b"--X--\r\n"
+        )
+        msg = _parse(raw)
+        assert _body_has_content(msg) is False
+
+
+# ---------------------------------------------------------------------------
+# Complete empty-email gate tests
+# ---------------------------------------------------------------------------
+
+
+class TestIsEmptyEmail:
+    def test_completely_empty_raw_bytes(self):
+        assert _is_empty_email(b"") is True
+
+    def test_only_headers_no_body(self):
+        raw = b"\r\n"
+        assert _is_empty_email(raw) is True
+
+    def test_no_subject_no_from_no_body(self):
+        raw = b"Date: Mon, 1 Jan 2024 00:00:00 +0000\r\n\r\n"
+        assert _is_empty_email(raw) is True
+
+    def test_has_subject_no_from_no_body(self):
+        """Subject alone is enough to keep the email."""
+        raw = b"Subject: Alert\r\n\r\n"
+        assert _is_empty_email(raw) is False
+
+    def test_has_from_no_subject_no_body(self):
+        """From alone is enough to keep the email."""
+        raw = b"From: sender@example.com\r\n\r\n"
+        assert _is_empty_email(raw) is False
+
+    def test_no_headers_but_body_has_content(self):
+        """Body content alone is enough to keep the email."""
+        raw = b"\r\nThis is the body."
+        assert _is_empty_email(raw) is False
+
+    def test_normal_email_is_not_empty(self):
+        raw = (
+            b"From: sender@example.com\r\n"
+            b"Subject: Hello\r\n\r\n"
+            b"Some body text.\r\n"
+        )
+        assert _is_empty_email(raw) is False
+
+    def test_crlf_only_is_empty(self):
+        assert _is_empty_email(b"\r\n\r\n") is True
+
+
+# ---------------------------------------------------------------------------
+# Status-clearing contract
+# ---------------------------------------------------------------------------
+
+
+class TestLastErrorMessageClearOnSuccess:
+    """
+    The `last_error_message` field must be cleared when a run completes with
+    zero forwarding failures, so the status page does not show stale errors.
+
+    This test exercises the same branch logic used in tasks.py without
+    needing a full Celery / DB setup.
+    """
+
+    def _simulate_account_status_update(
+        self, emails_failed: int, current_error_message: str | None
+    ) -> dict:
+        """
+        Simulate the account-status block from tasks.py:
+
+            if emails_failed == 0:
+                account.status = ACTIVE
+                account.last_error_message = None
+                account.last_error_at = None
+            else:
+                account.status = ERROR
+                account.last_error_at = <now>
+                account.last_error_message = f"{emails_failed} emails failed to forward"
+
+        Returns a dict with the resulting field values.
+        """
+        from app.models.database_models import AccountStatus
+
+        state = {
+            "last_error_message": current_error_message,
+            "last_error_at": "2024-01-01",
+        }
+
+        if emails_failed == 0:
+            state["status"] = AccountStatus.ACTIVE
+            state["last_error_message"] = None
+            state["last_error_at"] = None
+        else:
+            state["status"] = AccountStatus.ERROR
+            state["last_error_message"] = f"{emails_failed} emails failed to forward"
+            state["last_error_at"] = "2024-01-02"
+
+        return state
+
+    def test_error_cleared_on_zero_failures(self):
+        """A previously stored error is wiped when all emails forward OK."""
+        result = self._simulate_account_status_update(
+            emails_failed=0,
+            current_error_message="IMAP fetch error: UID only possible with …",
+        )
+        assert result["last_error_message"] is None
+        assert result["last_error_at"] is None
+
+    def test_error_set_when_failures_exist(self):
+        """When emails fail, the error message is updated, not cleared."""
+        result = self._simulate_account_status_update(
+            emails_failed=3,
+            current_error_message=None,
+        )
+        assert result["last_error_message"] == "3 emails failed to forward"
+        assert result["last_error_at"] is not None
+
+    def test_no_error_remains_none_on_success(self):
+        """A clean account stays clean after a successful run."""
+        result = self._simulate_account_status_update(
+            emails_failed=0,
+            current_error_message=None,
+        )
+        assert result["last_error_message"] is None

backend/tests/unit/test_mail_processor_imap.py

@@ -422,3 +422,60 @@ class TestFetchImapEmailsUidCommands:
         ) as mock_cls:
             await processor._fetch_imap_emails(10, set())
             mock_cls.assert_called_once()
+
+    async def test_whitespace_only_rfc822_body_is_skipped(self, processor, mock_imap):
+        """A UID FETCH that returns only whitespace bytes must not be added to results.
+
+        T-Online (and potentially other servers) can return RFC822 responses
+        whose body is just CR LF or other whitespace.  The extraction loop
+        must treat these as absent email data, not as a valid message.
+        """
+        # Simulate a server that returns b"\r\n" instead of real email bytes
+        whitespace_response = _make_imap_response(
+            result="OK",
+            lines=[b"1 (UID 99 RFC822 {2}", b"\r\n", b")"],
+        )
+
+        mock_imap.search.return_value = _make_imap_response(result="OK", lines=[b"99"])
+        mock_imap.fetch.return_value = _make_uid_list_response([("99", "99")])
+
+        mock_imap.uid = AsyncMock(
+            side_effect=lambda cmd, *args: (
+                whitespace_response if cmd == "fetch" else _make_imap_response()
+            )
+        )
+
+        with patch(
+            "app.services.mail_processor.aioimaplib.IMAP4_SSL",
+            return_value=mock_imap,
+        ):
+            emails, new_uids = await processor._fetch_imap_emails(10, set())
+
+        # Whitespace-only response must not produce a message
+        assert emails == []
+        assert new_uids == []
+
+    async def test_empty_bytes_rfc822_body_is_skipped(self, processor, mock_imap):
+        """A UID FETCH that returns b'' as the body must not be added to results."""
+        empty_response = _make_imap_response(
+            result="OK",
+            lines=[b"1 (UID 77 RFC822 {0}", b"", b")"],
+        )
+
+        mock_imap.search.return_value = _make_imap_response(result="OK", lines=[b"77"])
+        mock_imap.fetch.return_value = _make_uid_list_response([("77", "77")])
+
+        mock_imap.uid = AsyncMock(
+            side_effect=lambda cmd, *args: (
+                empty_response if cmd == "fetch" else _make_imap_response()
+            )
+        )
+
+        with patch(
+            "app.services.mail_processor.aioimaplib.IMAP4_SSL",
+            return_value=mock_imap,
+        ):
+            emails, new_uids = await processor._fetch_imap_emails(10, set())
+
+        assert emails == []
+        assert new_uids == []