Call to a member function has() on null in Drupal\Core\Mail\Plugin\Mail\PhpMail->mail()

-    if (!$this->request->server->has('WINDIR') && !str_contains($this->request->server->get('SERVER_SOFTWARE'), 'Win32')) {
+    if (!\Drupal::request()->server->has('WINDIR') && !str_contains(\Drupal::request()->server->get('SERVER_SOFTWARE'), 'Win32')) {

I don't see how this can possibly do anything. The constructor already sets
$this->request = \Drupal::request();

Your patch is basically a regression of some of the work done in #3226117: Uncaught RfcComplianceException when email From name contains a comma.

HIi _renify_

To change the services with Global Service not good practice , i tried to replicate using a custom call but not got getting the mentioned error, would be great if you an provide more info the process you followed.

Thanks

#2 Patch worked for me. Thanks

@linkanp: Please provide directions for reproducing this error.

The steps in the issue summary do not cause an error for me or for the poster in #6.

@TR I'll try to boil down the steps to reproduce but currently this is happening using the following modules:

1. Drupal 10.1 (of course) just upgraded from 9.5
2. monolog
3. mimemail and mailsystem
4. Our custom integration module https://github.com/ubc-cpsc/ubccs_monolog (but doesn't show in callstack so probably not part of it)

To reproduce it we have an error in our code and it is going to email it to us with monolog -> mimemail
Callstack:

Error: Call to a member function has() on null in Drupal\Core\Mail\Plugin\Mail\PhpMail->mail() (line 114 of /var/www/html/public/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php).

Drupal\Core\Mail\Plugin\Mail\PhpMail->mail(Array) (Line: 50)
Drupal\mailsystem\Adapter->mail(Array) (Line: 307)
Drupal\Core\Mail\MailManager->doMail('monolog', 'default', 'REDACTED@example.org', 'English', Array, NULL, 1) (Line: 180)
Drupal\Core\Mail\MailManager->Drupal\Core\Mail\{closure}() (Line: 592)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 181)
Drupal\Core\Mail\MailManager->mail('monolog', 'default', 'REDACTED@example.org', 'English', Array, NULL, 1) (Line: 70)
Drupal\mailsystem\MailsystemManager->mail('monolog', 'default', 'REDACTED@example.org', 'English', Array) (Line: 58)
Drupal\monolog\Logger\Handler\DrupalMailHandler->send('WARNING
Message:
Warning: Attempt to read property "server" on null in Drupal\Core\Mail\Plugin\Mail\PhpMail->mail() (line 114 of /var/www/html/public/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php) #0 /var/www/html/public/core/includes/bootstrap.inc(164): _drupal_error_handler_real(2, 'Attempt to read...', '/var/www/html/p...', 114)
#1 /var/www/html/public/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php(114): _drupal_error_handler(2, 'Attempt to read...', '/var/www/html/p...', 114)
#2 /var/www/html/public/modules/contrib/mailsystem/src/Adapter.php(50): Drupal\Core\Mail\Plugin\Mail\PhpMail->mail(Array)
#3 /var/www/html/public/core/lib/Drupal/Core/Mail/MailManager.php(307): Drupal\mailsystem\Adapter->mail(Array)
#4 /var/www/html/public/core/lib/Drupal/Core/Mail/MailManager.php(180): Drupal\Core\Mail\MailManager->doMail('monolog', 'default', 'REDACTED@example.org', 'English', Array, NULL, true)
#5 /var/www/html/public/core/lib/Drupal/Core/Render/Renderer.php(592): Drupal\Core\Mail\MailManager->Drupal\Core\Mail\{closure}()
#6 /var/www/html/public/core/lib/Drupal/Core/Mail/MailManager.php(181): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#7 /var/www/html/public/modules/contrib/mailsystem/src/MailsystemManager.php(70): Drupal\Core\Mail\MailManager->mail('monolog', 'default', 'REDACTED@example.org', 'English', Array, NULL, true)
#8 /var/www/html/public/modules/contrib/monolog/src/Logger/Handler/DrupalMailHandler.php(58): Drupal\mailsystem\MailsystemManager->mail('monolog', 'default', 'REDACTED@example.org', 'English', Array)
#9 /var/www/html/vendor/monolog/monolog/src/Monolog/Handler/MailHandler.php(61): Drupal\monolog\Logger\Handler\DrupalMailHandler->send('<h1 style="back...', Array)
#10 /var/www/html/vendor/monolog/monolog/src/Monolog/Handler/AbstractProcessingHandler.php(44): Monolog\Handler\MailHandler->write(Object(Monolog\LogRecord))
#11 /var/www/html/vendor/monolog/monolog/src/Monolog/Logger.php(379): Monolog\Handler\AbstractProcessingHandler->handle(Object(Monolog\LogRecord))
#12 /var/www/html/vendor/monolog/monolog/src/Monolog/Logger.php(567): Monolog\Logger->addRecord(Object(Monolog\Level), '%type: @message...', Array)
#13 /var/www/html/public/modules/contrib/monolog/src/Logger/LoggerInterfacesAdapter.php(134): Monolog\Logger->log(Object(Monolog\Level), '%type: @message...', Array)
#14 /var/www/html/public/core/lib/Drupal/Core/EventSubscriber/ExceptionLoggingSubscriber.php(70): Drupal\monolog\Logger\LoggerInterfacesAdapter->log(3, '%type: @message...', Array)
#15 /var/www/html/public/core/lib/Drupal/Core/EventSubscriber/ExceptionLoggingSubscriber.php(97): Drupal\Core\EventSubscriber\ExceptionLoggingSubscriber->onError(Object(Symfony\Component\HttpKernel\Event\ExceptionEvent))
#16 [internal function]: Drupal\Core\EventSubscriber\ExceptionLoggingSubscriber->onException(Object(Symfony\Component\HttpKernel\Event\ExceptionEvent), 'kernel.exceptio...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#17 /var/www/html/public/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\ExceptionEvent), 'kernel.exceptio...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#18 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(240): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\ExceptionEvent), 'kernel.exceptio...')
#19 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(91): Symfony\Component\HttpKernel\HttpKernel->handleThrowable(Object(Drupal\Core\Entity\Query\QueryException), Object(Symfony\Component\HttpFoundation\Request), 1)
#20 /var/www/html/public/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /var/www/html/public/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /var/www/html/public/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /var/www/html/public/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /var/www/html/public/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /var/www/html/public/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /var/www/html/public/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /var/www/html/public/core/lib/Drupal/Core/DrupalKernel.php(704): Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /var/www/html/public/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#29 {main}.

Don't think there is anything to review, Will need a test case showing this issue in core though.

Also don't think we should be undoing the dependency injection.

I tried to set a breakpoint in the constructor, but it never hit that breakpoint there for some reason... which explain why it's not set, but I'm confused as to why/how/when it can be created without calling that.

I'm using DDEV btw, restarted it, and rebuilt cache to try to get it to break there.

@smustgrave I agree with your assessment in #10 in all counts, thanks for chiming in.

That error seems to come from $this->request being NULL in PhpMail when it tries to get $this->request->server

The request is obtained in the PhpMail constructor:

   /**
     * PhpMail constructor.
     */
    public function __construct() {
        $this->configFactory = \Drupal::configFactory();
        $this->request = \Drupal::request();
    }

That tells me that perhaps this is being called in a non-request context?

Regardless, the problem is the lack of a request object. I still don't see how the patch in #2 can be doing anything at all if this is a problem with the global \Drupal::request() not containing a request object.

I guess the first thing I would do here is to change PhpMail to implement ContainerFactoryPluginInterface and do the injection the normal way, rather than using \Drupal::request() in the constructor (was there some reason for that in the first place?) And in fact, I believe current best practice is to inject the request stack instead. See https://mglaman.dev/blog/dependency-injection-anti-patterns-drupal

Still, this needs a test case - it's not clear why it's failing for you, and the instructions to reproduce in the issue summary aren't sufficient to narrow down the cause.

Note that \Drupal::request() was added to PhpMail pretty recently, in #3226117: Uncaught RfcComplianceException when email From name contains a comma, which may be why this problem is just showing up now. I don't see any discussion in that issue about injection, and the test case added in that issue didn't reveal any problems. That's why it would be important to have a test case here which could demonstrate the problem and prove the solution.

@TR I'm tracking it down (slowly) but I think mailsystem is creating an instance before it has the request instance.

I agree with that idea of implementing ContainerFactoryPluginInterface, good idea!

@TR, I see the problem for me (might be the same for the OP), I'll leave this for @_renify_

The problem was that mimemail overwrote the constructor
https://git.drupalcode.org/project/mimemail/-/blob/8.x-1.0-alpha5/src/Pl...

But it has been fixed already in the dev branch
https://git.drupalcode.org/project/mimemail/-/blob/8.x-1.x/src/Plugin/Ma...
@TR since you are a maintainer maybe you'd consider cutting an alpha6?

I'll move the the dev branch in the meantime.

Adding the related issue it was fixed in mimemail, thanks @TR for committing it!

MimeMail now has a new release. Thanks for that suggestion.

Still not sure if @_renify_ or @linkanp were using MimeMail or if they were encountering some other problem.

I also still think it's a good idea to do the injection properly in the PhpMail constructor rather than just use the global function. The only reason MimeMail had to be fixed was because MimeMail plugin needed the injected service and had to override the parent PhpMail class which wasn't doing injection properly.

Thanks @TR for getting that release out! I hope that fixes for the others as well!

After facing this same issue on my project, i fall into this old thread and i just updated the steps to reproduce.

Looks like the original problem of @_renify_ was related with the mimemail module and the fix pointed by @joelpittet and @TR resolves the error.

But in my case i wasn't using the mimemail, insted, i faced the issue using the moderation_note.

As following the same trail they left here, i checked the moderation_node module and also a very similar code gap was causing the error.

So, after adding the patch into moderation_note module (just added the related issue to this thread as well), the error is gone.

Maybe at this point is enough to say that the issue its not in the core.
But i also agree that it's a good idea to do the injection properly in the PhpMail constructor, rather than just use the global function.

Should be the case to close this one as a bug report, and open a feature request mentioning this thread?

Don’t see the test coverage

Hi @smustgrave!
Thanks for your quick reply.

I can see what you said on #10: "Will need a test case showing this issue in core though".

But i raised the idea on my last comment #20, that maybe we'll never find the way to show this issue on core.
As both scenarios we reproduce here was related to 3rd contrib modules: mimemail and moderated_note

Also, the patch #2 attached have some cons pointed by #6.

So maybe we have nothing to test here, and instead review if this original issue its a bug or can be replaced as a feature request.

Do you agree?

If the bug can't be reproduced in core then, to me, sounds like the contrib modules could be the bug.

Yes, i agree with you.

Both scenarios we found here should be fixed inside cobtrib modules issues, not here.

Thats why i suggest to close this Core Bug Issue and open a Core Feature request, as at least the thread serves to pointed out some core-code improvements.

As suggested, I'm closing this issue. There is an existing issue for converting calls to \Drupal::request() to container injection. So I'm considering this to be a duplicate.

Credit has been granted to the contributors who worked on the issue.