search

cividesk / com.cividesk.email.sparkpost

This extension allows CiviCRM to send emails and process bounces through the SparkPost service.
10 stars 30 forks source link

New bounce type and hold threshold for over-limit events #26

Closed universalhandle closed 8 years ago

universalhandle commented 8 years ago

All SparkPost requests following the last message allowed by the hourly/daily/monthly limit are marked as bounces of type Syntax, which has hold_threshold of 3. As a result, organizations which exceed their limits just three times run the risk of marking swaths of their audience as do-not-mail.

The extension should create a new bounce type to account for this case. Perhaps we'd call the bounce type "API refused" to account for other non-bounce failures that CiviCRM would improperly treat as a bounce. Addressing this problem will also require creating at least one new bounce pattern. (The bounce pattern is used to match error messages to bounce types.) Unfortunately, the way the core code is written, we do not have the option of giving a bounce type an unlimited threshold, so the extension will have to set a high threshold (say 9,999).

Ideally, the extension would not treat this scenario as a bounce event, but I think this is a heavier lift, and I haven't done the homework yet to see what it would take for an extension to register a new mailing event with core. Or perhaps this shouldn't be the extension's job -- perhaps core needs a new mailing event so that other SMTP-related extensions don't have to implement the same event.

nganivet commented 8 years ago

Turns out I am just now testing a feature that will resolve this issue, among others, but not quite in the way you envisaged. Stay tuned for a couple more hours as we finish the tests / packaging ...

nganivet commented 8 years ago

Frank,

I have added a 'fallback' feature whereas if Sparkpost fails for any reason (sending domain not validated, exceeded sending limits, ...) the email is going to be sent through another less reliable service. If Sparkpost fails once through a mailing run, all further emails are sent through the backup service, therefore saving a lot of superfluous calls.

This resolves nicely what you asked for. Can you please test and make sure that you stress test goes through now, with the early emails in the test sent through Sparkpost and the rest through the backup email service?

Than ks, --Nicolas.

ginkgomzd commented 8 years ago

If these over-limit emails are explicitly flagged and easily identifiable, then this issue might be addressed. Is it configurable what happens as a fallback? I'm not sure we know what an acceptable fallback service would be. Might be better to facilitate re-sending later after the limit is expired.

nganivet commented 8 years ago

Yes, there is a link on the setup screen to configure the fallback. You can use sendmail(), PHP mail(), an SMTP server.

We ABSOLUTELY need a fallback solution as when people send mail from an unverified domain Sparkpost rejects it with no appeal. A simple 'resend later' will not fix this. So the fallback kills at least 2 birds with one stone (I'm sure we'll find other use cases for a fallback later).

PS. This situation happens a lot: many of my customers use their personal email as the To on mailings so they can get the replies. They do not have a mailboxes on the organization's domain, and educating them to create such mailboxes, or at least aliases that redirect to their personal email, takes time ...

------ Original Message ------ From: "Michael Z Daryabeygi" notifications@github.com To: "cividesk/com.cividesk.email.sparkpost" com.cividesk.email.sparkpost@noreply.github.com Cc: "nganivet" nicolas@cividesk.com; "State change" state_change@noreply.github.com Sent: 5/1/2016 11:59:58 AM Subject: Re: [cividesk/com.cividesk.email.sparkpost] New bounce type and hold threshold for over-limit events (#26)

If these over-limit emails are explicitly flagged and easily identifiable, then this issue might be addressed. Is it configurable what happens as a fallback? I'm not sure we know what an acceptable fallback service would be. Might be better to facilitate re-sending later after the limit is expired.

— You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub

universalhandle commented 8 years ago

I disagree that the fallback addresses the issue. It's good that you made the fallback configurable so users can opt out of this behavior.

The fallback option rather defeats the purpose of the extension. If a client runs up against her daily limit, I'd much rather have a way to re-queue the messages than have them sent off a black-listed server, especially if the fallback happens transparently and the customer is paying for SparkPost services.

Please reopen this issue and we'll see about contributing code for this. From a skim of the code, it would appear that our approaches can play nice together.

mattlinduk commented 8 years ago

I agree with @GinkgoFJG on the point about the fallback. I would imagine that there are certainly a few users using this extension to reliably deliver emails without having to send from their server, and having a fallback for another STMP service does defeat the purpose of having the first set sent though sparkpost.

Re-queueing the messages certainly seems like the ideal solution here, for which the daily limit for becomes a bottleneck but not a barrier for delivery.

axon-obriend commented 8 years ago

Is it possible to abort the sending of the mailing when you get an API response from Sparkpost that indicates you've hit a sending limit, perhaps following the same path in the code that you'd take once you hit the throttling limit configured in CiviMail? That would allow the mailing to continue at the next cron run where it left off once a new Sparkpost limit period had started.

mattlinduk commented 8 years ago

@nganivet Is there any chance of picking this up again? It's something that we'd be interested in to smooth the bulk mailings sending experience so provisionally could help with payment for this. Would you be able to let us know a ballpark figure for the cost?

universalhandle commented 8 years ago

Another thought occurred to me. Should "syntax" "bounces" ever count against a contact's threshold? (I'm using "syntax" in quotes because going over your daily sending limit really has nothing to do with the syntax of the request; "syntax" is being used as a catch-all. I'm using "bounce" in quotes because this isn't really a bounce; another mail server isn't telling us, "Hey, I can't deliver this.")

I'd have to dig back into the core code (amazing how quickly I forget this stuff) to see if CiviCRM ever classifies real bounces as syntax bounces, but I suspect not; if we get a reply from another mail server, I think CiviCRM knows what to do with that and doesn't drop it in the "syntax" catch-all. If so, then another way to solve this problem is to update the threshold for this type of bounce. I don't believe CiviCRM provides a UI for this, but it's very easy to do via PHPMyAdmin or via a SQL query.

nganivet commented 8 years ago

@mattlinduk There has been multiple solutions outlined in this thread. What would you be interested in developing?

All,

There are no more bounces generated when sending over quotas, so that resolves the main issue outlined by Frank/Michael.

Queuing and re-sending when the limit period is over does not seem like a good solution: no reasonable nonprofit would accept having their mailings/donation receipts/event reminders sent with a 24hrs delay because they are over limit.

So the 2 remaining solutions are:

I am open to implementing other solutions if needed, but only if they make operational sense.

universalhandle commented 8 years ago

There are no more bounces generated when sending over quotas, so that resolves the main issue outlined by Frank/Michael.

This is news to me. Can you say more about this? How does the extension now deal with over-quota scenarios?

nganivet commented 8 years ago

Frank - see my reply (https://github.com/cividesk/com.cividesk.email.sparkpost/issues/26#issuecomment-216057008) above in this thread.

universalhandle commented 8 years ago

Oh, that. No, as I said before, I don't think that falling back to a backup service addresses the issue. Leaving aside the question of whether or not the approach is appropriate, it doesn't address the syntax bounce problem for users who don't have a backup service configured. Such users will end up with syntax bounces, which, if the threshold isn't adjusted, will result in email addresses being put on hold.

nganivet commented 8 years ago

Frank - there is no longer a bounce, but an exception thrown on sending the email (cf. https://github.com/cividesk/com.cividesk.email.sparkpost/blob/master/CRM/Sparkpost.php#L147). If a backup mailer is configured the exception is caught and the email resent with the backup. (TO BE VERIFIED) If not backup mailer is configured the mailing should abort and be restarted on the next cron schedule. So it implicitly implements the queue, but not sure if we're not losing the email triggering the Exception.

universalhandle commented 8 years ago

Ah, okay. That's a much more useful answer. Thanks for taking the extra minute to explain. We'll take a look and see if we can answer some of the questions around that.