From a9f00a2d40fdd1defbf90d4d2b3cdc1ce4c2d6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Thu, 17 Feb 2022 00:43:04 +0100 Subject: [PATCH] Retry sending email on temporary failure issue11179 review354581002 --- CHANGELOG | 1 + doc/ref/sendmail.rst | 4 +++- doc/topics/configuration.rst | 7 ++++++ trytond/sendmail.py | 43 +++++++++++++++++++++++++++--------- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 7c16bd1c4b..88df4fa176 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Retry sending email on temporary failure * Order not sorted Selection by index definition * Add optional column on tree view * Use dictionary as domain on Reference field diff --git a/doc/ref/sendmail.rst b/doc/ref/sendmail.rst index 37c3ba49a6..c2e03aa99c 100644 --- a/doc/ref/sendmail.rst +++ b/doc/ref/sendmail.rst @@ -23,8 +23,10 @@ The strict value is passed to instantiate the default :class:`SMTPDataManager`. .. method:: sendmail(from_addr, to_addrs, msg[, server[, strict]]) Send email message like :meth:`sendmail_transactional` but directly without -caring about the transaction. +caring about the transaction and return the `server`. The caller may pass a server instance from `smtplib`_. +It may return a new server instance if a reconnection was needed and if the +instance comes from :meth:`get_smtp_server`. If strict is ``True``, an exception is raised if it is not possible to connect to the server. diff --git a/doc/topics/configuration.rst b/doc/topics/configuration.rst index c9cbd46044..fd1cf44a5d 100644 --- a/doc/topics/configuration.rst +++ b/doc/topics/configuration.rst @@ -390,6 +390,13 @@ For example:: from: "Company Inc" +retry +~~~~~ + +The number of retries when the SMTP server returns a temporary error. + +Default: ``5`` + session ------- diff --git a/trytond/sendmail.py b/trytond/sendmail.py index 82088ecfeb..b415e6acb5 100644 --- a/trytond/sendmail.py +++ b/trytond/sendmail.py @@ -2,6 +2,7 @@ # this repository contains the full copyright notices and license terms. import logging import smtplib +import time from email.message import Message from email.mime.text import MIMEText from email.utils import formatdate @@ -12,6 +13,7 @@ from trytond.transaction import Transaction __all__ = ['sendmail_transactional', 'sendmail', 'SMTPDataManager'] logger = logging.getLogger(__name__) +retry = config.getint('email', 'retry', default=5) def sendmail_transactional( @@ -33,20 +35,36 @@ def sendmail(from_addr, to_addrs, msg, server=None, strict=False): return quit = True else: + assert server.uri quit = False if 'Date' not in msg: msg['Date'] = formatdate() - try: - senderrs = server.sendmail(from_addr, to_addrs, msg.as_string()) - except Exception: - if strict: - raise - logger.error('fail to send email', exc_info=True) - else: - if senderrs: - logger.warning('fail to send email to %s', senderrs) + for count in range(retry, -1, -1): + if count != retry: + time.sleep(0.02 * (retry - count)) + try: + senderrs = server.sendmail(from_addr, to_addrs, msg.as_string()) + except smtplib.SMTPResponseException as e: + if count and 400 <= e.smtp_code <= 499 and hasattr(server, 'uri'): + server.quit() + server = get_smtp_server(server.uri, strict=strict) + if server: + continue + if strict: + raise + logger.error('fail to send email', exc_info=True) + except Exception: + if strict: + raise + logger.error('fail to send email', exc_info=True) + else: + if senderrs: + logger.warning('fail to send email to %s', senderrs) + break if quit: server.quit() + else: + return server def send_test_email(to_addrs, server=None): @@ -62,6 +80,7 @@ def send_test_email(to_addrs, server=None): def get_smtp_server(uri=None, strict=False): if uri is None: uri = config.get('email', 'uri') + ini_uri = uri uri = parse_uri(uri) extra = {} if uri.query: @@ -87,6 +106,7 @@ def get_smtp_server(uri=None, strict=False): server.login( unquote_plus(uri.username), unquote_plus(uri.password)) + server.uri = ini_uri return server @@ -123,7 +143,10 @@ class SMTPDataManager(object): def tpc_finish(self, trans): if self._server is not None: for from_addr, to_addrs, msg in self.queue: - sendmail(from_addr, to_addrs, msg, server=self._server) + new_server = sendmail( + from_addr, to_addrs, msg, server=self._server) + if new_server: + self._server = new_server self._server.quit() self._finish()