From 428850b4767ee727a6fe0e4ffdfd3ab6357f0f31 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 3 May 2026 22:09:08 +0000 Subject: [PATCH] refactor: address code-review comments (DNS constant, last_exc, comment clarity) Agent-Logs-Url: https://github.com/christianlouis/InboxConverge/sessions/5ad49738-4b0b-4ed7-8686-07adb0ba9e5d Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- backend/app/services/mail_processor.py | 11 +++++++---- backend/app/workers/tasks.py | 3 +++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/backend/app/services/mail_processor.py b/backend/app/services/mail_processor.py index 6c97d5c..14acb85 100644 --- a/backend/app/services/mail_processor.py +++ b/backend/app/services/mail_processor.py @@ -56,6 +56,9 @@ def _set_cached_ipv4(host: str, port: int, ipv4: str) -> None: _GOOGLE_DNS = "8.8.8.8" _DNS_PORT = 53 _DNS_TIMEOUT = 3.0 # seconds +# DNS message compression: a label starting with the two high bits set (0xC0) +# is a pointer to elsewhere in the packet rather than an inline label sequence. +_DNS_COMPRESSION_MASK = 0xC0 def _build_dns_query(hostname: str) -> bytes: @@ -85,7 +88,7 @@ def _parse_dns_a_response(data: bytes, hostname: str) -> Optional[str]: # Skip QDCOUNT questions: each is QNAME + QTYPE(2) + QCLASS(2) for _ in range(qdcount): while pos < len(data) and data[pos] != 0: - if data[pos] & 0xC0 == 0xC0: # compression pointer + if data[pos] & _DNS_COMPRESSION_MASK == _DNS_COMPRESSION_MASK: # pointer pos += 2 break pos += 1 + data[pos] @@ -97,7 +100,7 @@ def _parse_dns_a_response(data: bytes, hostname: str) -> Optional[str]: if pos >= len(data): break # Skip NAME field (may be a compression pointer or a label sequence) - if data[pos] & 0xC0 == 0xC0: + if data[pos] & _DNS_COMPRESSION_MASK == _DNS_COMPRESSION_MASK: pos += 2 else: while pos < len(data) and data[pos] != 0: @@ -822,7 +825,7 @@ class MailProcessor: return await self._fetch_pop3_emails(effective_max, seen) # IMAP: retry transient errors up to _MAX_FETCH_ATTEMPTS times. - last_exc: BaseException = RuntimeError("no attempts made") + last_exc: BaseException = MailFetchError("IMAP fetch failed") for _attempt in range(1, _MAX_FETCH_ATTEMPTS + 1): try: return await self._fetch_imap_emails(effective_max, seen) @@ -965,7 +968,7 @@ class MailProcessor: # Retry loop: transient errors (EOF, timeout, connection reset) are # retried up to _MAX_FETCH_ATTEMPTS times with a short fixed delay. - last_exc: BaseException = RuntimeError("no attempts made") + last_exc: BaseException = MailFetchError("POP3 fetch failed") for _attempt in range(1, _MAX_FETCH_ATTEMPTS + 1): try: emails, new_uids = await loop.run_in_executor(None, fetch_pop3) diff --git a/backend/app/workers/tasks.py b/backend/app/workers/tasks.py index 6d9144a..6192970 100644 --- a/backend/app/workers/tasks.py +++ b/backend/app/workers/tasks.py @@ -538,8 +538,11 @@ async def process_mail_account(account_id: int): # Per-email failures are already tracked in ProcessingLog and the # run's emails_failed counter so the user can drill into them without # having the account badge stuck in ERROR indefinitely. + # Capture pre-mutation state for the post-commit notification logic. + # These must be read before the account status and flag are cleared below. _was_in_error = account.status == AccountStatus.ERROR # type: ignore[comparison-overlap] _had_notified = bool(account.error_notification_sent) # type: ignore[attr-defined] + 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]