From 4e9b2f59957c59992fcb4178ad9ae40a26d8d6fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 30 Jul 2025 13:34:24 +0200 Subject: [PATCH] Allow to define more than one smtp server --- app/config.py | 4 +- app/email_utils.py | 5 +- app/mail_sender.py | 125 +++++++++++++++++++++----------------- tests/test_mail_sender.py | 22 +++---- 4 files changed, 85 insertions(+), 71 deletions(-) diff --git a/app/config.py b/app/config.py index 784f1a90..63ad273d 100644 --- a/app/config.py +++ b/app/config.py @@ -144,8 +144,8 @@ MAX_NB_SUBDOMAIN = 5 ENFORCE_SPF = "ENFORCE_SPF" in os.environ # override postfix server locally -# use 240.0.0.1 here instead of 10.0.0.1 as existing SL instances use the 240.0.0.0 network -POSTFIX_SERVER = os.environ.get("POSTFIX_SERVER", "240.0.0.1") +POSTFIX_SERVERS = get_env_csv("POSTFIX_SERVER", "240.0.0.1") +POSTFIX_BACKUP_SERVERS = get_env_csv("POSTFIX_BACKUP_SERVER", "") DISABLE_REGISTRATION = "DISABLE_REGISTRATION" in os.environ diff --git a/app/email_utils.py b/app/email_utils.py index 4463fd40..0aa31242 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -1346,11 +1346,12 @@ def spf_pass( @cached(cache=TTLCache(maxsize=2, ttl=20)) def get_smtp_server(): LOG.d("get a smtp server") + server = random.choice(config.POSTFIX_SERVERS) if config.POSTFIX_SUBMISSION_TLS: - smtp = SMTP(config.POSTFIX_SERVER, 587) + smtp = SMTP(server, 587) smtp.starttls() else: - smtp = SMTP(config.POSTFIX_SERVER, config.POSTFIX_PORT) + smtp = SMTP(server, config.POSTFIX_PORT) return smtp diff --git a/app/mail_sender.py b/app/mail_sender.py index 797bffc4..278a00a1 100644 --- a/app/mail_sender.py +++ b/app/mail_sender.py @@ -4,6 +4,7 @@ import base64 import email import json import os +import random import time import uuid from concurrent.futures import ThreadPoolExecutor @@ -144,69 +145,81 @@ class MailSender: return True def _send_to_smtp(self, send_request: SendRequest, retries: int) -> bool: - start = time.time() - try: - with SMTP( - config.POSTFIX_SERVER, - config.POSTFIX_PORT, - timeout=config.POSTFIX_TIMEOUT, - ) as smtp: - if config.POSTFIX_SUBMISSION_TLS: - smtp.starttls() - - elapsed = time.time() - start - LOG.d("getting a smtp connection takes seconds %s", elapsed) + servers_to_try = config.POSTFIX_SERVERS.copy() + random.shuffle(servers_to_try) + if config.POSTFIX_BACKUP_SERVERS: + servers_to_try.extend(config.POSTFIX_BACKUP_SERVERS) + servers_tried = 0 + for server_hostname in servers_to_try: + servers_tried += 1 + start = time.time() + try: + return self.__send_to_server(server_hostname, send_request) + except ( + SMTPException, + ConnectionRefusedError, + TimeoutError, + ) as e: + LOG.w(f"Got error {e} while sending email to {server_hostname}") + newrelic.agent.record_custom_event("SmtpError", {"error": e.__class__}) + finally: newrelic.agent.record_custom_metric( - "Custom/smtp_connection_time", elapsed + "Custom/smtp_servers_tried", servers_tried ) - - # smtp.send_message has UnicodeEncodeError - # encode message raw directly instead - LOG.d( - "Sendmail mail_from:%s, rcpt_to:%s, header_from:%s, header_to:%s, header_cc:%s", - send_request.envelope_from, - send_request.envelope_to, - send_request.msg[headers.FROM], - send_request.msg[headers.TO], - send_request.msg[headers.CC], - ) - smtp.sendmail( - send_request.envelope_from, - send_request.envelope_to, - message_to_bytes(send_request.msg), - send_request.mail_options, - send_request.rcpt_options, - ) - newrelic.agent.record_custom_metric( "Custom/smtp_sending_time", time.time() - start ) - return True - except ( - SMTPException, - ConnectionRefusedError, - TimeoutError, - ) as e: - newrelic.agent.record_custom_metric( - "Custom/smtp_sending_time", time.time() - start + if retries > 0: + LOG.warning( + f"Retrying sending email due to error. {retries} retries left. Will wait {0.3*retries} seconds." ) - newrelic.agent.record_custom_event("SmtpError", {"error": e.__class__}) - if retries > 0: - LOG.warning( - f"Retrying sending email due to error {e}. {retries} retries left. Will wait {0.3*retries} seconds." - ) - time.sleep(0.3 * retries) - return self._send_to_smtp(send_request, retries - 1) - else: - if send_request.ignore_smtp_errors: - LOG.e(f"Ignore smtp error {e}") - return False - LOG.e( - f"Could not send message to smtp server {config.POSTFIX_SERVER}:{config.POSTFIX_PORT}" - ) - if config.SAVE_UNSENT_DIR: - send_request.save_request_to_unsent_dir() + time.sleep(0.3 * retries) + return self._send_to_smtp(send_request, retries - 1) + else: + if send_request.ignore_smtp_errors: + LOG.e("Ignore smtp error") return False + LOG.e( + f"Could not send message to smtp server {config.POSTFIX_SERVERS}:{config.POSTFIX_PORT}" + ) + if config.SAVE_UNSENT_DIR: + send_request.save_request_to_unsent_dir() + return False + + def __send_to_server(self, server_host: str, send_request: SendRequest): + start = time.time() + with SMTP( + server_host, + config.POSTFIX_PORT, + timeout=config.POSTFIX_TIMEOUT, + ) as smtp: + if config.POSTFIX_SUBMISSION_TLS: + smtp.starttls() + + elapsed = time.time() - start + LOG.d( + f"Getting a smtp connection to {server_host} takes seconds {elapsed:.3} seconds" + ) + newrelic.agent.record_custom_metric("Custom/smtp_connection_time", elapsed) + + # smtp.send_message has UnicodeEncodeError + # encode message raw directly instead + LOG.d( + "Sendmail mail_from:%s, rcpt_to:%s, header_from:%s, header_to:%s, header_cc:%s", + send_request.envelope_from, + send_request.envelope_to, + send_request.msg[headers.FROM], + send_request.msg[headers.TO], + send_request.msg[headers.CC], + ) + smtp.sendmail( + send_request.envelope_from, + send_request.envelope_to, + message_to_bytes(send_request.msg), + send_request.mail_options, + send_request.rcpt_options, + ) + return True mail_sender = MailSender() diff --git a/tests/test_mail_sender.py b/tests/test_mail_sender.py index 065b6dfc..254b5bb2 100644 --- a/tests/test_mail_sender.py +++ b/tests/test_mail_sender.py @@ -1,7 +1,7 @@ import os +import socket import tempfile import threading -import socket from email.message import Message from random import random from typing import Callable @@ -9,13 +9,13 @@ from typing import Callable import pytest from aiosmtpd.controller import Controller +from app import config from app.email import headers from app.mail_sender import ( mail_sender, SendRequest, load_unsent_mails_from_fs_and_resend, ) -from app import config def create_dummy_send_request() -> SendRequest: @@ -105,8 +105,8 @@ def compare_send_requests(expected: SendRequest, request: SendRequest): ], ) def test_mail_sender_save_unsent_to_disk(server_fn): - original_postfix_server = config.POSTFIX_SERVER - config.POSTFIX_SERVER = "localhost" + original_postfix_server = config.POSTFIX_SERVERS + config.POSTFIX_SERVERS = ["localhost"] config.NOT_SEND_EMAIL = False config.POSTFIX_SUBMISSION_TLS = False config.POSTFIX_PORT = server_fn() @@ -122,14 +122,14 @@ def test_mail_sender_save_unsent_to_disk(server_fn): ) compare_send_requests(loaded_send_request, send_request) finally: - config.POSTFIX_SERVER = original_postfix_server + config.POSTFIX_SERVERS = original_postfix_server config.NOT_SEND_EMAIL = True @mail_sender.store_emails_test_decorator def test_send_unsent_email_from_fs(): - original_postfix_server = config.POSTFIX_SERVER - config.POSTFIX_SERVER = "localhost" + original_postfix_server = config.POSTFIX_SERVERS + config.POSTFIX_SERVERS = ["localhost"] config.NOT_SEND_EMAIL = False with tempfile.TemporaryDirectory() as temp_dir: try: @@ -137,7 +137,7 @@ def test_send_unsent_email_from_fs(): send_request = create_dummy_send_request() assert not mail_sender.send(send_request, 1) finally: - config.POSTFIX_SERVER = original_postfix_server + config.POSTFIX_SERVERS = original_postfix_server config.NOT_SEND_EMAIL = True saved_files = os.listdir(config.SAVE_UNSENT_DIR) assert len(saved_files) == 1 @@ -154,8 +154,8 @@ def test_send_unsent_email_from_fs(): @mail_sender.store_emails_test_decorator def test_failed_resend_does_not_delete_file(): - original_postfix_server = config.POSTFIX_SERVER - config.POSTFIX_SERVER = "localhost" + original_postfix_server = config.POSTFIX_SERVERS + config.POSTFIX_SERVERS = ["localhost"] config.NOT_SEND_EMAIL = False try: with tempfile.TemporaryDirectory() as temp_dir: @@ -176,7 +176,7 @@ def test_failed_resend_does_not_delete_file(): # No more emails are stored in disk assert saved_files == os.listdir(config.SAVE_UNSENT_DIR) finally: - config.POSTFIX_SERVER = original_postfix_server + config.POSTFIX_SERVERS = original_postfix_server config.NOT_SEND_EMAIL = True