Transient Fault Handling

[dylan-asos]

This PR implements retry behaviour for transient faults when using HttpClient to send the request.

• For full backwards compatibility, default behaviour of retry logic is off. Consumer of the library must explicitly enable this using the ReliabilitySettings

• Retry logic implemented as a delegating handler in the HttpClient handlers pipeline

• New object ReliabilitySettings defines retry count and retry interval. Setting retry count > 0 enables the behaviour.

• New object SendGridClientOptions defines the settings to use with SendGridClient, and new ctor allows this object,

public SendGridClient(SendGridClientOptions options)

NB: The overloads were getting quite large and using a options object should allow future changes with no new ctors required. I could've added the reliability settings into one of the ctors, but this way seems cleaner and more future proof.

Testing: The retry logic is tested independently of the main Integration test class in new class RetryDelegatingHandlerTests.cs - just for maintainability and execution time of the unit test, we can test this behaviour independently. Am happy to stick to convention if you'd prefer it all in one file, let me know.

This should be fully backward compatible for all existing consumers - comments welcome, let me know if any issues.

[SendGridDX]

All committers have signed the CLA.

[thinkingserious]

First of all, great work!

I just have a few quick changes to start:

1. Please resolve the merge conflict
2. Please move this functionalities to the Helpers folder: https://github.com/sendgrid/sendgrid-csharp/tree/master/src/SendGrid/Helpers

Thank you!

[dylan-asos]

Sure - who updates the nuspec file? Should that be part of the PR, or do I leave to you?

If me, suggest change to 9.6.1

[thinkingserious]

I will be updating the nuspec file. When I do a minor version bump I start at patch zero. So we would go to 9.6.0. Thanks!

[thinkingserious]

Hi @dylan-asos,

This is just about ready for merge. I have one more ask, could you please add a section here that demonstrates usage?

With Best Regards,

Elmer

[dylan-asos]

Sure, done

https://github.com/dylan-asos/sendgrid-csharp/blob/master/USE_CASES.md#transient_faults

[thinkingserious]

Perfect, thank you @dylan-asos!

[thinkingserious]

Hello @dylan-asos,

Thanks again for the PR!

We want to show our appreciation by sending you some swag. Could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

[dylan-asos]

You're welcome - we'll be using this package in our project so it helps us out. Hopefully there won't be any, but any bugs or issues raised relating to this just tag me & I'll be happy to look

[dylan-asos]

NB: The nuspec file needs the Polly dependency adding. please see the example on my fork,

https://github.com/dylan-asos/sendgrid-csharp/blob/master/nuspec/Sendgrid.9.5.2.nuspec

[thinkingserious]

I'll be fixing this today, @dylan-asos, thanks for making the PR so quickly! There are some concerns about the added complexity of the Polly dependency here. Fixing this issue should be a step in the right direction.

[dylan-asos]

Yep I've been following that thread - we certainly can do it without Polly and roll our own. What we need is in the options (RetryCount / Interval), so our own try\catch could achieve the same thing.

Your proposal there sounds good, I'll close the outstanding PR and rework to lose the dependency.

[thinkingserious]

That's awesome @dylan-asos! Thanks for pushing forward on this :D

[Jericho]

This PR seems to retry all calls no matter what the error is. Was that the intent?

Did you intend to only retry when SendGrid throws HTTP 429 which indicates that you have reach your rate limit? If that's your intent, you should only retry when appropriate and also try to respect the X-RateLimit-Reset header as documented here.