[19.0][18.0] mail_tracking: send_email() swallows every exception when a tracking record exists — emails are reported as sent when they were not

[skanndar]

Summary

IrMailServer.send_email() catches every exception and, when a mail.tracking.email record exists for the message, records the error on the tracking record and returns instead of re-raising — mail_tracking/models/ir_mail_server.py (lines 161-165 on 19.0, 159-163 on 18.0):

        except Exception as e:
            if tracking_email:
                tracking_email.smtp_error(self, smtp_server_used, e)
            else:
                raise

Odoo core tells a successful send from a failed one by the exception, not by the return value, so returning normally tells core "this was delivered". Since this module creates a tracking record for every outgoing email (mail_tracking/models/mail_mail.py::_prepare_outgoing_list) and injects the pixel that _build_email__ reads back as X-Odoo-MailTracking-ID, tracking_email is truthy for every normal HTML email: the else: raise branch is effectively dead, and core's failure handling is disabled database-wide.

The visible consequence: emails that were never delivered are reported to users as delivered.

Measured behaviour

Odoo 19.0 CE (19.0-20260723, official image), fresh database, the SMTP session replaced by a stub so that valid recipients are really delivered and can be counted. Two partners are notified through the chatter (message_post, mail.mt_comment): one with an email address, one without — the case for which core raises AssertionError(NO_VALID_RECIPIENT).

Same scenario, run on a database with mail_tracking installed and on a control database with core only:

mail.notification of the recipient without email	with mail_tracking	control (core only)
notification_status	sent	exception
failure_type	not set	mail_email_invalid

The valid recipient is delivered in both cases (a single SMTP envelope, ['valid@example.com']) — so the module does not stop delivery here, it misreports it. Both orderings of the recipients give the same result.

Second scenario, mail.mail with auto_delete=False and the broken recipient last in email_list (verified by printing the list, not assumed):

mail.mail after sending	with mail_tracking	control (core only)
state	exception	sent
failure_type	unknown	—
failure_reason	"Error without exception. Probably due to sending an email without computed recipients."	—
valid recipient delivered?	yes	yes

So the same swallowed exception produces two opposite errors depending on position: a failed recipient in the middle is reported as delivered, and a failed recipient at the end marks the whole mail as failed even though everybody else received it.

Why (core's loop)

odoo/addons/mail/models/mail_mail.py::_send():

res = None
...
for email in email_list:
    try:
        res = SendIrMailServer.send_email(msg, ...)
        if processing_pid:
            success_pids.append(processing_pid)
    except AssertionError as error:
        if str(error) == IrMailServer.NO_VALID_RECIPIENT:
            ...
            failure_type = "mail_email_invalid"
if res:  # mail has been sent at least once, no major exception occurred

1. except AssertionError never runs → a recipient without a valid address is never classified as mail_email_invalid.
2. The recipient is appended to success_pids as soon as send_email returns, whatever it returns; _postprocess_sent_message() then writes notification_status = 'sent' for a message that was never delivered.
3. res is overwritten on every iteration and the verdict is if res:, so a failure on the last recipient marks the whole mail as an exception.

The same asymmetry applies to any hard SMTP failure, not only to missing addresses.

Note that the module already anticipates this exact exception: MailTrackingEmail.smtp_error() has a dedicated branch for str(exception) == IrMailServer.NO_VALID_RECIPIENT setting error_type = "no_recipient". The tracking record is annotated correctly — only core is left in the dark.

Proposed fix

Always re-raise. The error is still recorded on the tracking record by the line above, and control returns to core, which can then classify the failure, keep delivering to the remaining recipients when it is a NO_VALID_RECIPIENT, and mark the mail for what it actually is.

         except Exception as e:
             if tracking_email:
                 tracking_email.smtp_error(self, smtp_server_used, e)
-            else:
-                raise
+            # Always re-raise: MailMail._send() tells success from failure by
+            # the exception, not by the return value. Returning here makes core
+            # mark the notification as 'sent' for a message that was never
+            # delivered, prevents NO_VALID_RECIPIENT from ever being classified
+            # as 'mail_email_invalid', and leaves `res` False so a failure on
+            # the last recipient marks the whole mail as an exception.
+            # The error is already recorded on the tracking record above.
+            raise

Please read before merging

This is a behaviour change with a visible consequence: failures that used to be silent will start surfacing as exceptions. On the production database where we hit this, applying the fix immediately turned a backlog of previously hidden delivery failures into visible exception states — the tracking records had been sitting in error state all along, they were simply never propagated to the mails and notifications. That is the correct outcome, but it will look like a regression to anyone upgrading without warning, so it deserves a line in the migration notes.

Versions: 19.0.1.0.5 and 18.0.1.0.11 carry identical code. We are running this patch in production and are happy to open it as a PR if the approach is agreed.