redesign
This commit is contained in:
@@ -328,6 +328,7 @@ def send_email_message(
|
||||
|
||||
attempts: list[dict[str, Any]] = []
|
||||
last_error = ""
|
||||
fallback_from_address = smtp_username if smtp_username and smtp_username != from_address else None
|
||||
for mode in _smtp_mode_candidates(settings):
|
||||
meta = _smtp_attempt_metadata(mode)
|
||||
logger.info(
|
||||
@@ -341,40 +342,6 @@ def send_email_message(
|
||||
)
|
||||
try:
|
||||
_send_email_via_mode(mode, message, recipients, from_address)
|
||||
attempts.append(
|
||||
{
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
"status": "ok",
|
||||
}
|
||||
)
|
||||
logger.info(
|
||||
"SMTP send succeeded label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s",
|
||||
meta["label"],
|
||||
meta["smtp_host"],
|
||||
meta["smtp_port"],
|
||||
meta["transport"],
|
||||
meta["auth_user"],
|
||||
from_address,
|
||||
)
|
||||
return {
|
||||
"from_address": from_address,
|
||||
"recipient_count": len(recipients),
|
||||
"attachment_count": len(attachment_list),
|
||||
"subject": subject,
|
||||
"authenticated_as": smtp_username or None,
|
||||
"selected_mode": {
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
},
|
||||
"attempts": attempts,
|
||||
}
|
||||
except Exception as exc:
|
||||
last_error = describe_smtp_error(exc)
|
||||
attempts.append(
|
||||
@@ -388,9 +355,9 @@ def send_email_message(
|
||||
"error": last_error,
|
||||
}
|
||||
)
|
||||
if _smtp_sender_not_authorized(exc):
|
||||
if _smtp_sender_not_authorized(exc) and fallback_from_address:
|
||||
logger.warning(
|
||||
"SMTP send sender rejected label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s error=%s",
|
||||
"SMTP send sender rejected label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s error=%s; retrying with smtp_username",
|
||||
meta["label"],
|
||||
meta["smtp_host"],
|
||||
meta["smtp_port"],
|
||||
@@ -399,16 +366,124 @@ def send_email_message(
|
||||
from_address,
|
||||
last_error,
|
||||
)
|
||||
else:
|
||||
logger.warning(
|
||||
"SMTP send failed label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s error=%s",
|
||||
fallback_message, fallback_from = build_email_message(
|
||||
settings,
|
||||
recipients,
|
||||
subject,
|
||||
html_body,
|
||||
text_body,
|
||||
attachment_list,
|
||||
sender_address=fallback_from_address,
|
||||
reply_to_address=from_address,
|
||||
)
|
||||
try:
|
||||
_send_email_via_mode(mode, fallback_message, recipients, fallback_from)
|
||||
except Exception as fallback_exc:
|
||||
last_error = describe_smtp_error(fallback_exc)
|
||||
attempts.append(
|
||||
{
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
"status": "failed",
|
||||
"error": last_error,
|
||||
"sender_fallback": True,
|
||||
}
|
||||
)
|
||||
logger.warning(
|
||||
"SMTP send fallback failed label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s error=%s",
|
||||
meta["label"],
|
||||
meta["smtp_host"],
|
||||
meta["smtp_port"],
|
||||
meta["transport"],
|
||||
meta["auth_user"],
|
||||
fallback_from,
|
||||
last_error,
|
||||
)
|
||||
continue
|
||||
|
||||
attempts.append(
|
||||
{
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
"status": "ok",
|
||||
"sender_fallback": True,
|
||||
}
|
||||
)
|
||||
logger.info(
|
||||
"SMTP send succeeded via smtp_username label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s",
|
||||
meta["label"],
|
||||
meta["smtp_host"],
|
||||
meta["smtp_port"],
|
||||
meta["transport"],
|
||||
meta["auth_user"],
|
||||
from_address,
|
||||
last_error,
|
||||
fallback_from,
|
||||
)
|
||||
return {
|
||||
"from_address": fallback_from,
|
||||
"recipient_count": len(recipients),
|
||||
"attachment_count": len(attachment_list),
|
||||
"subject": subject,
|
||||
"authenticated_as": smtp_username or None,
|
||||
"selected_mode": {
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
},
|
||||
"attempts": attempts,
|
||||
}
|
||||
logger.warning(
|
||||
"SMTP send failed label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s error=%s",
|
||||
meta["label"],
|
||||
meta["smtp_host"],
|
||||
meta["smtp_port"],
|
||||
meta["transport"],
|
||||
meta["auth_user"],
|
||||
from_address,
|
||||
last_error,
|
||||
)
|
||||
continue
|
||||
|
||||
attempts.append(
|
||||
{
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
"status": "ok",
|
||||
}
|
||||
)
|
||||
logger.info(
|
||||
"SMTP send succeeded label=%s host=%s port=%s transport=%s auth_user=%s from_address=%s",
|
||||
meta["label"],
|
||||
meta["smtp_host"],
|
||||
meta["smtp_port"],
|
||||
meta["transport"],
|
||||
meta["auth_user"],
|
||||
from_address,
|
||||
)
|
||||
return {
|
||||
"from_address": from_address,
|
||||
"recipient_count": len(recipients),
|
||||
"attachment_count": len(attachment_list),
|
||||
"subject": subject,
|
||||
"authenticated_as": smtp_username or None,
|
||||
"selected_mode": {
|
||||
"label": meta["label"],
|
||||
"smtp_host": meta["smtp_host"],
|
||||
"smtp_port": meta["smtp_port"],
|
||||
"use_tls": meta["use_tls"],
|
||||
"use_ssl": meta["use_ssl"],
|
||||
},
|
||||
"attempts": attempts,
|
||||
}
|
||||
|
||||
raise RuntimeError(last_error or "SMTP delivery failed")
|
||||
|
||||
@@ -103,7 +103,7 @@ class MailerTests(unittest.TestCase):
|
||||
self.assertEqual(result["attempts"][1]["status"], "ok")
|
||||
fallback_smtp.send_message.assert_called_once()
|
||||
|
||||
def test_send_email_message_rejects_unauthorized_from_address(self) -> None:
|
||||
def test_send_email_message_retries_with_smtp_username_when_from_is_rejected(self) -> None:
|
||||
settings = SimpleNamespace(
|
||||
smtp_host="smtp.example.com",
|
||||
smtp_port=587,
|
||||
@@ -116,25 +116,30 @@ class MailerTests(unittest.TestCase):
|
||||
smtp_timeout=15,
|
||||
)
|
||||
smtp = MagicMock()
|
||||
smtp.send_message.side_effect = smtplib.SMTPDataError(
|
||||
551, b"5.7.1 Not authorised to send from this header address"
|
||||
)
|
||||
smtp.send_message.side_effect = [
|
||||
smtplib.SMTPDataError(551, b"5.7.1 Not authorised to send from this header address"),
|
||||
{},
|
||||
]
|
||||
smtp_factory = MagicMock(return_value=_SMTPContext(smtp))
|
||||
|
||||
with patch("media_library_viewer_api.services.mailer.smtplib.SMTP", smtp_factory), patch(
|
||||
"media_library_viewer_api.services.mailer.smtplib.SMTP_SSL"
|
||||
) as smtp_ssl:
|
||||
with self.assertRaises(RuntimeError) as ctx:
|
||||
send_email_message(
|
||||
settings,
|
||||
recipients=["alex@example.com"],
|
||||
subject="Hello",
|
||||
html_body="<p>Hello</p>",
|
||||
)
|
||||
result = send_email_message(
|
||||
settings,
|
||||
recipients=["alex@example.com"],
|
||||
subject="Hello",
|
||||
html_body="<p>Hello</p>",
|
||||
)
|
||||
|
||||
smtp_ssl.assert_not_called()
|
||||
self.assertIn("authorized alias", str(ctx.exception).lower())
|
||||
self.assertEqual(smtp.send_message.call_count, 1)
|
||||
self.assertEqual(result["from_address"], "mailer@example.com")
|
||||
self.assertEqual(smtp.send_message.call_count, 2)
|
||||
first_message = smtp.send_message.call_args_list[0].args[0]
|
||||
second_message = smtp.send_message.call_args_list[1].args[0]
|
||||
self.assertEqual(first_message["From"], "Manage <alias@example.com>")
|
||||
self.assertEqual(second_message["From"], "Manage <mailer@example.com>")
|
||||
self.assertEqual(second_message["Reply-To"], "alias@example.com")
|
||||
|
||||
def test_describe_smtp_error_handles_timeout(self) -> None:
|
||||
detail = describe_smtp_error(TimeoutError("timed out"))
|
||||
|
||||
Reference in New Issue
Block a user