pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] mail: add 'email_from' failure types log

Browse Source 
Purpose of this commit is to try to detect and log 'email_from' invalid
values when sending emails based on outgoing 'mail.mail'. This implies
checking the returned messages when having a generic Exception when sending
the emails, as we distinguish two use cases that raise through a simple
raise: missing from and invalid from.

New failure types 'mail_from_missing' and 'mail_from_invalid' are also
added at 'mail.notification' and 'mailing.trace' level, as other failure
types.

Task-3547653 (Mail: Add error type for wrong email_from)
Prepares Task-36879 (Mail: Support MultiCompany Aliases)

closes odoo/odoo#138202

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Thibault Delavallée
2023-10-10 14:07:16 +00:00
parent 3ffa1a0611
commit 2d7af57bd5
6 changed files with 92 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/models/mail_mail.py
+32 -3
View File
@@ -74,6 +74,8 @@ class MailMail(models.Model):
# mail
("mail_email_invalid", "Invalid email address"),
("mail_email_missing", "Missing email"),
("mail_from_invalid", "Invalid from address"),
("mail_from_missing", "Missing from address"),
("mail_smtp", "Connection failed (outgoing mail server problem)"),
# mass mode
("mail_bl", "Blacklisted Address"),
@@ -561,6 +563,7 @@ class MailMail(models.Model):
for mail_id in self.ids:
success_pids = []
failure_reason = None
failure_type = None
processing_pid = None
mail = None
@@ -667,10 +670,36 @@ class MailMail(models.Model):
mail.id, mail.message_id)
raise
except Exception as e:
failure_reason = tools.ustr(e)
if isinstance(e, AssertionError):
# Handle assert raised in IrMailServer to try to catch notably from-specific errors.
# Note that assert may raise several args, a generic error string then a specific
# message for logging in failure type
error_code = e.args[0]
if len(e.args) > 1 and error_code == IrMailServer.NO_VALID_FROM:
# log failing email in additional arguments message
failure_reason = tools.ustr(e.args[1])
else:
failure_reason = error_code
if error_code == IrMailServer.NO_VALID_FROM:
failure_type = "mail_from_invalid"
elif error_code in (IrMailServer.NO_FOUND_FROM, IrMailServer.NO_FOUND_SMTP_FROM):
failure_type = "mail_from_missing"
# generic (unknown) error as fallback
if not failure_reason:
failure_reason = tools.ustr(e)
if not failure_type:
failure_type = "unknown"
_logger.exception('failed sending mail (id: %s) due to %s', mail.id, failure_reason)
mail.write({'state': 'exception', 'failure_reason': failure_reason})
mail._postprocess_sent_message(success_pids=success_pids, failure_reason=failure_reason, failure_type='unknown')
mail.write({
"failure_reason": failure_reason,
"failure_type": failure_type,
"state": "exception",
})
mail._postprocess_sent_message(
success_pids=success_pids,
failure_reason=failure_reason, failure_type=failure_type
)
if raise_exception:
if isinstance(e, (AssertionError, UnicodeEncodeError)):
if isinstance(e, UnicodeEncodeError):
addons/mail/models/mail_message.py
+2 -1
View File
@@ -32,7 +32,8 @@ class Message(models.Model):
# generic
unknown,
# mail
"mail_email_invalid", "mail_smtp", "mail_email_missing"
"mail_email_invalid", "mail_smtp", "mail_email_missing",
"mail_from_invalid", "mail_from_missing",
# sms (SMS addon)
'sms_number_missing', 'sms_number_format', 'sms_credit',
'sms_server', 'sms_acc'
addons/mail/models/mail_notification.py
+2
View File
@@ -41,6 +41,8 @@ class MailNotification(models.Model):
("mail_bounce", "Bounce"),
("mail_email_invalid", "Invalid email address"),
("mail_email_missing", "Missing email address"),
("mail_from_invalid", "Invalid from address"),
("mail_from_missing", "Missing from address"),
("mail_smtp", "Connection failed (outgoing mail server problem)"),
], string='Failure type')
failure_reason = fields.Text('Failure reason', copy=False)
addons/mass_mailing/models/mailing_trace.py
+4 -1
View File
@@ -19,7 +19,8 @@ class MailingTrace(models.Model):
# generic
'unknown',
# mass_mailing
"mail_email_invalid", "mail_smtp", "mail_email_missing"
"mail_email_invalid", "mail_smtp", "mail_email_missing",
"mail_from_invalid", "mail_from_missing",
# mass mailing mass mode specific codes
"mail_bl", "mail_optout", "mail_dup"
# mass_mailing_sms
@@ -93,6 +94,8 @@ class MailingTrace(models.Model):
("mail_bounce", "Bounce"),
("mail_email_invalid", "Invalid email address"),
("mail_email_missing", "Missing email address"),
("mail_from_invalid", "Invalid from address"),
("mail_from_missing", "Missing from address"),
("mail_smtp", "Connection failed (outgoing mail server problem)"),
# mass mode
("mail_bl", "Blacklisted Address"),
addons/test_mail/tests/test_mail_mail.py
+38 -48
View File
@@ -292,39 +292,41 @@ class TestMailMail(MailCommon):
managed and stored at mail and notification level. """
mail, notification = self.test_mail, self.test_notification
# MailServer.build_email(): invalid from
self.env['ir.config_parameter'].set_param('mail.default.from', '')
self._reset_data()
with self.mock_mail_gateway(), mute_logger('odoo.addons.mail.models.mail_mail'):
mail.send(raise_exception=False)
self.assertFalse(self._mails[0]['email_from'])
self.assertEqual(
mail.failure_reason,
'You must either provide a sender address explicitly or configure using the combination of `mail.catchall.domain` and `mail.default.from` ICPs, in the server configuration file or with the --email-from startup parameter.')
self.assertFalse(mail.failure_type, 'Mail: void from: no failure type, should be updated')
self.assertEqual(mail.state, 'exception')
self.assertEqual(
notification.failure_reason,
'You must either provide a sender address explicitly or configure using the combination of `mail.catchall.domain` and `mail.default.from` ICPs, in the server configuration file or with the --email-from startup parameter.')
self.assertEqual(notification.failure_type, 'unknown', 'Mail: void from: unknown failure type, should be updated')
self.assertEqual(notification.notification_status, 'exception')
# MailServer.build_email(): invalid from (missing)
for default_from in [False, '']:
self.env['ir.config_parameter'].set_param('mail.default.from', default_from)
self._reset_data()
with self.mock_mail_gateway(), mute_logger('odoo.addons.mail.models.mail_mail'):
mail.send(raise_exception=False)
self.assertFalse(self._mails[0]['email_from'])
self.assertEqual(
mail.failure_reason,
'You must either provide a sender address explicitly or configure using the combination of `mail.catchall.domain` and `mail.default.from` ICPs, in the server configuration file or with the --email-from startup parameter.')
self.assertEqual(mail.failure_type, 'mail_from_missing')
self.assertEqual(mail.state, 'exception')
self.assertEqual(
notification.failure_reason,
'You must either provide a sender address explicitly or configure using the combination of `mail.catchall.domain` and `mail.default.from` ICPs, in the server configuration file or with the --email-from startup parameter.')
self.assertEqual(notification.failure_type, 'mail_from_missing')
self.assertEqual(notification.notification_status, 'exception')
# MailServer.send_email(): _prepare_email_message: unexpected ASCII
# MailServer.send_email(): _prepare_email_message: unexpected ASCII / Malformed 'Return-Path' or 'From' address
# Force catchall domain to void otherwise bounce is set to postmaster-odoo@domain
self.env['ir.config_parameter'].set_param('mail.catchall.domain', '')
self._reset_data()
mail.write({'email_from': 'strange@example¢¡.com'})
with self.mock_mail_gateway():
mail.send(raise_exception=False)
self.assertEqual(self._mails[0]['email_from'], 'strange@example¢¡.com')
self.assertEqual(mail.failure_reason, "Malformed 'Return-Path' or 'From' address: strange@example¢¡.com - It should contain one valid plain ASCII email")
self.assertFalse(mail.failure_type, 'Mail: bugged from (ascii): no failure type, should be updated')
self.assertEqual(mail.state, 'exception')
self.assertEqual(notification.failure_reason, "Malformed 'Return-Path' or 'From' address: strange@example¢¡.com - It should contain one valid plain ASCII email")
self.assertEqual(notification.failure_type, 'unknown', 'Mail: bugged from (ascii): unknown failure type, should be updated')
self.assertEqual(notification.notification_status, 'exception')
for email_from in ['strange@example¢¡.com', 'robert']:
self._reset_data()
mail.write({'email_from': email_from})
with self.mock_mail_gateway():
mail.send(raise_exception=False)
self.assertEqual(self._mails[0]['email_from'], email_from)
self.assertEqual(mail.failure_reason, f"Malformed 'Return-Path' or 'From' address: {email_from} - It should contain one valid plain ASCII email")
self.assertEqual(mail.failure_type, 'mail_from_invalid')
self.assertEqual(mail.state, 'exception')
self.assertEqual(notification.failure_reason, f"Malformed 'Return-Path' or 'From' address: {email_from} - It should contain one valid plain ASCII email")
self.assertEqual(notification.failure_type, 'mail_from_invalid')
self.assertEqual(notification.notification_status, 'exception')
# MailServer.send_email(): _prepare_email_message: unexpected ASCII based on catchall domain
# MailServer.send_email(): _prepare_email_message: unexpected ASCII based on catchall domain in bounce
self.env['ir.config_parameter'].set_param('mail.catchall.domain', 'domain¢¡.com')
self._reset_data()
mail.write({'email_from': 'test.user@example.com'})
@@ -332,24 +334,10 @@ class TestMailMail(MailCommon):
mail.send(raise_exception=False)
self.assertEqual(self._mails[0]['email_from'], 'test.user@example.com')
self.assertIn("Malformed 'Return-Path' or 'From' address: bounce.test@domain¢¡.com", mail.failure_reason)
self.assertFalse(mail.failure_type, 'Mail: bugged catchall domain (ascii): no failure type, should be updated')
self.assertEqual(mail.failure_type, 'mail_from_invalid')
self.assertEqual(mail.state, 'exception')
self.assertEqual(notification.failure_reason, "Malformed 'Return-Path' or 'From' address: bounce.test@domain¢¡.com - It should contain one valid plain ASCII email")
self.assertEqual(notification.failure_type, 'unknown', 'Mail: bugged catchall domain (ascii): unknown failure type, should be updated')
self.assertEqual(notification.notification_status, 'exception')
# MailServer.send_email(): _prepare_email_message: Malformed 'Return-Path' or 'From' address
self.env['ir.config_parameter'].set_param('mail.catchall.domain', '')
self._reset_data()
mail.write({'email_from': 'robert'})
with self.mock_mail_gateway():
mail.send(raise_exception=False)
self.assertEqual(self._mails[0]['email_from'], 'robert')
self.assertEqual(mail.failure_reason, "Malformed 'Return-Path' or 'From' address: robert - It should contain one valid plain ASCII email")
self.assertFalse(mail.failure_type, 'Mail: bugged from (ascii): no failure type, should be updated')
self.assertEqual(mail.state, 'exception')
self.assertEqual(notification.failure_reason, "Malformed 'Return-Path' or 'From' address: robert - It should contain one valid plain ASCII email")
self.assertEqual(notification.failure_type, 'unknown', 'Mail: bugged from (ascii): unknown failure type, should be updated')
self.assertEqual(notification.failure_type, 'mail_from_invalid')
self.assertEqual(notification.notification_status, 'exception')
@mute_logger('odoo.addons.mail.models.mail_mail')
@@ -377,8 +365,10 @@ class TestMailMail(MailCommon):
self.assertEqual(notification.notification_status, 'sent', 'Mail: missing email_to: notification is wrongly set as sent')
# MailServer.send_email(): _prepare_email_message: invalid To
for email_to, failure_type in zip(self.emails_invalid,
['mail_email_missing', 'mail_email_missing']):
for email_to, failure_type in zip(
self.emails_invalid,
['mail_email_missing', 'mail_email_missing']
):
self._reset_data()
mail.write({'email_to': email_to})
with self.mock_mail_gateway():
@@ -642,7 +632,7 @@ class TestMailMail(MailCommon):
self._reset_data()
mail.send(raise_exception=False)
self.assertEqual(mail.failure_reason, msg)
self.assertFalse(mail.failure_type, 'Mail: unlogged failure type to fix')
self.assertEqual(mail.failure_type, 'unknown', 'Mail: unlogged failure type to fix')
self.assertEqual(mail.state, 'exception')
self.assertEqual(notification.failure_reason, msg)
self.assertEqual(notification.failure_type, 'unknown', 'Mail: generic failure type')
odoo/addons/base/models/ir_mail_server.py
+14 -8
View File
@@ -91,6 +91,12 @@ class IrMailServer(models.Model):
NO_VALID_RECIPIENT = ("At least one valid recipient address should be "
"specified for outgoing emails (To/Cc/Bcc)")
NO_FOUND_FROM = ("You must either provide a sender address explicitly or configure "
"using the combination of `mail.catchall.domain` and `mail.default.from` "
"ICPs, in the server configuration file or with the --email-from startup "
"parameter.")
NO_FOUND_SMTP_FROM = "The Return-Path or From header is required for any outbound email"
NO_VALID_FROM = "Malformed 'Return-Path' or 'From' address. It should contain one valid plain ASCII email"
name = fields.Char(string='Name', required=True, index=True)
from_filter = fields.Char(
@@ -472,10 +478,7 @@ class IrMailServer(models.Model):
:return: the new RFC2822 email message
"""
email_from = email_from or self._get_default_from_address()
assert email_from, "You must either provide a sender address explicitly or configure "\
"using the combination of `mail.catchall.domain` and `mail.default.from` "\
"ICPs, in the server configuration file or with the "\
"--email-from startup parameter."
assert email_from, self.NO_FOUND_FROM
headers = headers or {} # need valid dict later
email_cc = email_cc or []
@@ -556,7 +559,7 @@ class IrMailServer(models.Model):
# Path (VERP) to detect no-longer valid email addresses.
bounce_address = message['Return-Path'] or self._get_default_bounce_address() or message['From']
smtp_from = message['From'] or bounce_address
assert smtp_from, "The Return-Path or From header is required for any outbound email"
assert smtp_from, self.NO_FOUND_SMTP_FROM
email_to = message['To']
email_cc = message['Cc']
@@ -599,9 +602,12 @@ class IrMailServer(models.Model):
# The email's "Envelope From" (Return-Path) must only contain ASCII characters.
smtp_from_rfc2822 = extract_rfc2822_addresses(smtp_from)
assert smtp_from_rfc2822, (
f"Malformed 'Return-Path' or 'From' address: {smtp_from} - "
"It should contain one valid plain ASCII email")
if not smtp_from_rfc2822:
raise AssertionError(
self.NO_VALID_FROM,
f"Malformed 'Return-Path' or 'From' address: {smtp_from} - "
"It should contain one valid plain ASCII email"
)
smtp_from = smtp_from_rfc2822[-1]
return smtp_from, smtp_to_list, message