SMTPMailSystem::mail throws exception "Invalid address: (to)" instead of returning FALSE

Hi

I get the same issue when using content_moderation_notifications to send the email on a workflow transition.

It'll only send email to the site email address, and attempt to send to additional emails address we get this same error.

Drupal\Core\Entity\EntityStorageException: Invalid address: (to): in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 846 of /var/www/html/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

and

Error sending email (from site@emailaddress.com to with reply-to not set).

I think the TO/REPLY-TO format that content_moderation_notifications sends to the SMTP module is the cause, but not sure on a fix.

@Adamation - relating to the "Invalid address" issue when using content_moderation_notification, this patch solved our situation.

The problem occurs when selecting to disable the site email address notification. Hope it helps your case.

We also experienced this exception , however when user account was blocked/unblocked , notification for those operations enabled but the account didn't had email set.

According to MailInterface::mail() :

Returns TRUE if the mail was successfully accepted for delivery, otherwise FALSE.

SMTPMailSystem::mail() breaks the contract and @throws \PHPMailer\PHPMailer\Exception instead.

Attached patch wraps most of the SMTPMailSystem::mail() body into a separate function to catch PHPMailer exceptions. I tried to minimise the change to simplify review.

In this patch:

• Update tests to account for new structure
• Revert unintended switch from list() => []

Found an issue in the patch - wrong parameter of the $message used in exception logger.

Re-roll for the latest dev - patch stopped applying after version 1.1.

Fixed tests.

FWIW interdiff between 9 and 10 is not representative - visual patch file comparison gives way better picture than what is generated by interdiff.

Moved patch from #12 to MR.

While patch applies cleanly to 8.x-1.3 and all tests on the MR passed, manually testing found a error as 8.x-1.3 introduced additional code that expected logger to be a factory instead of the logger object.

Rather than update the code in debug method I have reverted the change to the constructor logic. I think it could be considered outside the scope of this issue and in recent times keeping the factory as the dependency is preferred.

Made comments on the MR -- there are BC issues and I think we can use reflection instead of needing another public method.

Thanks for the review Jakob - I believe I have implemented all the changes requested.

Hi ericgsmith,

Thanks for the work, merge request 37 solved the problem for me.

Site Drupal core version 10.2.8, PHP version 8.1.

Greetings,

Fabio