search

aheckmann / node-ses

An Amazon SES api for nodejs with proper error handling.
http://aheckmann.github.com/node-ses
MIT License
200 stars 37 forks source link

Discussion: retry when requests are throttled #28

Closed markstos closed 8 years ago

markstos commented 8 years ago

@aheckmann Any objection to re-trying requests when they fail to due to AWS throttling them?

There are some downsides:

I could also argue that throttling code belongs in an external module, but it's also appealing to have this handled in the single module that all my mail sending gets routed through.

For comparison, I checked was the AWS JS SDK does. It doesn't support throttling, I presume because the module is trying to closely mirror the HTTP REST API, which doens't help with throttling / retries, either.

aheckmann commented 8 years ago

Sounds reasonable. I don't use this module anymore so feel free to take this over.

On Tue, Jun 28, 2016, 8:53 AM Mark Stosberg notifications@github.com wrote:

@aheckmann https://github.com/aheckmann Any objection to re-trying requests when they fail to due to AWS throttling them?

There are some downsides:

  • Memory growth due to email queued waiting for retry. This could be potentially be a lot. We either need to be OK with that, or have a maximum queue size to prevent exhausting memory.
    • Potentially calling back much later when a response.

I could also argue that throttling code belongs in an external module, but it's also appealing to have this handled in the single module that all my mail sending gets routed through.

For comparison, I checked was the AWS JS SDK does. It doesn't support throttling, I presume because the module is trying to closely mirror the HTTP REST API, which doens't help with throttling / retries, either.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aheckmann/node-ses/issues/28, or mute the thread https://github.com/notifications/unsubscribe/AAKLssoqw-5GiL-yr4BkUFAgojMqS7wsks5qQUOMgaJpZM4JAQNI .

markstos commented 8 years ago

@aheckmann Thanks for the reply. Is there another SES mailer that you prefer now instead?

aheckmann commented 8 years ago

I'd probably use the official SDK but I haven't looked at it so can't say

On Thu, Jun 30, 2016, 10:13 AM Mark Stosberg notifications@github.com wrote:

@aheckmann https://github.com/aheckmann Thanks for the reply. Is there another SES mailer that you prefer now instead?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aheckmann/node-ses/issues/28#issuecomment-229725394, or mute the thread https://github.com/notifications/unsubscribe/AAKLsv_cVgwE9l4oDqMOQrVY4guwG9ASks5qQ_ksgaJpZM4JAQNI .

markstos commented 8 years ago

Note to self: We previously discussed automatic retries in #8 In that ticket the framing was about retrying on /any/ kind of failure. I think a response from AWS that "you are throttled" is a unique case. We know that the service is up and that we have permission to access it. The only problem is that we are accessing it too quickly. Retrying after some delay could automatically solve that problem. I don't think we need to handle complex cases like huge queue sizes or complex retry algorithms, but I think smoothing over infrequent cases of throttling would be a welcome default behavior.

I'll look some at how the details of this might be implemented and also perhaps look at what an external solution usinig retry might look like.

markstos commented 8 years ago

Another case worth retrying automatically a few times is 'ECONNRESET', which could simply indicate a hiccup with the connection with SES.

markstos commented 8 years ago

After more thought, automatic retry should not be a default feature. Here's why: there are two common cases for email sending and each is interested in different kinds of trade-offs:

  1. Transanctional Emails An example would be sending a quick "welcome" email or a "forgot password" email. In this case, The app may be expecting an immediate response from sending the mail. An immediate failure response would be helpful here. It might result in a message to tell the user "Oops! Try again". An automatic-retry that might result in a 5 minute or 1 hour delay would be highly undesirable here.
  2. Bulk email sending Bulk email sending may go to thousands of emails and generally happen "offline" without someone waiting on them. In this case, the sender is very interested in maximum deliverability and not on speed. The bulk sender would want automatic retries on connection failures and throtting, because a mailing that completes successfully an hour later is much better that repeatedly fails when you try to send it immediately.

Some sort of plugin/wrapper/extension could be used to provide this feature. The pull request I just submitted, #35, could help with this, as it allows consuming modules to access the function where we process the AWS response. That's the place where we would decide to retry a failed request if we choose to.