fix: retry POP3/IMAP, DNS 8.8.8.8 fallback, debug counter, notification backoff

Agent-Logs-Url: https://github.com/christianlouis/InboxConverge/sessions/5ad49738-4b0b-4ed7-8686-07adb0ba9e5d

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-05-03 22:06:12 +00:00
committed by GitHub
parent eea50f5d74
commit 92f60c6052
10 changed files with 1045 additions and 169 deletions
+214
View File
@@ -0,0 +1,214 @@
"""
Unit tests for the DNS 8.8.8.8 fallback resolver in mail_processor.py.
Covers:
- _build_dns_query() produces a valid DNS A-record packet
- _parse_dns_a_response() extracts the first A record correctly
- _query_google_dns_sync() sends UDP query to 8.8.8.8 and returns an IPv4
- _resolve_ipv4_sync() falls through to 8.8.8.8 when system DNS and cache fail
- _resolve_ipv4_sync() caches the 8.8.8.8 result for subsequent calls
"""
import socket
import struct
from unittest.mock import MagicMock, patch
import pytest
from app.services.mail_processor import (
_build_dns_query,
_parse_dns_a_response,
_query_google_dns_sync,
_resolve_ipv4_sync,
_dns_cache,
_dns_cache_lock,
)
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _make_dns_response(txid: int, ip: str) -> bytes:
"""Craft a minimal valid DNS A-record response for the given IP."""
flags = 0x8180 # response, recursion available
qdcount = 1
ancount = 1
header = struct.pack(">HHHHHH", txid, flags, qdcount, ancount, 0, 0)
# Question: dummy single-label name "x" + QTYPE=A + QCLASS=IN
qname = b"\x01x\x00"
question = qname + struct.pack(">HH", 1, 1)
# Answer: pointer to question name (0xC00C), TYPE=A, CLASS=IN, TTL, RDLEN=4, IP
octets = tuple(int(o) for o in ip.split("."))
answer = struct.pack(">HHHiH", 0xC00C, 1, 1, 300, 4) + bytes(octets)
return header + question + answer
# ---------------------------------------------------------------------------
# _build_dns_query
# ---------------------------------------------------------------------------
class TestBuildDnsQuery:
def test_contains_hostname_labels(self):
data = _build_dns_query("pop.example.com")
# "pop" label should appear: length byte 3 followed by b"pop"
assert b"\x03pop" in data
def test_qtype_a_and_class_in(self):
data = _build_dns_query("mail.example.com")
# Last 4 bytes of question: QTYPE=0x0001, QCLASS=0x0001
assert data[-4:] == b"\x00\x01\x00\x01"
def test_transaction_id_is_0xAB12(self):
data = _build_dns_query("x.example.com")
txid = struct.unpack(">H", data[:2])[0]
assert txid == 0xAB12
# ---------------------------------------------------------------------------
# _parse_dns_a_response
# ---------------------------------------------------------------------------
class TestParseDnsAResponse:
def test_extracts_ip_from_valid_response(self):
response = _make_dns_response(0xAB12, "1.2.3.4")
result = _parse_dns_a_response(response, "example.com")
assert result == "1.2.3.4"
def test_returns_none_for_wrong_txid(self):
response = _make_dns_response(0x1234, "1.2.3.4")
result = _parse_dns_a_response(response, "example.com")
assert result is None
def test_returns_none_for_short_data(self):
assert _parse_dns_a_response(b"\x00\x01", "example.com") is None
def test_returns_none_when_no_answers(self):
# Build a header with ancount=0
header = struct.pack(">HHHHHH", 0xAB12, 0x8180, 0, 0, 0, 0)
assert _parse_dns_a_response(header, "x") is None
# ---------------------------------------------------------------------------
# _query_google_dns_sync
# ---------------------------------------------------------------------------
class TestQueryGoogleDnsSync:
def test_returns_ip_on_success(self):
response = _make_dns_response(0xAB12, "5.6.7.8")
mock_sock = MagicMock()
mock_sock.recvfrom.return_value = (response, ("8.8.8.8", 53))
with patch("socket.socket", return_value=mock_sock):
result = _query_google_dns_sync("pop.example.com")
assert result == "5.6.7.8"
mock_sock.sendto.assert_called_once()
def test_returns_none_on_socket_error(self):
with patch("socket.socket", side_effect=OSError("network down")):
result = _query_google_dns_sync("pop.example.com")
assert result is None
def test_returns_none_on_timeout(self):
mock_sock = MagicMock()
mock_sock.recvfrom.side_effect = socket.timeout("timed out")
with patch("socket.socket", return_value=mock_sock):
result = _query_google_dns_sync("pop.example.com")
assert result is None
# ---------------------------------------------------------------------------
# _resolve_ipv4_sync — 8.8.8.8 fallback path
# ---------------------------------------------------------------------------
class TestResolveIpv4SyncGoogleFallback:
def setup_method(self):
"""Clear the DNS cache before each test to avoid state bleed."""
with _dns_cache_lock:
_dns_cache.clear()
def test_falls_through_to_google_when_system_dns_and_cache_fail(self):
with (
patch(
"socket.getaddrinfo",
side_effect=OSError("Name or service not known"),
),
patch(
"app.services.mail_processor._query_google_dns_sync",
return_value="9.10.11.12",
) as mock_google,
patch(
"app.services.mail_processor.settings.DNS_CACHE_FALLBACK_ENABLED",
False,
),
):
result = _resolve_ipv4_sync("pop.web.de", 995)
assert result == "9.10.11.12"
mock_google.assert_called_once_with("pop.web.de")
def test_google_result_is_cached(self):
with (
patch(
"socket.getaddrinfo",
side_effect=OSError("Name or service not known"),
),
patch(
"app.services.mail_processor._query_google_dns_sync",
return_value="9.10.11.12",
),
patch(
"app.services.mail_processor.settings.DNS_CACHE_FALLBACK_ENABLED",
True,
),
):
_resolve_ipv4_sync("pop.web.de", 995)
with _dns_cache_lock:
cached = _dns_cache.get(("pop.web.de", 995))
assert cached == "9.10.11.12"
def test_returns_none_when_all_strategies_fail(self):
with (
patch(
"socket.getaddrinfo",
side_effect=OSError("Name or service not known"),
),
patch(
"app.services.mail_processor._query_google_dns_sync",
return_value=None,
),
patch(
"app.services.mail_processor.settings.DNS_CACHE_FALLBACK_ENABLED",
False,
),
):
result = _resolve_ipv4_sync("nonexistent.invalid", 995)
assert result is None
def test_system_dns_success_skips_google(self):
with (
patch(
"socket.getaddrinfo",
return_value=[(None, None, None, None, ("1.2.3.4", 995))],
),
patch(
"app.services.mail_processor._query_google_dns_sync",
) as mock_google,
patch(
"app.services.mail_processor.settings.DNS_CACHE_FALLBACK_ENABLED",
False,
),
):
result = _resolve_ipv4_sync("pop.example.com", 995)
assert result == "1.2.3.4"
mock_google.assert_not_called()
@@ -854,3 +854,90 @@ class TestForwardEmail:
body_text = body_part.get_payload(decode=True).decode("utf-8")
# Body should have header info but no HTML content extracted
assert "Originally from: x@y.com" in body_text
# ---------------------------------------------------------------------------
# Retry logic tests
# ---------------------------------------------------------------------------
class TestFetchPop3Retry:
"""_fetch_pop3_emails retries on transient errors."""
@patch("app.services.mail_processor._resolve_ipv4_sync", return_value=None)
@patch("app.services.mail_processor.poplib")
@pytest.mark.asyncio
async def test_retries_on_eof_error(self, mock_poplib, _mock_resolve):
"""EOF error on first attempt triggers a retry; second attempt succeeds."""
import asyncio
import poplib as real_poplib
# First call raises EOF; second succeeds
good_conn = MagicMock()
good_conn.stat.return_value = (0, 0)
good_conn.uidl.return_value = (b"+OK", [], 0)
good_conn.quit.return_value = None
mock_poplib.error_proto = real_poplib.error_proto
mock_poplib.POP3_SSL.side_effect = [
real_poplib.error_proto("-ERR EOF"),
good_conn,
]
account = _make_account(protocol="pop3_ssl")
proc = MailProcessor(account, "secret")
with patch(
"app.services.mail_processor.asyncio.sleep", new_callable=AsyncMock
) as mock_sleep:
emails, uids = await proc._fetch_pop3_emails(10, set())
assert emails == []
assert uids == []
# sleep should have been called once between attempt 1 and attempt 2
mock_sleep.assert_awaited_once()
@patch("app.services.mail_processor._resolve_ipv4_sync", return_value=None)
@patch("app.services.mail_processor.poplib")
@pytest.mark.asyncio
async def test_raises_after_max_attempts(self, mock_poplib, _mock_resolve):
"""All attempts fail with EOF → MailFetchError is raised."""
import poplib as real_poplib
mock_poplib.error_proto = real_poplib.error_proto
mock_poplib.POP3_SSL.side_effect = real_poplib.error_proto("-ERR EOF")
account = _make_account(protocol="pop3_ssl")
proc = MailProcessor(account, "secret")
with patch(
"app.services.mail_processor.asyncio.sleep", new_callable=AsyncMock
):
with pytest.raises(MailFetchError):
await proc._fetch_pop3_emails(10, set())
@patch("app.services.mail_processor._resolve_ipv4_sync", return_value=None)
@patch("app.services.mail_processor.poplib")
@pytest.mark.asyncio
async def test_no_retry_on_auth_error(self, mock_poplib, _mock_resolve):
"""Authentication errors are not retried (non-transient)."""
import poplib as real_poplib
conn = MagicMock()
mock_poplib.error_proto = real_poplib.error_proto
mock_poplib.POP3_SSL.return_value = conn
# user() succeeds; pass_() raises auth error
conn.user.return_value = b"+OK"
conn.pass_.side_effect = real_poplib.error_proto("-ERR Authentication failed")
account = _make_account(protocol="pop3_ssl")
proc = MailProcessor(account, "secret")
with patch(
"app.services.mail_processor.asyncio.sleep", new_callable=AsyncMock
) as mock_sleep:
with pytest.raises(MailFetchError):
await proc._fetch_pop3_emails(10, set())
# No sleep = no retry
mock_sleep.assert_not_awaited()
+246
View File
@@ -1822,3 +1822,249 @@ class TestCleanupOldLogs:
# Should not raise
await cleanup_old_logs.run(days_to_keep=30)
# ---------------------------------------------------------------------------
# Debug-logging counter auto-disable tests
# ---------------------------------------------------------------------------
class TestDebugLoggingCounter:
"""debug_logging is auto-disabled after exactly 5 runs since it was enabled."""
def _make_account_with_debug(self, run_count: int = 0) -> MagicMock:
account = _make_account(delivery_method=DeliveryMethod.GMAIL_API)
account.debug_logging = True
account.debug_logging_run_count = run_count
account.error_notification_sent = False
account.status = MagicMock(value="active")
return account
def _build_task_mocks(self, account):
maker, session = _mock_session_maker()
gmail_cred = _make_gmail_cred(user_id=account.user_id)
account_result = MagicMock()
account_result.scalar_one_or_none.return_value = account
seen_result = MagicMock()
seen_result.scalars.return_value.all.return_value = []
gmail_cred_result = MagicMock()
gmail_cred_result.scalar_one_or_none.return_value = gmail_cred
session.execute = AsyncMock(
side_effect=[account_result, seen_result, gmail_cred_result]
)
session.commit = AsyncMock()
session.refresh = AsyncMock()
return maker, session, gmail_cred
@pytest.mark.asyncio
async def test_counter_increments_each_run(self):
"""debug_logging_run_count increments from 0 to 1 after one successful run."""
account = self._make_account_with_debug(run_count=0)
maker, session, gmail_cred = self._build_task_mocks(account)
mock_processor = AsyncMock()
mock_processor.fetch_emails.return_value = ([], [])
mock_gmail_svc = _make_gmail_service()
with (
patch(f"{MODULE}.async_session_maker", maker),
patch(f"{MODULE}.engine", AsyncMock()),
patch(f"{MODULE}.decrypt_credential", return_value="pw"),
patch(f"{MODULE}.MailProcessor", return_value=mock_processor),
patch(f"{MODULE}.GmailService", return_value=mock_gmail_svc),
patch(f"{MODULE}.send_user_notification", new_callable=AsyncMock),
):
from app.workers.tasks import process_mail_account
await process_mail_account.run(1)
assert account.debug_logging_run_count == 1
assert account.debug_logging is True
@pytest.mark.asyncio
async def test_debug_disabled_at_run_5(self):
"""debug_logging turns off and counter resets when it reaches 5."""
account = self._make_account_with_debug(run_count=4)
maker, session, gmail_cred = self._build_task_mocks(account)
mock_processor = AsyncMock()
mock_processor.fetch_emails.return_value = ([], [])
mock_gmail_svc = _make_gmail_service()
with (
patch(f"{MODULE}.async_session_maker", maker),
patch(f"{MODULE}.engine", AsyncMock()),
patch(f"{MODULE}.decrypt_credential", return_value="pw"),
patch(f"{MODULE}.MailProcessor", return_value=mock_processor),
patch(f"{MODULE}.GmailService", return_value=mock_gmail_svc),
patch(f"{MODULE}.send_user_notification", new_callable=AsyncMock),
):
from app.workers.tasks import process_mail_account
await process_mail_account.run(1)
assert account.debug_logging is False
assert account.debug_logging_run_count == 0
@pytest.mark.asyncio
async def test_debug_not_disabled_at_run_4(self):
"""debug_logging stays on at run 4 (need one more)."""
account = self._make_account_with_debug(run_count=3)
maker, session, gmail_cred = self._build_task_mocks(account)
mock_processor = AsyncMock()
mock_processor.fetch_emails.return_value = ([], [])
mock_gmail_svc = _make_gmail_service()
with (
patch(f"{MODULE}.async_session_maker", maker),
patch(f"{MODULE}.engine", AsyncMock()),
patch(f"{MODULE}.decrypt_credential", return_value="pw"),
patch(f"{MODULE}.MailProcessor", return_value=mock_processor),
patch(f"{MODULE}.GmailService", return_value=mock_gmail_svc),
patch(f"{MODULE}.send_user_notification", new_callable=AsyncMock),
):
from app.workers.tasks import process_mail_account
await process_mail_account.run(1)
assert account.debug_logging is True
assert account.debug_logging_run_count == 4
# ---------------------------------------------------------------------------
# Notification backoff tests
# ---------------------------------------------------------------------------
class TestNotificationBackoff:
"""Failure notifications should only fire once per error streak."""
def _make_smtp_account(self, **overrides):
account = _make_account(delivery_method=DeliveryMethod.SMTP, **overrides)
account.debug_logging = False
account.debug_logging_run_count = 0
account.error_notification_sent = overrides.get("error_notification_sent", False)
account.status = MagicMock(value="active")
return account
def _smtp_session(self, account):
maker, session = _mock_session_maker()
user_smtp = MagicMock()
user_smtp.host = "smtp.x.com"
user_smtp.port = 587
user_smtp.username = "u"
user_smtp.encrypted_password = "ep"
user_smtp.use_tls = True
account_result = MagicMock()
account_result.scalar_one_or_none.return_value = account
seen_result = MagicMock()
seen_result.scalars.return_value.all.return_value = []
smtp_result = MagicMock()
smtp_result.scalar_one_or_none.return_value = user_smtp
session.execute = AsyncMock(
side_effect=[account_result, seen_result, smtp_result]
)
session.commit = AsyncMock()
session.refresh = AsyncMock()
return maker, session
@pytest.mark.asyncio
async def test_first_failure_sends_notification(self):
"""First email-forwarding failure in a streak sends a notification."""
raw_email = _build_raw_email()
account = self._make_smtp_account(error_notification_sent=False)
maker, session = self._smtp_session(account)
mock_processor = AsyncMock()
mock_processor.post_process_messages = AsyncMock()
mock_processor.fetch_emails.return_value = ([raw_email], ["uid-1"])
mock_notif_session = MagicMock()
mock_notif_ctx = MagicMock()
mock_notif_ctx.__aenter__ = AsyncMock(return_value=mock_notif_session)
mock_notif_ctx.__aexit__ = AsyncMock(return_value=False)
mock_notif_ctx.begin = MagicMock(return_value=mock_notif_ctx)
mock_notif_session.execute = AsyncMock()
def session_maker_side_effect():
return mock_notif_ctx
mock_send_notification = AsyncMock(return_value=1)
with (
patch(f"{MODULE}.async_session_maker", side_effect=[maker(), session_maker_side_effect()]),
patch(f"{MODULE}.engine", AsyncMock()),
patch(f"{MODULE}.decrypt_credential", return_value="pw"),
patch(f"{MODULE}.MailProcessor", return_value=mock_processor),
patch(
f"{MODULE}.MailProcessor.forward_email",
new_callable=AsyncMock,
return_value=False,
),
patch(f"{MODULE}.send_user_notification", mock_send_notification),
):
from app.workers.tasks import process_mail_account
await process_mail_account.run(1)
mock_send_notification.assert_awaited()
@pytest.mark.asyncio
async def test_second_failure_suppressed(self):
"""When error_notification_sent is already True, no new notification fires."""
raw_email = _build_raw_email()
account = self._make_smtp_account(error_notification_sent=True)
maker, session = self._smtp_session(account)
mock_processor = AsyncMock()
mock_processor.post_process_messages = AsyncMock()
mock_processor.fetch_emails.return_value = ([raw_email], ["uid-1"])
mock_send_notification = AsyncMock(return_value=0)
with (
patch(f"{MODULE}.async_session_maker", maker),
patch(f"{MODULE}.engine", AsyncMock()),
patch(f"{MODULE}.decrypt_credential", return_value="pw"),
patch(f"{MODULE}.MailProcessor", return_value=mock_processor),
patch(
f"{MODULE}.MailProcessor.forward_email",
new_callable=AsyncMock,
return_value=False,
),
patch(f"{MODULE}.send_user_notification", mock_send_notification),
):
from app.workers.tasks import process_mail_account
await process_mail_account.run(1)
mock_send_notification.assert_not_awaited()
@pytest.mark.asyncio
async def test_recovery_clears_flag(self):
"""A successful run clears error_notification_sent."""
account = self._make_smtp_account(error_notification_sent=False)
# Put account in "active" status recovery notice fires when was ERROR
account.status = MagicMock(value="active")
maker, session = self._smtp_session(account)
mock_processor = AsyncMock()
mock_processor.fetch_emails.return_value = ([], [])
mock_processor.post_process_messages = AsyncMock()
with (
patch(f"{MODULE}.async_session_maker", maker),
patch(f"{MODULE}.engine", AsyncMock()),
patch(f"{MODULE}.decrypt_credential", return_value="pw"),
patch(f"{MODULE}.MailProcessor", return_value=mock_processor),
patch(f"{MODULE}.send_user_notification", new_callable=AsyncMock),
):
from app.workers.tasks import process_mail_account
await process_mail_account.run(1)
# Flag is reset to False after successful connection
assert account.error_notification_sent is False