Parent constructor of MimeMail plugin changed its signature

The RequestStack parameter was added to the PhpMail constructor in Drupal 10.1.x (and ONLY in Drupal 10.1.x) by the commit in #3226117: Uncaught RfcComplianceException when email From name contains a comma.

Setting $this->request like you do in this patch will not work with core versions < 10.1.x because core PhpMail does not define the request property and because PHP 8 no longer allows dynamic properties.

The patch needs to work with all supported versions of Drupal core.

The automated tests of Mime Mail don't fail in Drupal 10.1. What are the conditions that cause this error message? We need a new test case that fails without your patch to prevent this from breaking in the future.

Making this a "task" because it is something we need to do to keep up with the development pre-release of Drupal 10.1.

What are the conditions that cause this error message?

I'm able to reproduce the above warning and error by following these steps:

1. Install Drupal 10.1.0
2. Install and enable mimemail 1.0-alpha5 + mailsystem 4.4.0
3. On /admin/config/system/mailsystem, set the default sender to Mime Mail
4. As an anonymous user, go to /user/password and request a password-reset email — ❌ Error: Call to a member function has() on null

In #3226117: Uncaught RfcComplianceException when email From name contains a comma, Drupal Core added the PhpMail::$request instance variable which gets initialized by PhpMail::__construct(). Since the MimeMail class derives from PhpMail but MimeMail::__construct() doesn't call parent::__construct();, the PhpMail::$request instance variable remains uninitialized, resulting in the above warning and error.

The automated tests of Mime Mail don't fail in Drupal 10.1.

I think that's because there isn't a test case for sending using the MimeMail class. I've attached a test that should fail.

And here's a patch that addresses the problem by invoking the parent constructor.

https://www.drupal.org/pift-ci-job/2701615 correctly failed with the Attempt to read property "server" on null error.

However, https://www.drupal.org/pift-ci-job/2701616 failed with PHPUnit\Framework\Exception: sh: 1: /usr/sbin/sendmail: not found. Unfortunately, the PhpMail::mail() method doesn't seem to be testable, since it invokes PhpMail::doMail() which invokes PHP's built-in mail() function, without any way to override it with a mock for testing.

Subscribing, same problem here.

@Praveen Saini: Are you working on this? If not, please unassign yourself from this issue.

@smokris: The unit test added in #3226117: Uncaught RfcComplianceException when email From name contains a comma could be subclassed and modified to test MimeMail.php instead of PhpMail.php - that would also help when it comes to tracking changes in the PhpMail parent class, because a new subclassed test would in theory track changes in the parent class. Basically, all the test needs to do at this time is to ensure the MimeMail plugin can be instantiated without error - a test like this will provide early warning in the future if the parent class constructor gets changed again.

@TR, thanks for the tip. Here's a new patch that includes a test that's a subclass of PhpMailTest, and which uses PhpMailTest's technique of creating a mock of the class being tested but with the doMail() method replaced.

OK, let's see if the test in #11 will fail when run with the current version of Mime Mail. What I would like to see is a test that will catch the problem if the parent constructor gets changed, so the test alone should fail here, and the test + patch of MimeMail.php should succeed like in #11.

Since the test-only CI job failed on PHP 8.1 (as expected), and the test-and-fix CI job passed, I'll tentatively set this to RTBC.

+1 to the patch, works as expected.

Committed. Thanks!