[CRITICAL] Email success is messured fundamentaly wrong by using wp_mail incorrectly

[calvinalkan]

Inside Handler.php you have the sendEmails() method.

This has the following main for each loop.

 foreach ($campaignEmails as $email) {
            if ($this->reachedEmailLimitPerSecond()) {
                $this->restartWhenOneSecondExceeds();
            } else {
                $response = Mailer::send($email->data());
                $this->dispatchedWithinOneSecond++;
                if (is_wp_error($response)) {
                    $failedIds[] = $email->id;
                } else {
                    CampaignEmail::where('id', $email->id)->whereNot('status', 'failed')->update([
                        'status'     => 'sent',
                        'updated_at' => current_time('mysql')
                    ]);
                    $sentIds[] = $email->id;
                }
            }
        }

Now lets look what Mailer::send() returns.

 public static function send($data)
    {
        $headers = static::buildHeaders($data);

        if (apply_filters('fluentcrm_is_simulated_mail', false, $data, $headers)) {
            return true;
        }

        return wp_mail(
            $data['to']['email'],
            $data['subject'],
            $data['body'],
            $headers
        );
    }

wp_mail() by contract RETURNS A BOOLEAN

That means that:

 $response = Mailer::send($email->data());
                $this->dispatchedWithinOneSecond++;
                if (is_wp_error($response)) {
                    $failedIds[] = $email->id;
                } 

WILL ALWAYS MARK THE EMAIL AS SUCCESSFULL

Its not possible that this function returns a WP_ERROR class. It returns booleans.

	/**
	 * Sends an email, similar to PHP's mail function.
	 *
	 * A true return value does not automatically mean that the user received the
	 * email successfully. It just only means that the method used was able to
	 * process the request without any errors.
	 *
	 * The default content type is `text/plain` which does not allow using HTML.
	 * However, you can set the content type of the email by using the
	 * {@see 'wp_mail_content_type'} filter.
	 *
	 * The default charset is based on the charset used on the blog. The charset can
	 * be set using the {@see 'wp_mail_charset'} filter.
	 *
	 * @since 1.2.1
	 * @since 5.5.0 is_email() is used for email validation,
	 *              instead of PHPMailer's default validator.
	 *
	 * @global PHPMailer\PHPMailer\PHPMailer $phpmailer
	 *
	 * @param string|string[] $to          Array or comma-separated list of email addresses to send message.
	 * @param string          $subject     Email subject.
	 * @param string          $message     Message contents.
	 * @param string|string[] $headers     Optional. Additional headers.
	 * @param string|string[] $attachments Optional. Paths to files to attach.
	 * @return bool Whether the email was sent successfully.
	 */
	function wp_mail( $to, $subject, $message, $headers = '', $attachments = array() ) {
	
	// code
	
	}

[techjewel]

Different SMTP plugins handle email sending differently. Even if it returns as true then it does not mean the email was sent.
Some plugins actually queue the mails first and send them later.

So in the WP environment, it's quite impossible to know if the email actually sent or not. If you use FluentSMTP or Amazon SES and set up SNS then it sent a notification if an email failed to send.

Mailer::send() 's job is to deliver the emails to wp_mail() and nothing else. Then we listen for SNS or webhook for other SMTP services to mark as bounced.

[calvinalkan]

@techjewel yes it doesnt reflect mail sucess per se but that wp_mail worked as intended. In fact if a SMTP plugin does not return either bool(true) or bool(false) for wp_mail it should be uninstalled immediately as it violates the contract provided by WordPress core.

FluentSMTP returns only boolean true/false so whats the point of checking for WP_Error.

Be so kind and make the following test. Install the lastet version of FluentCRM, and create a contact. Now before you send him an email go into wp_mail and just add this

	/**
	 * Sends an email, similar to PHP's mail function.
	 *
	 * A true return value does not automatically mean that the user received the
	 * email successfully. It just only means that the method used was able to
	 * process the request without any errors.
	 *
	 * The default content type is `text/plain` which does not allow using HTML.
	 * However, you can set the content type of the email by using the
	 * {@see 'wp_mail_content_type'} filter.
	 *
	 * The default charset is based on the charset used on the blog. The charset can
	 * be set using the {@see 'wp_mail_charset'} filter.
	 *
	 * @since 1.2.1
	 * @since 5.5.0 is_email() is used for email validation,
	 *              instead of PHPMailer's default validator.
	 *
	 * @global PHPMailer\PHPMailer\PHPMailer $phpmailer
	 *
	 * @param string|string[] $to          Array or comma-separated list of email addresses to send message.
	 * @param string          $subject     Email subject.
	 * @param string          $message     Message contents.
	 * @param string|string[] $headers     Optional. Additional headers.
	 * @param string|string[] $attachments Optional. Paths to files to attach.
	 * @return bool Whether the email was sent successfully.
	 */
	function wp_mail( $to, $subject, $message, $headers = '', $attachments = array() ) {
	
	return false;
	// code
	
	}

That email will never ever get send but is marked as successful in the UI including a flash massage. This is super misleading for users that don't know whats going on.

While bool(true) cant guarantee that the email was sent successfully bool(false) almost 100% percent guarantees that it did not work.

Relying on the handleFailedLog function is a bad solution. What happens if an uncaught exception gets thrown? That hook will never fire.

A more reasonable approach would be the following.

 foreach ($campaignEmails as $email) {
            if ($this->reachedEmailLimitPerSecond()) {
                $this->restartWhenOneSecondExceeds();
            } else {
                try {
    
                    $mail_sent_successfully = Mailer::send($email->data());
                    $this->dispatchedWithinOneSecond++;
                    
                }catch (\Throwable $e ) {
                    
                    error_log($e->getMessage() . 'TRACE:' . $e->getTraceAsString());
                    $mail_sent_successfully = false;
                }
               
                if ( ! $mail_sent_successfully) {
                    $failedIds[] = $email->id;
                } else {
                    CampaignEmail::where('id', $email->id)->whereNot('status', 'failed')->update([
                        'status'     => 'sent',
                        'updated_at' => current_time('mysql')
                    ]);
                    $sentIds[] = $email->id;
                }
            }
        }