From 48c1da3dc6d64c232340c4c8835f9cedc5836a31 Mon Sep 17 00:00:00 2001 From: dylanm Date: Thu, 17 Aug 2017 18:08:27 +0100 Subject: [PATCH 01/17] Transient fault handling --- .../Reliability/ReliabilitySettings.cs | 70 +++++++++++ .../Reliability/RetryDelegatingHandler.cs | 87 +++++++++++++ src/SendGrid/SendGridClient.cs | 66 +++++++--- src/SendGrid/SendGridClientOptions.cs | 52 ++++++++ tests/SendGrid.Tests/Integration.cs | 61 +++++++++ .../RetryDelegatingHandlerTests.cs | 119 ++++++++++++++++++ .../RetryTestBehaviourDelegatingHandler.cs | 65 ++++++++++ 7 files changed, 506 insertions(+), 14 deletions(-) create mode 100644 src/SendGrid/Reliability/ReliabilitySettings.cs create mode 100644 src/SendGrid/Reliability/RetryDelegatingHandler.cs create mode 100644 src/SendGrid/SendGridClientOptions.cs create mode 100644 tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs create mode 100644 tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs diff --git a/src/SendGrid/Reliability/ReliabilitySettings.cs b/src/SendGrid/Reliability/ReliabilitySettings.cs new file mode 100644 index 000000000..c201691af --- /dev/null +++ b/src/SendGrid/Reliability/ReliabilitySettings.cs @@ -0,0 +1,70 @@ +namespace SendGrid.Helpers.Reliability +{ + using System; + + /// + /// Defines the reliability settings to use on HTTP requests + /// + public class ReliabilitySettings + { + private int retryCount; + + private TimeSpan retryInterval; + + /// + /// Initializes a new instance of the class. + /// + public ReliabilitySettings() + { + this.retryCount = 0; + this.retryInterval = TimeSpan.FromSeconds(1); + } + + /// + /// Gets or sets the number of retries to execute against when sending an HTTP Request before throwing an exception. Defaults to 0 (no retries, you must explicitly enable) + /// + public int RetryCount + { + get + { + return this.retryCount; + } + + set + { + if (value < 0) + { + throw new ArgumentException("Retry count must be greater than zero"); + } + + if (value > 5) + { + throw new ArgumentException("The maximum number of retries is 5"); + } + + this.retryCount = value; + } + } + + /// + /// Gets or sets the interval between HTTP retries. Defaults to 1 second + /// + public TimeSpan RetryInterval + { + get + { + return this.retryInterval; + } + + set + { + if (value.TotalSeconds > 30) + { + throw new ArgumentException("The maximum retry interval is 30 seconds"); + } + + this.retryInterval = value; + } + } + } +} \ No newline at end of file diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs new file mode 100644 index 000000000..37a9e8054 --- /dev/null +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -0,0 +1,87 @@ +namespace SendGrid.Helpers.Reliability +{ + using System; + using System.Net.Http; + using System.Threading; + using System.Threading.Tasks; + + /// + /// A delegating handler that provides retry functionality while executing a request + /// + public class RetryDelegatingHandler : DelegatingHandler + { + private readonly ReliabilitySettings settings; + + /// + /// Initializes a new instance of the class. + /// + /// A ReliabilitySettings instance + public RetryDelegatingHandler(ReliabilitySettings settings) + : this(new HttpClientHandler(), settings) + { + } + + public RetryDelegatingHandler(HttpMessageHandler innerHandler, ReliabilitySettings settings) + : base(innerHandler) + { + this.settings = settings; + } + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (this.settings.RetryCount == 0) + { + return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + + HttpResponseMessage responseMessage = null; + var numberOfAttempts = 0; + var sent = false; + + while (!sent) + { + try + { + responseMessage = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + + ThrowHttpRequestExceptionIfResponseIsWithinTheServerErrorRange(responseMessage); + + sent = true; + } + catch (TaskCanceledException) + { + numberOfAttempts++; + + if (numberOfAttempts > this.settings.RetryCount) + { + throw new TimeoutException(); + } + + // ReSharper disable once MethodSupportsCancellation, cancel will be indicated on the token + await Task.Delay(this.settings.RetryInterval); + } + catch (HttpRequestException) + { + numberOfAttempts++; + + if (numberOfAttempts > this.settings.RetryCount) + { + throw; + } + + await Task.Delay(this.settings.RetryInterval); + } + } + + return responseMessage; + } + + private static void ThrowHttpRequestExceptionIfResponseIsWithinTheServerErrorRange(HttpResponseMessage responseMessage) + { + if ((int)responseMessage.StatusCode >= 500 && (int)responseMessage.StatusCode < 600) + { + throw new HttpRequestException(string.Format("Response Http Status code {0} indicates server error", responseMessage.StatusCode)); + } + } + } +} \ No newline at end of file diff --git a/src/SendGrid/SendGridClient.cs b/src/SendGrid/SendGridClient.cs index 6197967ce..eb326966d 100644 --- a/src/SendGrid/SendGridClient.cs +++ b/src/SendGrid/SendGridClient.cs @@ -16,12 +16,15 @@ namespace SendGrid using System.Text; using System.Threading; using System.Threading.Tasks; + using SendGrid.Helpers.Reliability; /// /// A HTTP client wrapper for interacting with SendGrid's API /// public class SendGridClient : ISendGridClient { + private readonly SendGridClientOptions options; + /// /// Gets or sets the path to the API resource. /// @@ -63,16 +66,48 @@ public SendGridClient(IWebProxy webProxy, string apiKey, string host = null, Dic PreAuthenticate = true, UseDefaultCredentials = false, }; - client = new HttpClient(httpClientHandler); + + var retryHandler = new RetryDelegatingHandler(httpClientHandler, options.ReliabilitySettings); + + client = new HttpClient(retryHandler); } else { - client = new HttpClient(); + client = CreateHttpClientWithRetryHandler(); } InitiateClient(apiKey, host, requestHeaders, version, urlPath); } + /// + /// Initializes a new instance of the class. + /// + /// A instance that defines the configuration settings to use with the client + /// Interface to the SendGrid REST API + public SendGridClient(SendGridClientOptions options) + : this(null, options) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// An optional http client which may me injected in order to facilitate testing. + /// A instance that defines the configuration settings to use with the client + /// Interface to the SendGrid REST API + public SendGridClient(HttpClient httpClient, SendGridClientOptions options) + { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + this.options = options; + client = (httpClient == null) ? CreateHttpClientWithRetryHandler() : httpClient; + + InitiateClient(options.ApiKey, options.Host, options.RequestHeaders, options.Version, options.UrlPath); + } + /// /// Initializes a new instance of the class. /// @@ -84,10 +119,8 @@ public SendGridClient(IWebProxy webProxy, string apiKey, string host = null, Dic /// Path to endpoint (e.g. /path/to/endpoint) /// Interface to the SendGrid REST API public SendGridClient(HttpClient httpClient, string apiKey, string host = null, Dictionary requestHeaders = null, string version = "v3", string urlPath = null) + : this(httpClient, new SendGridClientOptions() { ApiKey = apiKey, Host = host, RequestHeaders = requestHeaders, Version = version, UrlPath = urlPath }) { - client = (httpClient == null) ? new HttpClient() : httpClient; - - InitiateClient(apiKey, host, requestHeaders, version, urlPath); } /// @@ -159,6 +192,11 @@ private void InitiateClient(string apiKey, string host, Dictionary /// The supported API methods. /// @@ -227,11 +265,11 @@ public virtual AuthenticationHeaderValue AddAuthorization(KeyValuePair public async Task RequestAsync( - SendGridClient.Method method, - string requestBody = null, - string queryParams = null, - string urlPath = null, - CancellationToken cancellationToken = default(CancellationToken)) + SendGridClient.Method method, + string requestBody = null, + string queryParams = null, + string urlPath = null, + CancellationToken cancellationToken = default(CancellationToken)) { var endpoint = client.BaseAddress + BuildUrl(urlPath, queryParams); @@ -261,10 +299,10 @@ public async Task RequestAsync( public async Task SendEmailAsync(SendGridMessage msg, CancellationToken cancellationToken = default(CancellationToken)) { return await RequestAsync( - Method.POST, - msg.Serialize(), - urlPath: "mail/send", - cancellationToken: cancellationToken).ConfigureAwait(false); + Method.POST, + msg.Serialize(), + urlPath: "mail/send", + cancellationToken: cancellationToken).ConfigureAwait(false); } /// diff --git a/src/SendGrid/SendGridClientOptions.cs b/src/SendGrid/SendGridClientOptions.cs new file mode 100644 index 000000000..a77a86e1e --- /dev/null +++ b/src/SendGrid/SendGridClientOptions.cs @@ -0,0 +1,52 @@ +namespace SendGrid +{ + using System.Collections.Generic; + using SendGrid.Helpers.Reliability; + + /// + /// Defines the options to use with the SendGrid client + /// + public class SendGridClientOptions + { + /// + /// Initializes a new instance of the class. + /// + public SendGridClientOptions() + { + ReliabilitySettings = new ReliabilitySettings(); + RequestHeaders = new Dictionary(); + Host = "https://api.sendgrid.com"; + Version = "v3"; + } + + /// + /// Gets the reliability settings to use on HTTP Requests + /// + public ReliabilitySettings ReliabilitySettings { get; } + + /// + /// Gets or sets the SendGrid API key + /// + public string ApiKey { get; set; } + + /// + /// Gets or sets the request headers to use on HttpRequests sent to SendGrid + /// + public Dictionary RequestHeaders { get; set; } + + /// + /// Gets or sets base url (e.g. https://api.sendgrid.com, this is the default) + /// + public string Host { get; set; } + + /// + /// Gets or sets API version, override AddVersion to customize + /// + public string Version { get; set; } + + /// + /// Gets or sets the path to the API endpoint. + /// + public string UrlPath { get; set; } + } +} \ No newline at end of file diff --git a/tests/SendGrid.Tests/Integration.cs b/tests/SendGrid.Tests/Integration.cs index 5ec2cbe65..ded66c73d 100644 --- a/tests/SendGrid.Tests/Integration.cs +++ b/tests/SendGrid.Tests/Integration.cs @@ -11,6 +11,8 @@ using Xunit; using System.Threading; using System.Text; + using Helpers.Reliability; + using Reliability; using Xunit.Abstractions; public class IntegrationFixture : IDisposable @@ -6106,6 +6108,65 @@ public void TestJsonNetReferenceHandling(string referenceHandlingProperty) bool containsReferenceHandlingProperty = serializedMessage.Contains(referenceHandlingProperty); Assert.False(containsReferenceHandlingProperty); } + + [Fact] + public async Task TestRetryBehaviourThrowsTimeoutException() + { + var msg = new SendGridMessage(); + msg.SetFrom(new EmailAddress("test@example.com")); + msg.AddTo(new EmailAddress("test@example.com")); + msg.SetSubject("Hello World from the SendGrid CSharp Library"); + msg.AddContent(MimeType.Html, "HTML content"); + + var options = new SendGridClientOptions + { + ApiKey = fixture.apiKey, + ReliabilitySettings = { RetryCount = 1 }, + Host = "http://localhost:4010" + }; + + var id = "test_url_param"; + + var retryHandler = new RetryDelegatingHandler(new HttpClientHandler(), options.ReliabilitySettings); + + HttpClient clientToInject = new HttpClient(retryHandler) { Timeout = TimeSpan.FromMilliseconds(1) }; + var sg = new SendGridClient(clientToInject, options); + + var exception = await Assert.ThrowsAsync(() => sg.SendEmailAsync(msg)); + + Assert.NotNull(exception); + } + + [Fact] + public async Task TestRetryBehaviourSucceedsOnSecondAttempt() + { + var msg = new SendGridMessage(); + msg.SetFrom(new EmailAddress("test@example.com")); + msg.AddTo(new EmailAddress("test@example.com")); + msg.SetSubject("Hello World from the SendGrid CSharp Library"); + msg.AddContent(MimeType.Html, "HTML content"); + + var options = new SendGridClientOptions + { + ApiKey = fixture.apiKey, + ReliabilitySettings = { RetryCount = 1 } + }; + + var id = "test_url_param"; + + var httpMessageHandler = new RetryTestBehaviourDelegatingHandler(); + httpMessageHandler.AddBehaviour(httpMessageHandler.TaskCancelled); + httpMessageHandler.AddBehaviour(httpMessageHandler.OK); + + var retryHandler = new RetryDelegatingHandler(httpMessageHandler, options.ReliabilitySettings); + + HttpClient clientToInject = new HttpClient(retryHandler); + var sg = new SendGridClient(clientToInject, options); + + var result = await sg.SendEmailAsync(msg); + + Assert.Equal(HttpStatusCode.OK, result.StatusCode); + } } public class FakeHttpMessageHandler : HttpMessageHandler diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs new file mode 100644 index 000000000..c7d190c9e --- /dev/null +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -0,0 +1,119 @@ +namespace SendGrid.Tests.Reliability +{ + using System; + using System.Net; + using System.Net.Http; + using System.Threading.Tasks; + using SendGrid.Helpers.Reliability; + using Xunit; + + public class RetryDelegatingHandlerTests + { + private readonly HttpClient client; + + private readonly RetryTestBehaviourDelegatingHandler innerHandler; + + public RetryDelegatingHandlerTests() + { + var reliabilitySettings = new ReliabilitySettings + { + RetryCount = 1 + }; + innerHandler = new RetryTestBehaviourDelegatingHandler(); + client = new HttpClient(new RetryDelegatingHandler(innerHandler, reliabilitySettings)) + { + BaseAddress = new Uri("http://localhost") + }; + } + + [Fact] + public async Task Invoke_ShouldReturnHttpResponseAndNotRetryWhenSuccessful() + { + innerHandler.AddBehaviour(innerHandler.OK); + + var result = await client.SendAsync(new HttpRequestMessage()); + + Assert.Equal(result.StatusCode, HttpStatusCode.OK); + Assert.Equal(1, innerHandler.InvocationCount); + } + + [Fact] + public async Task Invoke_ShouldReturnHttpResponseAndNotRetryWhenUnauthorised() + { + innerHandler.AddBehaviour(innerHandler.AuthenticationError); + + var result = await client.SendAsync(new HttpRequestMessage()); + + Assert.Equal(result.StatusCode, HttpStatusCode.Unauthorized); + Assert.Equal(1, innerHandler.InvocationCount); + } + + [Fact] + public async Task Invoke_ShouldReturnErrorWithoutRetryWhenErrorIsNotTransient() + { + innerHandler.AddBehaviour(innerHandler.NonTransientException); + + await Assert.ThrowsAsync(() => client.SendAsync(new HttpRequestMessage())); + + Assert.Equal(1, innerHandler.InvocationCount); + } + + [Fact] + public async Task Invoke_ShoulddRetryOnceWhenFailedOnFirstAttemptThenSuccessful() + { + innerHandler.AddBehaviour(innerHandler.TaskCancelled); + innerHandler.AddBehaviour(innerHandler.OK); + + var result = await client.SendAsync(new HttpRequestMessage()); + + Assert.Equal(result.StatusCode, HttpStatusCode.OK); + Assert.Equal(2, innerHandler.InvocationCount); + } + + [Fact] + public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnTimeoutExceptionWhenTasksCancelled() + { + innerHandler.AddBehaviour(innerHandler.TaskCancelled); + innerHandler.AddBehaviour(innerHandler.TaskCancelled); + + await Assert.ThrowsAsync(() => client.SendAsync(new HttpRequestMessage())); + + Assert.Equal(2, innerHandler.InvocationCount); + } + + [Fact] + public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnExceptionWhenInternalServerErrorsEncountered() + { + innerHandler.AddBehaviour(innerHandler.InternalServerError); + innerHandler.AddBehaviour(innerHandler.InternalServerError); + + await Assert.ThrowsAsync(() => client.SendAsync(new HttpRequestMessage())); + + Assert.Equal(2, innerHandler.InvocationCount); + } + + [Fact] + public void ReliabilitySettingsShouldNotAllowNegativeRetryCount() + { + var settings = new ReliabilitySettings(); + + Assert.Throws(() => settings.RetryCount = -1); + } + + [Fact] + public void ReliabilitySettingsShouldNotAllowRetryCountGreaterThan5() + { + var settings = new ReliabilitySettings(); + + Assert.Throws(() => settings.RetryCount = 6); + } + + [Fact] + public void ReliabilitySettingsShouldNotAllowRetryIntervalGreaterThan30Seconds() + { + var settings = new ReliabilitySettings(); + + Assert.Throws(() => settings.RetryInterval = TimeSpan.FromSeconds(31)); + } + } +} \ No newline at end of file diff --git a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs new file mode 100644 index 000000000..40d60ca29 --- /dev/null +++ b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs @@ -0,0 +1,65 @@ +namespace SendGrid.Tests.Reliability +{ + using System; + using System.Collections.Generic; + using System.Net; + using System.Net.Http; + using System.Threading; + using System.Threading.Tasks; + + public class RetryTestBehaviourDelegatingHandler : DelegatingHandler + { + private readonly List>> behaviours; + + public RetryTestBehaviourDelegatingHandler() + { + behaviours = new List>>(); + } + + public int InvocationCount { get; private set; } + + public void AddBehaviour(Func> configuredBehavior) + { + Task behaviour() + { + InvocationCount++; + return configuredBehavior(); + } + + behaviours.Add(behaviour); + } + + public Task OK() + { + var httpResponseMessage = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(string.Empty) }; + return Task.Factory.StartNew(() => httpResponseMessage); + } + + public Task InternalServerError() + { + var httpResponseMessage = new HttpResponseMessage(HttpStatusCode.InternalServerError) { Content = new StringContent(string.Empty) }; + return Task.Factory.StartNew(() => httpResponseMessage); + } + + public Task AuthenticationError() + { + var httpResponseMessage = new HttpResponseMessage(HttpStatusCode.Unauthorized) { Content = new StringContent(string.Empty) }; + return Task.Factory.StartNew(() => httpResponseMessage); + } + + public Task TaskCancelled() + { + throw new TaskCanceledException(); + } + + public Task NonTransientException() + { + throw new InvalidOperationException(); + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + return behaviours[InvocationCount](); + } + } +} \ No newline at end of file From d17205d5ffc57d1acae1a747be1b89957d7fee1f Mon Sep 17 00:00:00 2001 From: Dylan Morley Date: Thu, 17 Aug 2017 18:13:43 +0100 Subject: [PATCH 02/17] Added transient fault documentation --- USE_CASES.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/USE_CASES.md b/USE_CASES.md index 7dd98b924..a5c507eea 100644 --- a/USE_CASES.md +++ b/USE_CASES.md @@ -8,6 +8,7 @@ This documentation provides examples for specific use cases. Please [open an iss * [Email - Send a Single Email to a Single Recipient](#singleemailsinglerecipient) * [Email - Send Multiple Emails to Multiple Recipients](#multipleemailsmultiplerecipients) * [Email - Transactional Templates](#transactional_templates) +* [Transient Fault Handling](#transient_faults) # Attachments @@ -582,3 +583,52 @@ namespace Example } } ``` + + +# Transient Fault Handling + +The SendGridClient provides functionality for handling transient errors that might occur when sending an HttpRequest. This includes client side timeouts while sending the mail, or any errors returned within the 500 range. + +By default, retry behaviour is off, you must explicitly enable it by setting the retry count to a value greater than zero. To set the retry count, you must use the SendGridClient construct that takes a **SendGridClientOptions** object, allowing you to configure the **ReliabilitySettings** + +### RetryCount + +The amount of times to retry the operation before reporting an exception to the caller. This is in addition to the initial attempt, so setting a value of 1 would result in 2 attempts - the initial attempt and the retry. Defaults to zero, so retry behaviour is not enabled. The maximum amount of retries permitted is 5. + +### RetryInterval + +The policy implemented is a 'wait and retry'. The RetryInterval setting defines the amount of time to wait between operations that fail before attmepting a retry. By default, this is set to 1 second - the maximum amount of time allowed is 30 seconds. + +## Examples + +In this example, we are setting RetryCount to 1, with an interval between attempts of 5 seconds. This means that if the inital attempt to send mail fails, then it will wait 5 seconds then try again a single time. + +```csharp + +var options = new SendGridClientOptions +{ + ApiKey = "Your-Api-Key", + ReliabilitySettings = {RetryCount = 1, RetryInterval = TimeSpan.FromSeconds(5)} +}; + +var client = new SendGridClient(options); + +``` + +The SendGridClientOptions object defines all the settings that can be set for the client, e.g. + +```csharp + +var options = new SendGridClientOptions +{ + ApiKey = "Your-Api-Key", + ReliabilitySettings = {RetryCount = 1, RetryInterval = TimeSpan.FromSeconds(5)}, + Host = "Your-Host", + UrlPath = "Url-Path", + Version = "3", + RequestHeaders = new Dictionary() {{"header-key", "header-value"}} +}; + +var client = new SendGridClient(options); + +``` From 7ef6f022e9b9d970bf0b1e53306a13da88843f0c Mon Sep 17 00:00:00 2001 From: Dylan Morley Date: Thu, 17 Aug 2017 18:14:53 +0100 Subject: [PATCH 03/17] Fixed spelling error --- USE_CASES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/USE_CASES.md b/USE_CASES.md index a5c507eea..c5ed81124 100644 --- a/USE_CASES.md +++ b/USE_CASES.md @@ -597,7 +597,7 @@ The amount of times to retry the operation before reporting an exception to the ### RetryInterval -The policy implemented is a 'wait and retry'. The RetryInterval setting defines the amount of time to wait between operations that fail before attmepting a retry. By default, this is set to 1 second - the maximum amount of time allowed is 30 seconds. +The policy implemented is a 'wait and retry'. The RetryInterval setting defines the amount of time to wait between operations that fail before attempting a retry. By default, this is set to 1 second - the maximum amount of time allowed is 30 seconds. ## Examples From ab1f24d5e3cc48d4f3128826a3a0661e94bdd8e8 Mon Sep 17 00:00:00 2001 From: dylanm Date: Thu, 17 Aug 2017 18:26:20 +0100 Subject: [PATCH 04/17] Added EOF new lines --- src/SendGrid/Reliability/ReliabilitySettings.cs | 2 +- src/SendGrid/Reliability/RetryDelegatingHandler.cs | 2 +- src/SendGrid/SendGridClientOptions.cs | 2 +- tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs | 2 +- .../Reliability/RetryTestBehaviourDelegatingHandler.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SendGrid/Reliability/ReliabilitySettings.cs b/src/SendGrid/Reliability/ReliabilitySettings.cs index c201691af..8f7cb7d40 100644 --- a/src/SendGrid/Reliability/ReliabilitySettings.cs +++ b/src/SendGrid/Reliability/ReliabilitySettings.cs @@ -67,4 +67,4 @@ public TimeSpan RetryInterval } } } -} \ No newline at end of file +} diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index 37a9e8054..dcc70c73c 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -84,4 +84,4 @@ private static void ThrowHttpRequestExceptionIfResponseIsWithinTheServerErrorRan } } } -} \ No newline at end of file +} diff --git a/src/SendGrid/SendGridClientOptions.cs b/src/SendGrid/SendGridClientOptions.cs index a77a86e1e..02a354212 100644 --- a/src/SendGrid/SendGridClientOptions.cs +++ b/src/SendGrid/SendGridClientOptions.cs @@ -49,4 +49,4 @@ public SendGridClientOptions() /// public string UrlPath { get; set; } } -} \ No newline at end of file +} diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index c7d190c9e..4af70bac1 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -116,4 +116,4 @@ public void ReliabilitySettingsShouldNotAllowRetryIntervalGreaterThan30Seconds() Assert.Throws(() => settings.RetryInterval = TimeSpan.FromSeconds(31)); } } -} \ No newline at end of file +} diff --git a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs index 40d60ca29..7b148c1f6 100644 --- a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs +++ b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs @@ -62,4 +62,4 @@ protected override Task SendAsync(HttpRequestMessage reques return behaviours[InvocationCount](); } } -} \ No newline at end of file +} From 2657ef119e338b0aaa4f24434107a5750603552d Mon Sep 17 00:00:00 2001 From: dylanm Date: Fri, 18 Aug 2017 10:02:52 +0100 Subject: [PATCH 05/17] Added ConfigureAwait(false) to Task.Delay --- src/SendGrid/Reliability/RetryDelegatingHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index dcc70c73c..e208c9cf8 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -58,7 +58,7 @@ protected override async Task SendAsync(HttpRequestMessage } // ReSharper disable once MethodSupportsCancellation, cancel will be indicated on the token - await Task.Delay(this.settings.RetryInterval); + await Task.Delay(this.settings.RetryInterval).ConfigureAwait(false); } catch (HttpRequestException) { @@ -69,7 +69,7 @@ protected override async Task SendAsync(HttpRequestMessage throw; } - await Task.Delay(this.settings.RetryInterval); + await Task.Delay(this.settings.RetryInterval).ConfigureAwait(false); } } From b0802c662ed65568f941c19893345b2e5fd7ac6e Mon Sep 17 00:00:00 2001 From: dylanm Date: Fri, 18 Aug 2017 17:45:54 +0100 Subject: [PATCH 06/17] Made ctor with HttpClient and Options object internal. --- src/SendGrid/SendGridClient.cs | 2 +- tests/SendGrid.Tests/Integration.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SendGrid/SendGridClient.cs b/src/SendGrid/SendGridClient.cs index eb326966d..46587f147 100644 --- a/src/SendGrid/SendGridClient.cs +++ b/src/SendGrid/SendGridClient.cs @@ -95,7 +95,7 @@ public SendGridClient(SendGridClientOptions options) /// An optional http client which may me injected in order to facilitate testing. /// A instance that defines the configuration settings to use with the client /// Interface to the SendGrid REST API - public SendGridClient(HttpClient httpClient, SendGridClientOptions options) + internal SendGridClient(HttpClient httpClient, SendGridClientOptions options) { if (options == null) { diff --git a/tests/SendGrid.Tests/Integration.cs b/tests/SendGrid.Tests/Integration.cs index ded66c73d..023db2073 100644 --- a/tests/SendGrid.Tests/Integration.cs +++ b/tests/SendGrid.Tests/Integration.cs @@ -6130,7 +6130,7 @@ public async Task TestRetryBehaviourThrowsTimeoutException() var retryHandler = new RetryDelegatingHandler(new HttpClientHandler(), options.ReliabilitySettings); HttpClient clientToInject = new HttpClient(retryHandler) { Timeout = TimeSpan.FromMilliseconds(1) }; - var sg = new SendGridClient(clientToInject, options); + var sg = new SendGridClient(clientToInject, options.ApiKey, options.Host); var exception = await Assert.ThrowsAsync(() => sg.SendEmailAsync(msg)); @@ -6161,7 +6161,7 @@ public async Task TestRetryBehaviourSucceedsOnSecondAttempt() var retryHandler = new RetryDelegatingHandler(httpMessageHandler, options.ReliabilitySettings); HttpClient clientToInject = new HttpClient(retryHandler); - var sg = new SendGridClient(clientToInject, options); + var sg = new SendGridClient(clientToInject, options.ApiKey, options.Host); var result = await sg.SendEmailAsync(msg); From 4af5dbe251fbca128b98173ed46683864b3b06e8 Mon Sep 17 00:00:00 2001 From: dylanm Date: Fri, 18 Aug 2017 18:36:35 +0100 Subject: [PATCH 07/17] Added tests to cover WebProxy construct --- src/SendGrid/SendGridClient.cs | 2 +- tests/SendGrid.Tests/Integration.cs | 33 ++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/SendGrid/SendGridClient.cs b/src/SendGrid/SendGridClient.cs index 46587f147..b5c5a7fd9 100644 --- a/src/SendGrid/SendGridClient.cs +++ b/src/SendGrid/SendGridClient.cs @@ -23,7 +23,7 @@ namespace SendGrid /// public class SendGridClient : ISendGridClient { - private readonly SendGridClientOptions options; + private readonly SendGridClientOptions options = new SendGridClientOptions(); /// /// Gets or sets the path to the API resource. diff --git a/tests/SendGrid.Tests/Integration.cs b/tests/SendGrid.Tests/Integration.cs index 023db2073..9faa289ab 100644 --- a/tests/SendGrid.Tests/Integration.cs +++ b/tests/SendGrid.Tests/Integration.cs @@ -45,7 +45,7 @@ public void Dispose() { if (Environment.GetEnvironmentVariable("TRAVIS") != "true") { - process.Kill(); + process.Kill(); Trace.WriteLine("Shutting Down Prism"); } } @@ -5994,6 +5994,22 @@ public async Task TestTakesHttpClientFactoryAsConstructorArgumentAndUsesItInHttp Assert.Equal(httpStatusCode, response.StatusCode); Assert.Equal(httpResponse, response.Body.ReadAsStringAsync().Result); } + + [Fact] + public void TestTakesProxyAsConstructorArgumentAndInitiailsesHttpClient() + { + var sg = new SendGridClient(new FakeWebProxy(), fixture.apiKey); + + Assert.NotNull(sg); + } + + [Fact] + public void TestTakesNullProxyAsConstructorArgumentAndInitiailsesHttpClient() + { + var sg = new SendGridClient(null as IWebProxy, fixture.apiKey); + + Assert.NotNull(sg); + } /// /// Tests the conditions in issue #358. @@ -6169,6 +6185,21 @@ public async Task TestRetryBehaviourSucceedsOnSecondAttempt() } } + public class FakeWebProxy : IWebProxy + { + public Uri GetProxy(Uri destination) + { + return new Uri("https://dummy-proxy"); + } + + public bool IsBypassed(Uri host) + { + return false; + } + + public ICredentials Credentials { get; set; } + } + public class FakeHttpMessageHandler : HttpMessageHandler { public virtual HttpResponseMessage Send(HttpRequestMessage request) From 547c65bc0fdbaa9d48fa5578c34817a015f9a47e Mon Sep 17 00:00:00 2001 From: dylanm Date: Fri, 18 Aug 2017 18:43:53 +0100 Subject: [PATCH 08/17] Asserting on UrlPath being set correctly --- tests/SendGrid.Tests/Integration.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/SendGrid.Tests/Integration.cs b/tests/SendGrid.Tests/Integration.cs index 9faa289ab..993ee5a20 100644 --- a/tests/SendGrid.Tests/Integration.cs +++ b/tests/SendGrid.Tests/Integration.cs @@ -5998,17 +5998,21 @@ public async Task TestTakesHttpClientFactoryAsConstructorArgumentAndUsesItInHttp [Fact] public void TestTakesProxyAsConstructorArgumentAndInitiailsesHttpClient() { - var sg = new SendGridClient(new FakeWebProxy(), fixture.apiKey); + var urlPath = "urlPath"; - Assert.NotNull(sg); + var sg = new SendGridClient(new FakeWebProxy(), fixture.apiKey, urlPath: "urlPath"); + + Assert.Equal(sg.UrlPath, urlPath); } [Fact] public void TestTakesNullProxyAsConstructorArgumentAndInitiailsesHttpClient() { - var sg = new SendGridClient(null as IWebProxy, fixture.apiKey); + var urlPath = "urlPath"; + + var sg = new SendGridClient(null as IWebProxy, fixture.apiKey, urlPath: "urlPath"); - Assert.NotNull(sg); + Assert.Equal(sg.UrlPath, urlPath); } /// From 4478cba24c01eb673aeeb661588d1108a59d796a Mon Sep 17 00:00:00 2001 From: Dylan Morley Date: Mon, 21 Aug 2017 16:45:45 +0100 Subject: [PATCH 09/17] Only retry 500 errors within 500-504 range --- .../Reliability/RetryDelegatingHandler.cs | 2 +- .../Reliability/RetryDelegatingHandlerTests.cs | 11 +++++++++++ .../RetryTestBehaviourDelegatingHandler.cs | 17 ++++++++++++----- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index e208c9cf8..91a159492 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -78,7 +78,7 @@ protected override async Task SendAsync(HttpRequestMessage private static void ThrowHttpRequestExceptionIfResponseIsWithinTheServerErrorRange(HttpResponseMessage responseMessage) { - if ((int)responseMessage.StatusCode >= 500 && (int)responseMessage.StatusCode < 600) + if ((int)responseMessage.StatusCode >= 500 && (int)responseMessage.StatusCode <= 504) { throw new HttpRequestException(string.Format("Response Http Status code {0} indicates server error", responseMessage.StatusCode)); } diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index 4af70bac1..f5e5b8192 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -58,6 +58,17 @@ public async Task Invoke_ShouldReturnErrorWithoutRetryWhenErrorIsNotTransient() Assert.Equal(1, innerHandler.InvocationCount); } + [Fact] + public async Task Invoke_ShouldReturnErrorWithoutRetryWhen500ErrorStatusIsNotTransient() + { + innerHandler.AddBehaviour(innerHandler.HttpVersionNotSupported); + + var response = await client.SendAsync(new HttpRequestMessage()); + + Assert.Equal((HttpStatusCode)505, response.StatusCode); + Assert.Equal(1, innerHandler.InvocationCount); + } + [Fact] public async Task Invoke_ShoulddRetryOnceWhenFailedOnFirstAttemptThenSuccessful() { diff --git a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs index 7b148c1f6..f3453b5f5 100644 --- a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs +++ b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs @@ -31,19 +31,26 @@ Task behaviour() public Task OK() { - var httpResponseMessage = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(string.Empty) }; - return Task.Factory.StartNew(() => httpResponseMessage); + return CreateHttpResponse(HttpStatusCode.OK); } public Task InternalServerError() { - var httpResponseMessage = new HttpResponseMessage(HttpStatusCode.InternalServerError) { Content = new StringContent(string.Empty) }; - return Task.Factory.StartNew(() => httpResponseMessage); + return CreateHttpResponse(HttpStatusCode.InternalServerError); } public Task AuthenticationError() { - var httpResponseMessage = new HttpResponseMessage(HttpStatusCode.Unauthorized) { Content = new StringContent(string.Empty) }; + return CreateHttpResponse(HttpStatusCode.Unauthorized); + } + public Task HttpVersionNotSupported() + { + return CreateHttpResponse((HttpStatusCode)505); + } + + public Task CreateHttpResponse(HttpStatusCode statusCode) + { + var httpResponseMessage = new HttpResponseMessage(statusCode) { Content = new StringContent(string.Empty) }; return Task.Factory.StartNew(() => httpResponseMessage); } From e4e7dff1b538f0efb37fdd9440c481b3e01e79a3 Mon Sep 17 00:00:00 2001 From: Dylan Morley Date: Mon, 21 Aug 2017 16:54:41 +0100 Subject: [PATCH 10/17] Limiting retry behaviour to certain 50x errors (+1 squashed commits) Squashed commits: [2ef4f3e] Fixed type in test name --- .../Reliability/RetryDelegatingHandler.cs | 13 +++++++++---- .../Reliability/RetryDelegatingHandlerTests.cs | 17 ++++++++++++++--- .../RetryTestBehaviourDelegatingHandler.cs | 10 +++++++++- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index 91a159492..92c372bb8 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -1,6 +1,7 @@ namespace SendGrid.Helpers.Reliability { using System; + using System.Collections.Generic; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -10,6 +11,8 @@ /// public class RetryDelegatingHandler : DelegatingHandler { + private static List retriableStatusCodes = new List() { 500, 502, 503, 504 }; + private readonly ReliabilitySettings settings; /// @@ -44,7 +47,7 @@ protected override async Task SendAsync(HttpRequestMessage { responseMessage = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - ThrowHttpRequestExceptionIfResponseIsWithinTheServerErrorRange(responseMessage); + ThrowHttpRequestExceptionIfResponseCodeIsRetriable(responseMessage); sent = true; } @@ -76,11 +79,13 @@ protected override async Task SendAsync(HttpRequestMessage return responseMessage; } - private static void ThrowHttpRequestExceptionIfResponseIsWithinTheServerErrorRange(HttpResponseMessage responseMessage) + private static void ThrowHttpRequestExceptionIfResponseCodeIsRetriable(HttpResponseMessage responseMessage) { - if ((int)responseMessage.StatusCode >= 500 && (int)responseMessage.StatusCode <= 504) + int statusCode = (int)responseMessage.StatusCode; + + if (retriableStatusCodes.Contains(statusCode)) { - throw new HttpRequestException(string.Format("Response Http Status code {0} indicates server error", responseMessage.StatusCode)); + throw new HttpRequestException(string.Format("Http status code '{0}' indicates server error", statusCode)); } } } diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index f5e5b8192..00562a85c 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -65,12 +65,23 @@ public async Task Invoke_ShouldReturnErrorWithoutRetryWhen500ErrorStatusIsNotTra var response = await client.SendAsync(new HttpRequestMessage()); - Assert.Equal((HttpStatusCode)505, response.StatusCode); + Assert.Equal(HttpStatusCode.HttpVersionNotSupported, response.StatusCode); Assert.Equal(1, innerHandler.InvocationCount); } [Fact] - public async Task Invoke_ShoulddRetryOnceWhenFailedOnFirstAttemptThenSuccessful() + public async Task Invoke_ShouldReturnErrorWithoutRetryWhen501ErrorStatus() + { + innerHandler.AddBehaviour(innerHandler.NotImplemented); + + var response = await client.SendAsync(new HttpRequestMessage()); + + Assert.Equal(HttpStatusCode.NotImplemented, response.StatusCode); + Assert.Equal(1, innerHandler.InvocationCount); + } + + [Fact] + public async Task Invoke_ShouldRetryOnceWhenFailedOnFirstAttemptThenSuccessful() { innerHandler.AddBehaviour(innerHandler.TaskCancelled); innerHandler.AddBehaviour(innerHandler.OK); @@ -96,7 +107,7 @@ public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnTimeoutExce public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnExceptionWhenInternalServerErrorsEncountered() { innerHandler.AddBehaviour(innerHandler.InternalServerError); - innerHandler.AddBehaviour(innerHandler.InternalServerError); + innerHandler.AddBehaviour(innerHandler.ServiceUnavailable); await Assert.ThrowsAsync(() => client.SendAsync(new HttpRequestMessage())); diff --git a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs index f3453b5f5..b594ee789 100644 --- a/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs +++ b/tests/SendGrid.Tests/Reliability/RetryTestBehaviourDelegatingHandler.cs @@ -38,6 +38,10 @@ public Task InternalServerError() { return CreateHttpResponse(HttpStatusCode.InternalServerError); } + public Task ServiceUnavailable() + { + return CreateHttpResponse(HttpStatusCode.ServiceUnavailable); + } public Task AuthenticationError() { @@ -45,7 +49,11 @@ public Task AuthenticationError() } public Task HttpVersionNotSupported() { - return CreateHttpResponse((HttpStatusCode)505); + return CreateHttpResponse(HttpStatusCode.HttpVersionNotSupported); + } + public Task NotImplemented() + { + return CreateHttpResponse(HttpStatusCode.NotImplemented); } public Task CreateHttpResponse(HttpStatusCode statusCode) From 2c02990a60b1f161da26567568bbd4ce902d27ee Mon Sep 17 00:00:00 2001 From: Dylan Morley Date: Mon, 21 Aug 2017 19:34:31 +0100 Subject: [PATCH 11/17] Renamed some fields. Added exponential calculator --- .../Reliability/ReliabilitySettings.cs | 6 +-- .../Reliability/RetryDelegatingHandler.cs | 40 +++++++++++++++---- tests/SendGrid.Tests/Integration.cs | 4 +- .../RetryDelegatingHandlerTests.cs | 6 +-- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/SendGrid/Reliability/ReliabilitySettings.cs b/src/SendGrid/Reliability/ReliabilitySettings.cs index 8f7cb7d40..0a576eb30 100644 --- a/src/SendGrid/Reliability/ReliabilitySettings.cs +++ b/src/SendGrid/Reliability/ReliabilitySettings.cs @@ -21,9 +21,9 @@ public ReliabilitySettings() } /// - /// Gets or sets the number of retries to execute against when sending an HTTP Request before throwing an exception. Defaults to 0 (no retries, you must explicitly enable) + /// Gets or sets the maximum number of retries to execute against when sending an HTTP Request before throwing an exception. Defaults to 0 (no retries, you must explicitly enable) /// - public int RetryCount + public int MaximumNumberOfRetries { get { @@ -39,7 +39,7 @@ public int RetryCount if (value > 5) { - throw new ArgumentException("The maximum number of retries is 5"); + throw new ArgumentException("The maximum number of retries that can be attempted is 5"); } this.retryCount = value; diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index 92c372bb8..564fd972d 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -11,7 +11,7 @@ /// public class RetryDelegatingHandler : DelegatingHandler { - private static List retriableStatusCodes = new List() { 500, 502, 503, 504 }; + private static readonly List RetriableStatusCodes = new List() { 500, 502, 503, 504 }; private readonly ReliabilitySettings settings; @@ -32,12 +32,15 @@ public RetryDelegatingHandler(HttpMessageHandler innerHandler, ReliabilitySettin protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - if (this.settings.RetryCount == 0) + if (this.settings.MaximumNumberOfRetries == 0) { return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); } HttpResponseMessage responseMessage = null; + var backOffCalculator = new ExponentialBackOffCalculator(settings.RetryInterval); + + var waitFor = settings.RetryInterval; var numberOfAttempts = 0; var sent = false; @@ -55,25 +58,27 @@ protected override async Task SendAsync(HttpRequestMessage { numberOfAttempts++; - if (numberOfAttempts > this.settings.RetryCount) + if (numberOfAttempts > this.settings.MaximumNumberOfRetries) { throw new TimeoutException(); } // ReSharper disable once MethodSupportsCancellation, cancel will be indicated on the token - await Task.Delay(this.settings.RetryInterval).ConfigureAwait(false); + await Task.Delay(waitFor).ConfigureAwait(false); } catch (HttpRequestException) { numberOfAttempts++; - if (numberOfAttempts > this.settings.RetryCount) + if (numberOfAttempts > this.settings.MaximumNumberOfRetries) { throw; } - await Task.Delay(this.settings.RetryInterval).ConfigureAwait(false); + await Task.Delay(waitFor).ConfigureAwait(false); } + + waitFor = backOffCalculator.GetNextWaitInterval(numberOfAttempts); } return responseMessage; @@ -83,10 +88,31 @@ private static void ThrowHttpRequestExceptionIfResponseCodeIsRetriable(HttpRespo { int statusCode = (int)responseMessage.StatusCode; - if (retriableStatusCodes.Contains(statusCode)) + if (RetriableStatusCodes.Contains(statusCode)) { throw new HttpRequestException(string.Format("Http status code '{0}' indicates server error", statusCode)); } } + + private class ExponentialBackOffCalculator + { + private TimeSpan baseInterval; + + private readonly Random random = new Random(); + + public ExponentialBackOffCalculator(TimeSpan baseInterval) + { + this.baseInterval = baseInterval; + } + + public TimeSpan GetNextWaitInterval(int numberOfAttempts) + { + var interval = this.baseInterval.TotalMilliseconds + (Math.Pow(2, numberOfAttempts) * 1000); + + var randomDelay = this.random.Next(0, 1000); + + return TimeSpan.FromMilliseconds(interval + randomDelay); + } + } } } diff --git a/tests/SendGrid.Tests/Integration.cs b/tests/SendGrid.Tests/Integration.cs index 993ee5a20..1a1c03a08 100644 --- a/tests/SendGrid.Tests/Integration.cs +++ b/tests/SendGrid.Tests/Integration.cs @@ -6141,7 +6141,7 @@ public async Task TestRetryBehaviourThrowsTimeoutException() var options = new SendGridClientOptions { ApiKey = fixture.apiKey, - ReliabilitySettings = { RetryCount = 1 }, + ReliabilitySettings = { MaximumNumberOfRetries = 1 }, Host = "http://localhost:4010" }; @@ -6169,7 +6169,7 @@ public async Task TestRetryBehaviourSucceedsOnSecondAttempt() var options = new SendGridClientOptions { ApiKey = fixture.apiKey, - ReliabilitySettings = { RetryCount = 1 } + ReliabilitySettings = { MaximumNumberOfRetries = 1 } }; var id = "test_url_param"; diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index 00562a85c..363ca7fd1 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -17,7 +17,7 @@ public RetryDelegatingHandlerTests() { var reliabilitySettings = new ReliabilitySettings { - RetryCount = 1 + MaximumNumberOfRetries = 1 }; innerHandler = new RetryTestBehaviourDelegatingHandler(); client = new HttpClient(new RetryDelegatingHandler(innerHandler, reliabilitySettings)) @@ -119,7 +119,7 @@ public void ReliabilitySettingsShouldNotAllowNegativeRetryCount() { var settings = new ReliabilitySettings(); - Assert.Throws(() => settings.RetryCount = -1); + Assert.Throws(() => settings.MaximumNumberOfRetries = -1); } [Fact] @@ -127,7 +127,7 @@ public void ReliabilitySettingsShouldNotAllowRetryCountGreaterThan5() { var settings = new ReliabilitySettings(); - Assert.Throws(() => settings.RetryCount = 6); + Assert.Throws(() => settings.MaximumNumberOfRetries = 6); } [Fact] From 58e95c756af428aa1fa6cd68a11b8350fe789710 Mon Sep 17 00:00:00 2001 From: Dylan Morley Date: Mon, 21 Aug 2017 19:42:42 +0100 Subject: [PATCH 12/17] Renamed for clarity (+1 squashed commits) Squashed commits: [e86f374] Made access private to GetNextWaitInterval (+1 squashed commits) Squashed commits: [462983b] Renamed some unit tests --- .../Reliability/RetryDelegatingHandler.cs | 29 ++++++------------- .../RetryDelegatingHandlerTests.cs | 16 +++++----- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index 564fd972d..1bf4ea2b5 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -15,6 +15,8 @@ public class RetryDelegatingHandler : DelegatingHandler private readonly ReliabilitySettings settings; + private readonly Random random = new Random(); + /// /// Initializes a new instance of the class. /// @@ -38,7 +40,6 @@ protected override async Task SendAsync(HttpRequestMessage } HttpResponseMessage responseMessage = null; - var backOffCalculator = new ExponentialBackOffCalculator(settings.RetryInterval); var waitFor = settings.RetryInterval; var numberOfAttempts = 0; @@ -50,7 +51,7 @@ protected override async Task SendAsync(HttpRequestMessage { responseMessage = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - ThrowHttpRequestExceptionIfResponseCodeIsRetriable(responseMessage); + ThrowHttpRequestExceptionIfResponseCodeCanBeRetried(responseMessage); sent = true; } @@ -78,13 +79,13 @@ protected override async Task SendAsync(HttpRequestMessage await Task.Delay(waitFor).ConfigureAwait(false); } - waitFor = backOffCalculator.GetNextWaitInterval(numberOfAttempts); + waitFor = GetNextWaitInterval(numberOfAttempts); } return responseMessage; } - private static void ThrowHttpRequestExceptionIfResponseCodeIsRetriable(HttpResponseMessage responseMessage) + private static void ThrowHttpRequestExceptionIfResponseCodeCanBeRetried(HttpResponseMessage responseMessage) { int statusCode = (int)responseMessage.StatusCode; @@ -94,25 +95,13 @@ private static void ThrowHttpRequestExceptionIfResponseCodeIsRetriable(HttpRespo } } - private class ExponentialBackOffCalculator + private TimeSpan GetNextWaitInterval(int numberOfAttempts) { - private TimeSpan baseInterval; - - private readonly Random random = new Random(); - - public ExponentialBackOffCalculator(TimeSpan baseInterval) - { - this.baseInterval = baseInterval; - } - - public TimeSpan GetNextWaitInterval(int numberOfAttempts) - { - var interval = this.baseInterval.TotalMilliseconds + (Math.Pow(2, numberOfAttempts) * 1000); + var interval = this.settings.RetryInterval.TotalMilliseconds + (Math.Pow(2, numberOfAttempts) * 1000); - var randomDelay = this.random.Next(0, 1000); + var randomDelay = this.random.Next(0, 1000); - return TimeSpan.FromMilliseconds(interval + randomDelay); - } + return TimeSpan.FromMilliseconds(interval + randomDelay); } } } diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index 363ca7fd1..2bbcc0aa1 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -27,7 +27,7 @@ public RetryDelegatingHandlerTests() } [Fact] - public async Task Invoke_ShouldReturnHttpResponseAndNotRetryWhenSuccessful() + public async Task ShouldReturnHttpResponseAndNotRetryWhenSuccessful() { innerHandler.AddBehaviour(innerHandler.OK); @@ -38,7 +38,7 @@ public async Task Invoke_ShouldReturnHttpResponseAndNotRetryWhenSuccessful() } [Fact] - public async Task Invoke_ShouldReturnHttpResponseAndNotRetryWhenUnauthorised() + public async Task ShouldReturnHttpResponseAndNotRetryWhenUnauthorised() { innerHandler.AddBehaviour(innerHandler.AuthenticationError); @@ -49,7 +49,7 @@ public async Task Invoke_ShouldReturnHttpResponseAndNotRetryWhenUnauthorised() } [Fact] - public async Task Invoke_ShouldReturnErrorWithoutRetryWhenErrorIsNotTransient() + public async Task ShouldReturnErrorWithoutRetryWhenErrorIsNotTransient() { innerHandler.AddBehaviour(innerHandler.NonTransientException); @@ -59,7 +59,7 @@ public async Task Invoke_ShouldReturnErrorWithoutRetryWhenErrorIsNotTransient() } [Fact] - public async Task Invoke_ShouldReturnErrorWithoutRetryWhen500ErrorStatusIsNotTransient() + public async Task ShouldReturnErrorWithoutRetryWhen500ErrorStatusIsNotTransient() { innerHandler.AddBehaviour(innerHandler.HttpVersionNotSupported); @@ -70,7 +70,7 @@ public async Task Invoke_ShouldReturnErrorWithoutRetryWhen500ErrorStatusIsNotTra } [Fact] - public async Task Invoke_ShouldReturnErrorWithoutRetryWhen501ErrorStatus() + public async Task ShouldReturnErrorWithoutRetryWhen501ErrorStatus() { innerHandler.AddBehaviour(innerHandler.NotImplemented); @@ -81,7 +81,7 @@ public async Task Invoke_ShouldReturnErrorWithoutRetryWhen501ErrorStatus() } [Fact] - public async Task Invoke_ShouldRetryOnceWhenFailedOnFirstAttemptThenSuccessful() + public async Task ShouldRetryOnceWhenFailedOnFirstAttemptThenSuccessful() { innerHandler.AddBehaviour(innerHandler.TaskCancelled); innerHandler.AddBehaviour(innerHandler.OK); @@ -93,7 +93,7 @@ public async Task Invoke_ShouldRetryOnceWhenFailedOnFirstAttemptThenSuccessful() } [Fact] - public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnTimeoutExceptionWhenTasksCancelled() + public async Task ShouldRetryTheExpectedAmountOfTimesAndReturnTimeoutExceptionWhenTasksCancelled() { innerHandler.AddBehaviour(innerHandler.TaskCancelled); innerHandler.AddBehaviour(innerHandler.TaskCancelled); @@ -104,7 +104,7 @@ public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnTimeoutExce } [Fact] - public async Task Invoke_ShouldRetryTheExpectedAmountOfTimesAndReturnExceptionWhenInternalServerErrorsEncountered() + public async Task ShouldRetryTheExpectedAmountOfTimesAndReturnExceptionWhenInternalServerErrorsEncountered() { innerHandler.AddBehaviour(innerHandler.InternalServerError); innerHandler.AddBehaviour(innerHandler.ServiceUnavailable); From 8d6395407b789e9d7d2377c8becd88fa435065d6 Mon Sep 17 00:00:00 2001 From: dylanm Date: Tue, 22 Aug 2017 09:18:45 +0100 Subject: [PATCH 13/17] Refactored, use HttpStatusCode enum for retriable errors and better naming --- .../Reliability/RetryDelegatingHandler.cs | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index 1bf4ea2b5..2080e1cf3 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -2,6 +2,7 @@ { using System; using System.Collections.Generic; + using System.Net; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -11,7 +12,14 @@ /// public class RetryDelegatingHandler : DelegatingHandler { - private static readonly List RetriableStatusCodes = new List() { 500, 502, 503, 504 }; + private static readonly List RetriableServerErrorStatusCodes = + new List() + { + HttpStatusCode.InternalServerError, + HttpStatusCode.BadGateway, + HttpStatusCode.ServiceUnavailable, + HttpStatusCode.GatewayTimeout + }; private readonly ReliabilitySettings settings; @@ -26,6 +34,11 @@ public RetryDelegatingHandler(ReliabilitySettings settings) { } + /// + /// Initializes a new instance of the class. + /// + /// A HttpMessageHandler instance to set as the innner handler + /// A ReliabilitySettings instance public RetryDelegatingHandler(HttpMessageHandler innerHandler, ReliabilitySettings settings) : base(innerHandler) { @@ -41,12 +54,13 @@ protected override async Task SendAsync(HttpRequestMessage HttpResponseMessage responseMessage = null; - var waitFor = settings.RetryInterval; var numberOfAttempts = 0; var sent = false; while (!sent) { + var waitFor = this.GetNextWaitInterval(numberOfAttempts); + try { responseMessage = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); @@ -78,8 +92,6 @@ protected override async Task SendAsync(HttpRequestMessage await Task.Delay(waitFor).ConfigureAwait(false); } - - waitFor = GetNextWaitInterval(numberOfAttempts); } return responseMessage; @@ -87,21 +99,26 @@ protected override async Task SendAsync(HttpRequestMessage private static void ThrowHttpRequestExceptionIfResponseCodeCanBeRetried(HttpResponseMessage responseMessage) { - int statusCode = (int)responseMessage.StatusCode; - - if (RetriableStatusCodes.Contains(statusCode)) + if (RetriableServerErrorStatusCodes.Contains(responseMessage.StatusCode)) { - throw new HttpRequestException(string.Format("Http status code '{0}' indicates server error", statusCode)); + throw new HttpRequestException(string.Format("Http status code '{0}' indicates server error", responseMessage.StatusCode)); } } private TimeSpan GetNextWaitInterval(int numberOfAttempts) { - var interval = this.settings.RetryInterval.TotalMilliseconds + (Math.Pow(2, numberOfAttempts) * 1000); + var randomDelay = this.random.Next(0, 500); + + if (numberOfAttempts == 0) + { + return TimeSpan.FromMilliseconds(this.settings.RetryInterval.TotalMilliseconds + randomDelay); + } + + var exponentialIncrease = Math.Pow(2, numberOfAttempts) * 1000; - var randomDelay = this.random.Next(0, 1000); + var actualIncrease = TimeSpan.FromMilliseconds(this.settings.RetryInterval.TotalMilliseconds + exponentialIncrease + randomDelay); - return TimeSpan.FromMilliseconds(interval + randomDelay); + return actualIncrease; } } } From cc033330cc18b8e0c5dadc0044b0f8345b38992c Mon Sep 17 00:00:00 2001 From: dylanm Date: Tue, 22 Aug 2017 09:46:10 +0100 Subject: [PATCH 14/17] Introduced min, max and delta concepts for calculating interval --- .../Reliability/ReliabilitySettings.cs | 60 +++++++++++++++---- .../Reliability/RetryDelegatingHandler.cs | 16 ++--- .../RetryDelegatingHandlerTests.cs | 2 +- 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/SendGrid/Reliability/ReliabilitySettings.cs b/src/SendGrid/Reliability/ReliabilitySettings.cs index 0a576eb30..32cf122f1 100644 --- a/src/SendGrid/Reliability/ReliabilitySettings.cs +++ b/src/SendGrid/Reliability/ReliabilitySettings.cs @@ -7,17 +7,21 @@ namespace SendGrid.Helpers.Reliability /// public class ReliabilitySettings { - private int retryCount; + private int maximumNumberOfRetries; - private TimeSpan retryInterval; + private TimeSpan minimumBackOff; + private TimeSpan maximumBackOff; + private TimeSpan deltaBackOff; /// /// Initializes a new instance of the class. /// public ReliabilitySettings() { - this.retryCount = 0; - this.retryInterval = TimeSpan.FromSeconds(1); + this.maximumNumberOfRetries = 0; + this.minimumBackOff = TimeSpan.FromSeconds(1); + this.deltaBackOff = TimeSpan.FromSeconds(1); + this.maximumBackOff = TimeSpan.FromSeconds(10); } /// @@ -27,7 +31,7 @@ public int MaximumNumberOfRetries { get { - return this.retryCount; + return this.maximumNumberOfRetries; } set @@ -42,28 +46,64 @@ public int MaximumNumberOfRetries throw new ArgumentException("The maximum number of retries that can be attempted is 5"); } - this.retryCount = value; + this.maximumNumberOfRetries = value; } } /// /// Gets or sets the interval between HTTP retries. Defaults to 1 second /// - public TimeSpan RetryInterval + public TimeSpan MinimumBackOff { get { - return this.retryInterval; + return this.minimumBackOff; } set { if (value.TotalSeconds > 30) { - throw new ArgumentException("The maximum retry interval is 30 seconds"); + throw new ArgumentException("The maximum setting for minimum back off is 30 seconds"); } - this.retryInterval = value; + this.minimumBackOff = value; + } + } + + public TimeSpan MaximumBackOff + { + get + { + return this.maximumBackOff; + } + + set + { + if (value.TotalSeconds > 30) + { + throw new ArgumentException("The maximum setting to back off for is 30 seconds"); + } + + this.maximumBackOff = value; + } + } + + public TimeSpan DeltaBackOff + { + get + { + return this.deltaBackOff; + } + + set + { + if (value.TotalSeconds > 30) + { + throw new ArgumentException("The maximum delta interval is 5 seconds"); + } + + this.deltaBackOff = value; } } } diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index 2080e1cf3..a295150e7 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -107,18 +107,14 @@ private static void ThrowHttpRequestExceptionIfResponseCodeCanBeRetried(HttpResp private TimeSpan GetNextWaitInterval(int numberOfAttempts) { - var randomDelay = this.random.Next(0, 500); + var delta = (int)((Math.Pow(2.0, numberOfAttempts) - 1.0) * + this.random.Next( + (int)(this.settings.DeltaBackOff.TotalMilliseconds * 0.8), + (int)(this.settings.DeltaBackOff.TotalMilliseconds * 1.2))); - if (numberOfAttempts == 0) - { - return TimeSpan.FromMilliseconds(this.settings.RetryInterval.TotalMilliseconds + randomDelay); - } - - var exponentialIncrease = Math.Pow(2, numberOfAttempts) * 1000; - - var actualIncrease = TimeSpan.FromMilliseconds(this.settings.RetryInterval.TotalMilliseconds + exponentialIncrease + randomDelay); + var interval = (int)Math.Min(this.settings.MinimumBackOff.TotalMilliseconds + delta, this.settings.MaximumBackOff.TotalMilliseconds); - return actualIncrease; + return TimeSpan.FromMilliseconds(interval); } } } diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index 2bbcc0aa1..d37d876ee 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -135,7 +135,7 @@ public void ReliabilitySettingsShouldNotAllowRetryIntervalGreaterThan30Seconds() { var settings = new ReliabilitySettings(); - Assert.Throws(() => settings.RetryInterval = TimeSpan.FromSeconds(31)); + Assert.Throws(() => settings.MinimumBackOff = TimeSpan.FromSeconds(31)); } } } From d0e09040af8c6fdcdb1458ae272d2773cf5bc20b Mon Sep 17 00:00:00 2001 From: dylanm Date: Tue, 22 Aug 2017 09:53:13 +0100 Subject: [PATCH 15/17] Made reliabilty settings immutable, refactored associated code --- USE_CASES.md | 23 ++-- .../Reliability/ReliabilitySettings.cs | 114 +++++++----------- src/SendGrid/SendGridClientOptions.cs | 13 +- tests/SendGrid.Tests/Integration.cs | 4 +- .../Reliability/ReliabilitySettingsTests.cs | 106 ++++++++++++++++ .../RetryDelegatingHandlerTests.cs | 31 +---- 6 files changed, 183 insertions(+), 108 deletions(-) create mode 100644 tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs diff --git a/USE_CASES.md b/USE_CASES.md index c5ed81124..b86e7cc93 100644 --- a/USE_CASES.md +++ b/USE_CASES.md @@ -587,28 +587,37 @@ namespace Example # Transient Fault Handling -The SendGridClient provides functionality for handling transient errors that might occur when sending an HttpRequest. This includes client side timeouts while sending the mail, or any errors returned within the 500 range. +The SendGridClient provides functionality for handling transient errors that might occur when sending an HttpRequest. This includes client side timeouts while sending the mail, or certain errors returned within the 500 range. Errors within the 500 range are limited to 500 Internal Server Error, 502 Bad Gateway, 503 Service unavailable and 504 Gateway timeout. By default, retry behaviour is off, you must explicitly enable it by setting the retry count to a value greater than zero. To set the retry count, you must use the SendGridClient construct that takes a **SendGridClientOptions** object, allowing you to configure the **ReliabilitySettings** ### RetryCount -The amount of times to retry the operation before reporting an exception to the caller. This is in addition to the initial attempt, so setting a value of 1 would result in 2 attempts - the initial attempt and the retry. Defaults to zero, so retry behaviour is not enabled. The maximum amount of retries permitted is 5. +The amount of times to retry the operation before reporting an exception to the caller. This is in addition to the initial attempt so setting a value of 1 would result in 2 attempts, the initial attempt and the retry. Defaults to zero, retry behaviour is not enabled. The maximum amount of retries permitted is 5. -### RetryInterval +### MinimumBackOff + +The minimum amount of time to wait between retries. + +### MaximumBackOff + +The maximum possible amount of time to wait between retries. The maximum value allowed is 30 seconds + +### DeltaBackOff + +The value that will be used to calculate a random delta in the exponential delay between retries. A random element of time is factored into the delta calculation as this helps avoid many clients retrying at regular intervals. -The policy implemented is a 'wait and retry'. The RetryInterval setting defines the amount of time to wait between operations that fail before attempting a retry. By default, this is set to 1 second - the maximum amount of time allowed is 30 seconds. ## Examples -In this example, we are setting RetryCount to 1, with an interval between attempts of 5 seconds. This means that if the inital attempt to send mail fails, then it will wait 5 seconds then try again a single time. +In this example we are setting RetryCount to 2, with a mimimum wait time of 1 seconds, a maximum of 10 seconds and a delta of 3 seconds ```csharp var options = new SendGridClientOptions { ApiKey = "Your-Api-Key", - ReliabilitySettings = {RetryCount = 1, RetryInterval = TimeSpan.FromSeconds(5)} + ReliabilitySettings = new ReliabilitySettings(2, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(3)) }; var client = new SendGridClient(options); @@ -622,7 +631,7 @@ The SendGridClientOptions object defines all the settings that can be set for th var options = new SendGridClientOptions { ApiKey = "Your-Api-Key", - ReliabilitySettings = {RetryCount = 1, RetryInterval = TimeSpan.FromSeconds(5)}, + ReliabilitySettings = new ReliabilitySettings(2, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(3)), Host = "Your-Host", UrlPath = "Url-Path", Version = "3", diff --git a/src/SendGrid/Reliability/ReliabilitySettings.cs b/src/SendGrid/Reliability/ReliabilitySettings.cs index 32cf122f1..d3316f936 100644 --- a/src/SendGrid/Reliability/ReliabilitySettings.cs +++ b/src/SendGrid/Reliability/ReliabilitySettings.cs @@ -7,104 +7,82 @@ namespace SendGrid.Helpers.Reliability /// public class ReliabilitySettings { - private int maximumNumberOfRetries; - - private TimeSpan minimumBackOff; - private TimeSpan maximumBackOff; - private TimeSpan deltaBackOff; - /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class with default settings. /// public ReliabilitySettings() + : this(0, TimeSpan.Zero, TimeSpan.Zero, TimeSpan.Zero) { - this.maximumNumberOfRetries = 0; - this.minimumBackOff = TimeSpan.FromSeconds(1); - this.deltaBackOff = TimeSpan.FromSeconds(1); - this.maximumBackOff = TimeSpan.FromSeconds(10); } /// - /// Gets or sets the maximum number of retries to execute against when sending an HTTP Request before throwing an exception. Defaults to 0 (no retries, you must explicitly enable) + /// Initializes a new instance of the class. /// - public int MaximumNumberOfRetries + /// The maximum number of retries to execute against when sending an HTTP Request before throwing an exception + /// The minimum amount of time to wait between between HTTP retries + /// the maximum amount of time to wait between between HTTP retries + /// the value that will be used to calculate a random delta in the exponential delay between retries + public ReliabilitySettings(int maximumNumberOfRetries, TimeSpan minimumBackoff, TimeSpan maximumBackOff, TimeSpan deltaBackOff) { - get + if (maximumNumberOfRetries < 0) { - return this.maximumNumberOfRetries; + throw new ArgumentOutOfRangeException(nameof(maximumNumberOfRetries), "maximumNumberOfRetries must be greater than 0"); } - set + if (maximumNumberOfRetries > 5) { - if (value < 0) - { - throw new ArgumentException("Retry count must be greater than zero"); - } - - if (value > 5) - { - throw new ArgumentException("The maximum number of retries that can be attempted is 5"); - } - - this.maximumNumberOfRetries = value; + throw new ArgumentOutOfRangeException(nameof(maximumNumberOfRetries), "The maximum number of retries allowed is 5"); } - } - /// - /// Gets or sets the interval between HTTP retries. Defaults to 1 second - /// - public TimeSpan MinimumBackOff - { - get + if (minimumBackoff.Ticks < 0) { - return this.minimumBackOff; + throw new ArgumentOutOfRangeException(nameof(minimumBackoff), "minimumBackoff must be greater than 0"); } - set + if (maximumBackOff.Ticks < 0) { - if (value.TotalSeconds > 30) - { - throw new ArgumentException("The maximum setting for minimum back off is 30 seconds"); - } - - this.minimumBackOff = value; + throw new ArgumentOutOfRangeException(nameof(maximumBackOff), "maximumBackOff must be greater than 0"); } - } - public TimeSpan MaximumBackOff - { - get + if (maximumBackOff.TotalSeconds > 30) { - return this.maximumBackOff; + throw new ArgumentOutOfRangeException(nameof(maximumBackOff), "maximumBackOff must be less than 30 seconds"); } - set + if (deltaBackOff.Ticks < 0) { - if (value.TotalSeconds > 30) - { - throw new ArgumentException("The maximum setting to back off for is 30 seconds"); - } - - this.maximumBackOff = value; + throw new ArgumentOutOfRangeException(nameof(deltaBackOff), "deltaBackOff must be greater than 0"); } - } - public TimeSpan DeltaBackOff - { - get + if (minimumBackoff.TotalMilliseconds > maximumBackOff.TotalMilliseconds) { - return this.deltaBackOff; + throw new ArgumentOutOfRangeException(nameof(minimumBackoff), "minimumBackoff must be less than maximumBackOff"); } - set - { - if (value.TotalSeconds > 30) - { - throw new ArgumentException("The maximum delta interval is 5 seconds"); - } - - this.deltaBackOff = value; - } + this.MaximumNumberOfRetries = maximumNumberOfRetries; + this.MinimumBackOff = minimumBackoff; + this.DeltaBackOff = deltaBackOff; + this.MaximumBackOff = maximumBackOff; } + + /// + /// Gets the maximum number of retries to execute against when sending an HTTP Request before throwing an exception. Defaults to 0 (no retries, you must explicitly enable) + /// + public int MaximumNumberOfRetries { get; } + + /// + /// Gets the minimum amount of time to wait between between HTTP retries. Defaults to 1 second + /// + public TimeSpan MinimumBackOff { get; } + + /// + /// Gets the maximum amount of time to wait between between HTTP retries. Defaults to 10 seconds + /// + public TimeSpan MaximumBackOff { get; } + + /// + /// Gets the value that will be used to calculate a random delta in the exponential delay between retries. Defaults to 1 second + /// + public TimeSpan DeltaBackOff { get; } } } diff --git a/src/SendGrid/SendGridClientOptions.cs b/src/SendGrid/SendGridClientOptions.cs index 02a354212..1d830c700 100644 --- a/src/SendGrid/SendGridClientOptions.cs +++ b/src/SendGrid/SendGridClientOptions.cs @@ -1,5 +1,6 @@ namespace SendGrid { + using System; using System.Collections.Generic; using SendGrid.Helpers.Reliability; @@ -8,21 +9,27 @@ /// public class SendGridClientOptions { + private ReliabilitySettings reliabilitySettings = new ReliabilitySettings(); + /// /// Initializes a new instance of the class. /// public SendGridClientOptions() { - ReliabilitySettings = new ReliabilitySettings(); RequestHeaders = new Dictionary(); Host = "https://api.sendgrid.com"; Version = "v3"; } /// - /// Gets the reliability settings to use on HTTP Requests + /// Gets or sets the reliability settings to use on HTTP Requests /// - public ReliabilitySettings ReliabilitySettings { get; } + public ReliabilitySettings ReliabilitySettings + { + get => this.reliabilitySettings; + + set => this.reliabilitySettings = value ?? throw new ArgumentNullException(nameof(value)); + } /// /// Gets or sets the SendGrid API key diff --git a/tests/SendGrid.Tests/Integration.cs b/tests/SendGrid.Tests/Integration.cs index 1a1c03a08..0791ed31d 100644 --- a/tests/SendGrid.Tests/Integration.cs +++ b/tests/SendGrid.Tests/Integration.cs @@ -6141,7 +6141,7 @@ public async Task TestRetryBehaviourThrowsTimeoutException() var options = new SendGridClientOptions { ApiKey = fixture.apiKey, - ReliabilitySettings = { MaximumNumberOfRetries = 1 }, + ReliabilitySettings = new ReliabilitySettings(1, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(1)), Host = "http://localhost:4010" }; @@ -6169,7 +6169,7 @@ public async Task TestRetryBehaviourSucceedsOnSecondAttempt() var options = new SendGridClientOptions { ApiKey = fixture.apiKey, - ReliabilitySettings = { MaximumNumberOfRetries = 1 } + ReliabilitySettings = new ReliabilitySettings(1, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(1)) }; var id = "test_url_param"; diff --git a/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs b/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs new file mode 100644 index 000000000..0811d755d --- /dev/null +++ b/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs @@ -0,0 +1,106 @@ +namespace SendGrid.Tests.Reliability +{ + using System; + + using Helpers.Reliability; + using Xunit; + + public class ReliabilitySettingsTests + { + [Fact] + public void ShouldNotAllowNegativeRetryCount() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(-1, + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(1))); + + Assert.Contains("maximumNumberOfRetries must be greater than 0", exception.Message); + } + + [Fact] + public void ShouldNotAllowNegativeMinimumBackoffTime() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(1, + TimeSpan.FromSeconds(-1), + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(1))); + + Assert.Contains("minimumBackoff must be greater than 0", exception.Message); + } + + [Fact] + public void ShouldNotAllowNegativeMaximumBackoffTime() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(1, + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(-11), + TimeSpan.FromSeconds(1))); + + Assert.Contains("maximumBackOff must be greater than 0", exception.Message); + } + + [Fact] + public void ShouldNotAllowNegativeDeltaBackoffTime() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(1, + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(-1))); + + Assert.Contains("deltaBackOff must be greater than 0", exception.Message); + } + + [Fact] + public void ShouldNotAllowRetryCountGreaterThan5() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(6, + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(1))); + + Assert.Contains("The maximum number of retries allowed is 5", exception.Message); + } + + [Fact] + public void ShouldNotAllowMinimumBackOffGreaterThanMaximumBackoff() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(1, + TimeSpan.FromSeconds(11), + TimeSpan.FromSeconds(10), + TimeSpan.FromSeconds(1))); + + Assert.Contains("minimumBackoff must be less than maximumBackOff", exception.Message); + } + + [Fact] + public void ShouldNotAllowMaximumBackOffGreaterThan30Seconds() + { + var exception = Assert.Throws(() => + new ReliabilitySettings(1, + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(31), + TimeSpan.FromSeconds(1))); + + Assert.Contains("maximumBackOff must be less than 30 seconds", exception.Message); + } + + [Fact] + public void ShouldPassValidValuesFromDefaultConstruct() + { + var defaultSettings = new ReliabilitySettings(); + + Assert.Equal(TimeSpan.Zero, defaultSettings.MaximumBackOff); + Assert.Equal(TimeSpan.Zero, defaultSettings.MinimumBackOff); + Assert.Equal(TimeSpan.Zero, defaultSettings.DeltaBackOff); + Assert.Equal(0, defaultSettings.MaximumNumberOfRetries); + } + } +} + diff --git a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs index d37d876ee..f8d026427 100644 --- a/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs +++ b/tests/SendGrid.Tests/Reliability/RetryDelegatingHandlerTests.cs @@ -15,10 +15,9 @@ public class RetryDelegatingHandlerTests public RetryDelegatingHandlerTests() { - var reliabilitySettings = new ReliabilitySettings - { - MaximumNumberOfRetries = 1 - }; + var reliabilitySettings = new ReliabilitySettings(1, TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(10), + TimeSpan.FromSeconds(1)); + innerHandler = new RetryTestBehaviourDelegatingHandler(); client = new HttpClient(new RetryDelegatingHandler(innerHandler, reliabilitySettings)) { @@ -113,29 +112,5 @@ public async Task ShouldRetryTheExpectedAmountOfTimesAndReturnExceptionWhenInter Assert.Equal(2, innerHandler.InvocationCount); } - - [Fact] - public void ReliabilitySettingsShouldNotAllowNegativeRetryCount() - { - var settings = new ReliabilitySettings(); - - Assert.Throws(() => settings.MaximumNumberOfRetries = -1); - } - - [Fact] - public void ReliabilitySettingsShouldNotAllowRetryCountGreaterThan5() - { - var settings = new ReliabilitySettings(); - - Assert.Throws(() => settings.MaximumNumberOfRetries = 6); - } - - [Fact] - public void ReliabilitySettingsShouldNotAllowRetryIntervalGreaterThan30Seconds() - { - var settings = new ReliabilitySettings(); - - Assert.Throws(() => settings.MinimumBackOff = TimeSpan.FromSeconds(31)); - } } } From 5fef75020ab56732934c7f7b54a2d6c2f9266ba9 Mon Sep 17 00:00:00 2001 From: dylanm Date: Tue, 22 Aug 2017 13:17:58 +0100 Subject: [PATCH 16/17] Test for null settings sent to client options --- .../Reliability/ReliabilitySettingsTests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs b/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs index 0811d755d..dad52ce78 100644 --- a/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs +++ b/tests/SendGrid.Tests/Reliability/ReliabilitySettingsTests.cs @@ -101,6 +101,14 @@ public void ShouldPassValidValuesFromDefaultConstruct() Assert.Equal(TimeSpan.Zero, defaultSettings.DeltaBackOff); Assert.Equal(0, defaultSettings.MaximumNumberOfRetries); } + + [Fact] + public void ShouldNotAllowNullInstanceOnSendGridClientOptions() + { + var options = new SendGridClientOptions(); + + Assert.Throws(() => options.ReliabilitySettings = null); + } } } From a3312a44d47575235c16bd7b397aa56cbddc6109 Mon Sep 17 00:00:00 2001 From: dylanm Date: Tue, 22 Aug 2017 18:14:29 +0100 Subject: [PATCH 17/17] Made random locally scoped to GetNextWaitInterval method --- src/SendGrid/Reliability/RetryDelegatingHandler.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SendGrid/Reliability/RetryDelegatingHandler.cs b/src/SendGrid/Reliability/RetryDelegatingHandler.cs index a295150e7..222701dfa 100644 --- a/src/SendGrid/Reliability/RetryDelegatingHandler.cs +++ b/src/SendGrid/Reliability/RetryDelegatingHandler.cs @@ -23,8 +23,6 @@ public class RetryDelegatingHandler : DelegatingHandler private readonly ReliabilitySettings settings; - private readonly Random random = new Random(); - /// /// Initializes a new instance of the class. /// @@ -107,8 +105,10 @@ private static void ThrowHttpRequestExceptionIfResponseCodeCanBeRetried(HttpResp private TimeSpan GetNextWaitInterval(int numberOfAttempts) { + var random = new Random(); + var delta = (int)((Math.Pow(2.0, numberOfAttempts) - 1.0) * - this.random.Next( + random.Next( (int)(this.settings.DeltaBackOff.TotalMilliseconds * 0.8), (int)(this.settings.DeltaBackOff.TotalMilliseconds * 1.2)));