Refactor HandlerOptions with defaults and exception handling

Author: peombwa

src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs

@@ -22,7 +22,7 @@ internal static class Codes
 
             internal static string TooManyRetries = "tooManyRetries";
 
-           
+            internal static string MaximumValueExceeded = "MaximumValueExceeded";
         }
 
         internal static class Messages
@@ -53,6 +53,7 @@ internal static class Messages
 
             internal static string UnexpectedExceptionResponse = "Unexpected exception returned from the service.";
 
+            internal static string MaximumValueExceeded = "{0} exceeds the maximum value of {1}.";
         }
     }
 }

src/Microsoft.Graph.Core/Extensions/BaseRequestExtensions.cs

@@ -62,15 +62,15 @@ public static T WithPerRequestAuthProvider<T>(this T baseRequest) where T : IBas
         }
 
         /// <summary>
-        /// Sets a ShouldRetry <see cref="Func{HttpResponseMessage, Boolean}"/> delegate to the default Retry Middleware Handler for this request.
+        /// Sets a ShouldRetry <see cref="Func{HttpResponseMessage httpResponseMessage, Boolean}"/> delegate to the default Retry Middleware Handler for this request.
         /// This only works with the default Retry Middleware Handler.
         /// If you use a custom Retry Middleware Handler, you have to handle it's retreival in your implementation.
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="baseRequest">The <see cref="BaseRequest"/> for the request.</param>
         /// <param name="shouldRetry">A <see cref="Func{HttpResponseMessage, Boolean}"/> for the request.</param>
         /// <returns></returns>
-        public static T WithShouldRetry<T>(this T baseRequest, Func<HttpResponseMessage, bool> shouldRetry) where T : IBaseRequest
+        public static T WithShouldRetry<T>(this T baseRequest, Func<int, int, HttpResponseMessage, bool> shouldRetry) where T : IBaseRequest
         {
             string retryOptionKey = typeof(RetryHandlerOption).ToString();
             if (baseRequest.MiddlewareOptions.ContainsKey(retryOptionKey))
@@ -121,11 +121,11 @@ public static T WithMaxRedirects<T>(this T baseRequest, int maxRedirects) where
             string redirectOptionKey = typeof(RedirectHandlerOption).ToString();
             if (baseRequest.MiddlewareOptions.ContainsKey(redirectOptionKey))
             {
-                (baseRequest.MiddlewareOptions[redirectOptionKey] as RedirectHandlerOption).MaxRedirects = maxRedirects;
+                (baseRequest.MiddlewareOptions[redirectOptionKey] as RedirectHandlerOption).MaxRedirect = maxRedirects;
             }
             else
             {
-                baseRequest.MiddlewareOptions.Add(redirectOptionKey, new RedirectHandlerOption { MaxRedirects = maxRedirects });
+                baseRequest.MiddlewareOptions.Add(redirectOptionKey, new RedirectHandlerOption { MaxRedirect = maxRedirects });
             }
             return baseRequest;
         }

src/Microsoft.Graph.Core/Requests/Handlers/AuthenticationHandler.cs renamed to src/Microsoft.Graph.Core/Requests/Middleware/AuthenticationHandler.cs

@@ -10,13 +10,13 @@ namespace Microsoft.Graph
     public class AuthenticationHandlerOption : IMiddlewareOption
     {
         /// <summary>
-        /// An Authentication Provider
+        /// An authentication provider
         /// </summary>
         internal IAuthenticationProvider AuthenticationProvider { get; set; }
 
         /// <summary>
-        /// An auth provider option property
+        /// An authentication provider option.
         /// </summary>
-        public IAuthProviderOption AuthProviderOption { get; set; }
+        public IAuthenticationProviderOption AuthenticationProviderOption { get; set; }
     }
 }

src/Microsoft.Graph.Core/Requests/MiddlewareOptions/AuthenticationHandlerOption.cs renamed to src/Microsoft.Graph.Core/Requests/Middleware/Options/AuthenticationHandlerOption.cs

@@ -8,7 +8,7 @@ namespace Microsoft.Graph
     /// An interface used to pass auth provider options in a request.
     /// Auth providers will be incharge of implementing this interface and providing <see cref="IBaseRequest"/> extensions to set it's values.
     /// </summary>
-    public interface IAuthProviderOption
+    public interface IAuthenticationProviderOption
     {
         /// <summary>
         /// Microsoft Graph scopes property.

src/Microsoft.Graph.Core/Requests/MiddlewareOptions/IAuthProviderOption.cs renamed to src/Microsoft.Graph.Core/Requests/Middleware/Options/IAuthProviderOption.cs

@@ -0,0 +1,54 @@
+// ------------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the MIT License.  See License in the project root for license information.
+// ------------------------------------------------------------------------------
+
+using System;
+using System.Net.Http;
+
+namespace Microsoft.Graph
+{
+    /// <summary>
+    /// The redirect middleware option class
+    /// </summary>
+    public class RedirectHandlerOption : IMiddlewareOption
+    {
+        internal const int DEFAULT_MAX_REDIRECT = 5;
+        internal const int MAX_MAX_REDIRECT = 20;
+        /// <summary>
+        /// Constructs a new <see cref="RedirectHandlerOption"/>
+        /// </summary>
+        public RedirectHandlerOption()
+        {
+
+        }
+
+
+        private int _maxRedirect = DEFAULT_MAX_REDIRECT;
+
+        /// <summary>
+        /// The maximum number of redirects with a maximum value of 20. This defaults to 5 redirects.
+        /// </summary>
+        public int MaxRedirect
+        {
+            get { return _maxRedirect; }
+            set
+            {
+                if (value > MAX_MAX_REDIRECT)
+                {
+                    throw new ServiceException(
+                        new Error
+                        {
+                            Code = ErrorConstants.Codes.MaximumValueExceeded,
+                            Message = string.Format(ErrorConstants.Messages.MaximumValueExceeded, "MaxRedirect", MAX_MAX_REDIRECT)
+                        });
+                }
+                _maxRedirect = value;
+            }
+        }
+
+        /// <summary>
+        /// A delegate that's called to determine whether a response should be redirected or not. The delegate method should accept <see cref="HttpResponseMessage"/> as it's parameter and return a <see cref="bool"/>. This defaults to true.
+        /// </summary>
+        public Func<HttpResponseMessage, bool> ShouldRedirect { get; set; } = (response) => true;
+    }
+}

src/Microsoft.Graph.Core/Requests/MiddlewareOptions/IMiddlewareOption.cs renamed to src/Microsoft.Graph.Core/Requests/Middleware/Options/IMiddlewareOption.cs

@@ -0,0 +1,81 @@
+// ------------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the MIT License.  See License in the project root for license information.
+// ------------------------------------------------------------------------------
+
+namespace Microsoft.Graph
+{
+    using System;
+    using System.Net.Http;
+
+    /// <summary>
+    /// The retry middleware option class
+    /// </summary>
+    public class RetryHandlerOption : IMiddlewareOption
+    {
+        internal const int DEFAULT_DELAY = 3;
+        internal const int DEFAULT_MAX_RETRY = 3;
+        internal const int MAX_MAX_RETRY = 10;
+        internal const int MAX_DELAY = 180;
+
+        /// <summary>
+        /// Constructs a new <see cref="RetryHandlerOption"/>
+        /// </summary>
+        public RetryHandlerOption()
+        {
+        }
+
+        private int _delay = DEFAULT_DELAY;
+        /// <summary>
+        /// The waiting time in seconds before retrying a request with a maximum value of 180 seconds. This defaults to 3 seconds.
+        /// </summary>
+        public int Delay
+        {
+            get { return _delay; }
+            set
+            {
+                if (value > MAX_DELAY)
+                {
+                    throw new ServiceException(
+                        new Error
+                        {
+                            Code = ErrorConstants.Codes.MaximumValueExceeded,
+                            Message = string.Format(ErrorConstants.Messages.MaximumValueExceeded, "Delay", MAX_DELAY)
+                        });
+                }
+
+                _delay = value;
+            }
+        }
+
+        private int _maxRetry = DEFAULT_MAX_RETRY;
+        /// <summary>
+        /// The maximum number of retries for a request with a maximum value of 10. This defaults to 3.
+        /// </summary>
+        public int MaxRetry
+        {
+            get
+            {
+                return _maxRetry;
+            }
+            set
+            {
+                if (value > MAX_MAX_RETRY)
+                {
+                    throw new ServiceException(
+                        new Error
+                        {
+                            Code = ErrorConstants.Codes.MaximumValueExceeded,
+                            Message = string.Format(ErrorConstants.Messages.MaximumValueExceeded, "MaxRetry", MAX_MAX_RETRY)
+                        });
+                }
+                _maxRetry = value;
+            }
+        }
+
+        /// <summary>
+        /// A delegate that's called to determine whether a request should be retried or not.
+        /// The delegate method should accept a delay time in seconds of, number of retry attempts and <see cref="HttpResponseMessage"/> as it's parameters and return a <see cref="bool"/>. This defaults to true
+        /// </summary>
+        public Func<int, int, HttpResponseMessage, bool> ShouldRetry { get; set; } = (delay, attempt, response) => true;
+    }
+}

src/Microsoft.Graph.Core/Requests/Middleware/Options/RedirectHandlerOption.cs

@@ -53,8 +53,8 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
             // send request first time to get response
             var response = await base.SendAsync(request, cancellationToken);
 
-            // check response status code 
-            if (IsRedirect(response.StatusCode))
+            // check response status code and redirect handler option
+            if (IsRedirect(response.StatusCode) && RedirectOption.ShouldRedirect(response) && RedirectOption.MaxRedirect > 0)
             {
                 if (response.Headers.Location == null)
                 {
@@ -68,7 +68,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 
                 var redirectCount = 0;
 
-                while (redirectCount < RedirectOption.MaxRedirects)
+                while (redirectCount < RedirectOption.MaxRedirect)
                 {
                     // general clone request with internal CloneAsync (see CloneAsync for details) extension method 
                     var newRequest = await response.RequestMessage.CloneAsync();
@@ -100,6 +100,7 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                     }
                     redirectCount++;
                 }
+
                 throw new ServiceException(
                         new Error
                         {

src/Microsoft.Graph.Core/Requests/Middleware/Options/RetryHandlerOption.cs

@@ -60,7 +60,7 @@ protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage
 
             var response = await base.SendAsync(httpRequest, cancellationToken);
 
-            if (RetryOption.ShouldRetry(response) && httpRequest.IsBuffered())
+            if(IsRetry(response) && httpRequest.IsBuffered() && RetryOption.MaxRetry > 0 && RetryOption.ShouldRetry(RetryOption.Delay, 0, response))
             {
                 response = await SendRetryAsync(response, cancellationToken);
             }
@@ -74,13 +74,13 @@ protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage
         /// <param name="response">The <see cref="HttpResponseMessage"/> which is returned and includes the HTTP request needs to be retried.</param>
         /// <param name="cancellationToken">The <see cref="CancellationToken"/> for the retry.</param>
         /// <returns></returns>
-        public async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage response, CancellationToken cancellationToken)
+        private async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage response, CancellationToken cancellationToken)
         {
             int retryCount = 0;
             while (retryCount < RetryOption.MaxRetry)
             {
                 // Call Delay method to get delay time from response's Retry-After header or by exponential backoff 
-                Task delay = Delay(response, retryCount, cancellationToken);
+                Task delay = Delay(response, retryCount, RetryOption.Delay, cancellationToken);
 
                 // general clone request with internal CloneAsync (see CloneAsync for details) extension method 
                 var request = await response.RequestMessage.CloneAsync();
@@ -95,11 +95,10 @@ public async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage respon
                 // Call base.SendAsync to send the request
                 response = await base.SendAsync(request, cancellationToken);
 
-                if (!RetryOption.ShouldRetry(response) || !request.IsBuffered())
+                if (!IsRetry(response) || !request.IsBuffered() || !RetryOption.ShouldRetry(RetryOption.Delay, retryCount, response))
                 {
                     return response;
                 }
-
             }
             throw new ServiceException(
                          new Error
@@ -129,34 +128,47 @@ private void AddOrUpdateRetryAttempt(HttpRequestMessage request, int retry_count
         /// </summary>
         /// <param name="response">The <see cref="HttpResponseMessage"/>returned.</param>
         /// <param name="retry_count">The retry counts</param>
+        /// <param name="delay">Delay value in seconds.</param>
         /// <param name="cancellationToken">The cancellationToken for the Http request</param>
         /// <returns>The <see cref="Task"/> for delay operation.</returns>
-        public Task Delay(HttpResponseMessage response, int retry_count, CancellationToken cancellationToken)
+        public Task Delay(HttpResponseMessage response, int retry_count, int delay, CancellationToken cancellationToken)
         {
-            
-            TimeSpan delay = TimeSpan.FromMilliseconds(0);
+            //TimeSpan delayTimeSpan = TimeSpan.FromMilliseconds(0);
             HttpHeaders headers = response.Headers;
+            double delayInSeconds = RetryOption.Delay;
             if (headers.TryGetValues(RETRY_AFTER, out IEnumerable<string> values))
             {
                 string retry_after = values.First();    
                 if (Int32.TryParse(retry_after, out int delay_seconds))
                 {
-                    delay = TimeSpan.FromSeconds(delay_seconds);
+                    delayInSeconds = delay_seconds;
                 }
             }
             else
             {
+                m_pow = Math.Pow(2, retry_count);
+                delayInSeconds = m_pow * RetryOption.Delay;
+            }
 
-                m_pow = Math.Pow(2, retry_count); // m_pow = Pow(2, retry_count)
+            TimeSpan delayTimeSpan = TimeSpan.FromSeconds(Math.Min(delayInSeconds, RetryHandlerOption.MAX_DELAY));
 
-                double delay_time = m_pow * DELAY_MILLISECONDS;
-              
-                delay = TimeSpan.FromMilliseconds(delay_time);
-            }
-            return Task.Delay(delay, cancellationToken);
+            return Task.Delay(delayTimeSpan, cancellationToken);
 
         }
 
-
+        /// <summary>
+        /// Check the HTTP response's status to determine whether it should be retried or not.
+        /// </summary>
+        /// <param name="response">The <see cref="HttpResponseMessage"/>returned.</param>
+        /// <returns></returns>
+        private bool IsRetry(HttpResponseMessage response)
+        {
+            if ((response.StatusCode == HttpStatusCode.ServiceUnavailable ||
+                response.StatusCode == (HttpStatusCode)429))
+            {
+                return true;
+            }
+            return false;
+        }
     }
 }