Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry-able emails should not include ones explicitly cancelled by plugin/modules #1842

Closed
bossanova808 opened this issue Nov 3, 2020 · 4 comments
Assignees
Labels

Comments

@bossanova808
Copy link
Contributor

Description

We've started seeing a bunch of failed queue entries. The (erroneous) error reported is:

Email “Service Order to Ticket”, for order "cf5279c" was cancelled by plugin.

was cancelled by plugin is the important thing here...this is not an error, but a deliberate cancellation of the email. It's not the same as e.g. a template error in an email, and it should not be reported as an error at all.

To Reproduce

Have module code that cancels an email given some condition

Expected behaviour

Email should be silently cancelled (entry in a log file is ok, but an alarming looking failure message in the CP is over-doing it...)

Additional info

composer update-d all the things just now...

@pdaleramirez pdaleramirez added the 🔎 status: investigating Trying to reproduce label Nov 3, 2020
@pdaleramirez
Copy link
Contributor

@bossanova808
The cause of this issue is that the event Emails::EVENT_BEFORE_SEND_MAIL got triggered and set to cancel an email. I would recommend scan all installed plugins and modules and check how it is being used. It should looks something like this

Event::on(
    Emails::class,
    Emails::EVENT_BEFORE_SEND_MAIL,
    function (MailEvent $event) {

        // Cancel email
        $event->isValid = false;
    }
);

@pdaleramirez pdaleramirez self-assigned this Nov 3, 2020
@bossanova808
Copy link
Contributor Author

@pdaleramirez Thanks - & yep I know what the cause is, as I wrote the module that does precisely that.

My point is - it's not an error, it's a deliberate use of a system to suppress emails, and until the very latest Commerce update, did not result in failure reports in the queue manager...and it should still not result in them....

@lukeholder
Copy link
Member

@bossanova808 Fixed this for the next release. Plugins that use the event to stop the email being sent will no longer be shown as failed on the queue, just silently not sent. An error will still be logged that the email was prevented from being sent by a plugin.

@bossanova808
Copy link
Contributor Author

Great, thanks Luke!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants