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

[8.x] Fix failover mailer when used with Mailgun & SES mailers #40254

Merged
merged 1 commit into from
Jan 5, 2022
Merged

[8.x] Fix failover mailer when used with Mailgun & SES mailers #40254

merged 1 commit into from
Jan 5, 2022

Conversation

gbuckingham89
Copy link
Contributor

@gbuckingham89 gbuckingham89 commented Jan 4, 2022

Description

#38344 introduced the failover mailer into the framework, with the idea being that if a mail fails to send via one mailer, alternative mailer(s) will be tried.

It seems that the first-party HTTP based mailers / transport included with the framework (Mailgun & SES) don't trigger an attempt to send using an alternative mailer if the API request fails - the exception from the underlying library isn't caught (a GuzzleException for Mailgun and AwsException for SES).

The issue seems to be that the send() method in the underlying \Swift_Transport_FailoverTransport class only catches Swift_TransportException exceptions, which trigger the failover logic. Other exceptions aren't handed.

Steps To Reproduce (with Mailgun):

  1. Setup a failover mailer in config/mail.php:
'failover' => [
    'transport' => 'failover',
    'mailers' => [
        'mailgun',
        'log',
    ],
],
  1. Ensure your attempt to send via Mailgun will fail (e.g. use incorrect credentials / block requests to the API domain on your local network).

  2. Attempt to send a mail - you'll get aClientException exception, rather than the alternative failover log mailer being used.

Solution

The upstream swiftmailer/swiftamiler package is no longer maintained, so we can't suggest a PR to catch other exceptions in the failover logic. Plus this feels more like an issue with the Laravel transport classes not throwing the correct exceptions.

This PR simply catches any \GuzzleHttp\Exception\GuzzleException (Mailgun) / \Aws\Exception\AwsException (SES) exceptions and then throws a new Swift_TransportException instead (with the original passed in via $previous).

I haven't added any tests as there don't appear to be any for the MailgunTransport class, and wasn't sure of the best way of mocking a failure for SES - however I'm happy to add tests if anyone can assist!

@gbuckingham89 gbuckingham89 changed the title Fix failover mailer when used with Mailgun & SES mailers [8.x] Fix failover mailer when used with Mailgun & SES mailers Jan 4, 2022
@taylorotwell taylorotwell merged commit 1563361 into laravel:8.x Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants