inpsyde / Wonolog

Monolog-based logging package for WordPress.
https://inpsyde.github.io/Wonolog/
MIT License
172 stars 17 forks source link

fix: PHPMailer namespace in MailerListener #85

Open frugan-dev opened 3 weeks ago

frugan-dev commented 3 weeks ago

@see https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/PHPMailer/PHPMailer.php#L22

gmazzap commented 3 weeks ago

Thanks @frugan-dev

Do you know in which WP version the change happened?

Maybe we could do something like:

if (
  ($mailer instanceof \PHPMailer)
  || ($mailer instanceof \PHPMailer\PHPMailer\PHPMailer)
) {
}

To support both use cases. And it is really bncesary to replace 2 with PHPMailer\PHPMailer\SMTP::DEBUG_SERVER? I understand tthat is more "elegant" code, but if we plan to support both non-namespaced and namespaced classes, this could be problematic. And honestly, I'm not expecting PHPMailer in WordPress to ever change that constant.

chesio commented 3 weeks ago

Do you know in which WP version the change happened?

It seem to be WordPress 5.5, where PHPMailer has been updated to version 6: https://make.wordpress.org/core/2020/07/01/external-library-updates-in-wordpress-5-5-call-for-testing/

frugan-dev commented 3 weeks ago

Yes, I think we need a more gradual approach, that also supports WP versions prior to 5.5, but that always gradually aims to avoid magic numbers (see https://github.com/povils/phpmnd).

Also because in my opinion, it would be useful to be able to customize the debug level, perhaps with a constant like WONOLOG_PHPMAILER_SMTP_DEBUG_LEVEL. Just in these days I had to manually change it in $mailer->SMTPDebug = SMTP::DEBUG_LOWLEVEL inside the vendor folder to be able to temporarily raise the debug level..

For example something like this (namespace plus, namespace minus):

use PHPMailer\PHPMailer\PHPMailer;
use PHPMailer\PHPMailer\SMTP;

if ($mailer instanceof \PHPMailer || $mailer instanceof PHPMailer) {
    $mailer->SMTPDebug = \defined('WONOLOG_PHPMAILER_SMTP_DEBUG_LEVEL') ? \constant('WONOLOG_PHPMAILER_SMTP_DEBUG_LEVEL') : (class_exists('\PHPMailer\PHPMailer\SMTP') ? SMTP::DEBUG_SERVER : 2);
    $mailer->Debugoutput = static function (string $message) use ($updater): void {
        $updater->update(new Debug($message, Channels::HTTP));
    };
}
frugan-dev commented 3 weeks ago

Do you know in which WP version the change happened?

It seem to be WordPress 5.5, where PHPMailer has been updated to version 6: https://make.wordpress.org/core/2020/07/01/external-library-updates-in-wordpress-5-5-call-for-testing/

Yes I confirm! https://core.svn.wordpress.org/tags/5.4.9/wp-includes/class-phpmailer.php https://core.svn.wordpress.org/tags/5.5/wp-includes/PHPMailer/SMTP.php

gmazzap commented 2 weeks ago

Thanks for the investigation @frugan-dev

I'm not sure I want to add a constant for this. In version 2, most constant-based configurations were removed in favor of code-based configurations.

What about adding a constructor parameter to set the PHPMailer debug level?

This way, one could do the following:

add_action(
    'wonolog.setup',
    function (Inpsyde\Wonolog\Configurator $config) {
        $config
            ->disableDefaultHookListeners(MailerListener::class)
            ->addActionListener(new MailerListener(phpmailerDebugLevel: SMTP::DEBUG_LOWLEVEL))
    }
);
frugan-dev commented 2 weeks ago

Ok as you prefer, LGTM! On the user side, the important thing is to be able to customize the SMTP::DEBUG_* constant.