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

How to handle email delivery failures #403

Closed
denvaar opened this issue Jun 16, 2018 · 4 comments
Closed

How to handle email delivery failures #403

denvaar opened this issue Jun 16, 2018 · 4 comments
Labels

Comments

@denvaar
Copy link

denvaar commented Jun 16, 2018

I am using Bamboo to send account confirmation emails when someone creates an account. I'm using deliver_later to send emails in the background. In my application, an account is initially created as "inactive" until they use the confirmation link in the email.

I want to be able to handle the case where a fake email is used to create an account, because I don't want a bunch of fake stuff hanging around in the database. My application doesn't know it's fake until this exception is raised from within a different process:

(Bamboo.SMTPAdapter.SMTPError) There was a problem sending the email through SMTP

I looked at the docs and came up with a solution using a custom delivery strategy, but I am not sure if this type of thing is what it's intended for, so I wanted to get some validation, and check if this is a good thing to do. As you can see from my code below, I am basically just catching the exception and then deleting the fake user account from the database.

defmodule MyApp.AsyncEmailStrategey do
  @behaviour Bamboo.DeliverLaterStrategy

  alias MyApp.Accounts

  def deliver_later(adapter, email, config) do
    Task.async fn ->
      try do
        adapter.deliver(email, config)
      rescue
        error in Bamboo.SMTPAdapter.SMTPError ->
          remove_user_account(email)
          raise error
      end
    end
  end

  defp remove_user_account(email) do
    with [nil: bad_email] = email.to,
         user = Accounts.get_user_by_email(bad_email)
    do
      Accounts.delete_user(user)
    end
  end
end
@claytongentry
Copy link

Would like to bump this issue.

We have tested a similar try / rescue flow using Amazon SES Simulator inbox addresses and have found the adapter does not raise as we'd expect on deliveries to the faulty addresses, e.g. bounce@simulator.amazonses.com.

This makes it difficult to clean our lists of bouncing, suppressed or complaint emails.

@claytongentry
Copy link

claytongentry commented Oct 24, 2018

Hey @denvaar, don't know if you're still having this issue but if on the off-chance you happen to be using SES, the solution we ended up going with is subscribing to bounce and complaint notifications via SNS.

https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notifications-via-sns.html

@blacksph3re
Copy link

Still it would be nice having this for any smtp service which doesn't support separate bounce/complaint notifications

@germsvel
Copy link
Collaborator

I know it's been a while since this issue was opened. But I wanted to drop a note since there were some changes in Bamboo 2 that might be helpful.

With Bamboo 2.0.0, deliver_now doesn't raise errors on failure — and therefore adapters shouldn't raise errors on adapter.deliver. So I think having something like @denvaar originally posted makes sense, except now you don't have to catch an exception. You can use a case statement and pattern match on {:error, error} instead:

defmodule MyApp.AsyncEmailStrategey do
  @behaviour Bamboo.DeliverLaterStrategy

  alias MyApp.Accounts

  def deliver_later(adapter, email, config) do
    Task.async fn ->
      case adapter.deliver(email, config) do
        {:ok, _} -> # success
        {:error, error} -> remove_user_account(email)
      end
    end
  end
end

I think that is a good way to handle email delivery failures. So I'll go ahead and close this issue. If others think it should still be open, I'd be happy to reopen it.

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

5 participants