diff --git a/addons/mail/models/mail_mail.py b/addons/mail/models/mail_mail.py index a8f76e6a83f..0c31e94428e 100644 --- a/addons/mail/models/mail_mail.py +++ b/addons/mail/models/mail_mail.py @@ -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): diff --git a/addons/mail/models/mail_message.py b/addons/mail/models/mail_message.py index 2160ca586d1..15af41e6d95 100644 --- a/addons/mail/models/mail_message.py +++ b/addons/mail/models/mail_message.py @@ -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' diff --git a/addons/mail/models/mail_notification.py b/addons/mail/models/mail_notification.py index 6c4497df79b..81c8f8c72a9 100644 --- a/addons/mail/models/mail_notification.py +++ b/addons/mail/models/mail_notification.py @@ -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) diff --git a/addons/mass_mailing/models/mailing_trace.py b/addons/mass_mailing/models/mailing_trace.py index cf0f2fd1380..e1a59010969 100644 --- a/addons/mass_mailing/models/mailing_trace.py +++ b/addons/mass_mailing/models/mailing_trace.py @@ -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"), diff --git a/addons/test_mail/tests/test_mail_mail.py b/addons/test_mail/tests/test_mail_mail.py index 9636dd68a2e..aee64987025 100644 --- a/addons/test_mail/tests/test_mail_mail.py +++ b/addons/test_mail/tests/test_mail_mail.py @@ -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') diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py index 70df04aadf5..fd088a21bb7 100644 --- a/odoo/addons/base/models/ir_mail_server.py +++ b/odoo/addons/base/models/ir_mail_server.py @@ -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