Merge of #650. Thanks @patrikhellgren

pnp · Nov 23, 2021 · 1501583 · 1501583
2 parents c136924 + d97279c
commit 1501583
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 42 deletions.
diff --git a/src/sdk/CHANGELOG.md b/src/sdk/CHANGELOG.md
@@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fix: multi value lookup only persisted first lookup value #628 [jansenbe - Bert Jansen]
 - Fix: LINQ queries on the sub webs (webs) now work ~ improved logic that chooses between REST and Graph #631 [jansenbe - Bert Jansen]
 - Http clients and retry handlers never got their logger assigned, fixed now so that for example throttling will show in logs [jansenbe - Bert Jansen]
+- Increased reliability via retry on SocketException in SendAsync #650 [patrikhellgren - Patrik Hellgren]
 
 ## [1.4.0]
 

diff --git a/src/sdk/PnP.Core/Services/Core/Http/RetryHandlerBase.cs b/src/sdk/PnP.Core/Services/Core/Http/RetryHandlerBase.cs
@@ -4,7 +4,7 @@
 using System.Linq;
 using System.Net;
 using System.Net.Http;
-using System.Net.Http.Headers;
+using System.Net.Sockets;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -38,38 +38,69 @@ public RetryHandlerBase(ILogger<RetryHandlerBase> log, PnPGlobalSettingsOptions
         internal int DelayInSeconds { get; set; } = 3;
         internal bool IncrementalDelay { get; set; } = true;
 
-        protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
-            var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
+            int retryCount = 0;
 
-            if (ShouldRetry(response.StatusCode))
+            while (true)
             {
-                if (GlobalSettings != null && GlobalSettings.Logger != null)
-                {
-                    GlobalSettings.Logger.LogInformation($"Retrying request {request.RequestUri} due to status code {response.StatusCode}");
-                }
+                HttpResponseMessage response = null;
 
-                // Handle retry handling
-                response = await SendRetryAsync(response, cancellationToken).ConfigureAwait(false);
-            }
-
-            return response;
-        }
+                try
+                {
+                    response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
+
+                    if (!ShouldRetry(response.StatusCode))
+                    {
+                        return response;
+                    }
+
+                    if (retryCount >= MaxRetries)
+                    {
+                        // Drain response content to free connections. Need to perform this
+                        // before retry attempt and before the TooManyRetries ServiceException.
+                        if (response.Content != null)
+                        {
+#if NET5_0_OR_GREATER
+                            await response.Content.ReadAsByteArrayAsync(cancellationToken).ConfigureAwait(false);
+#else
+                            await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
+#endif
+                        }
 
+                        throw new ServiceException(ErrorType.TooManyRetries, (int)response.StatusCode,
+                            string.Format(PnPCoreResources.Exception_ServiceException_MaxRetries, retryCount));
+                    }
 
-        /// <summary>
-        /// Retry sending the HTTP request 
-        /// </summary>
-        /// <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>
-        private async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage response, CancellationToken cancellationToken)
-        {
-            int retryCount = 0;
+                    if (GlobalSettings != null && GlobalSettings.Logger != null)
+                    {
+                        GlobalSettings.Logger.LogInformation($"Retrying request {request.RequestUri} due to status code {response.StatusCode}");
+                    }
+                }
+                catch (Exception ex)
+                {
+                    // Find innermost exception and check if it is a SocketException
+                    Exception innermostEx = ex;
+
+                    while (innermostEx.InnerException != null) innermostEx = innermostEx.InnerException;
+                    if (!(innermostEx is SocketException))
+                    {
+                        throw;
+                    }
+
+                    if (retryCount >= MaxRetries)
+                    {
+                        throw;
+                    }
+
+                    if (GlobalSettings != null && GlobalSettings.Logger != null)
+                    {
+                        GlobalSettings.Logger.LogInformation($"Retrying request {request.RequestUri} due to exception {innermostEx.GetType()}: {innermostEx.Message}");
+                    }
+                }
 
-            while (retryCount < MaxRetries)
-            {
-                // Drain response content to free responses.
+                // Drain response content to free connections. Need to perform this
+                // before retry attempt and before the TooManyRetries ServiceException.
                 if (response.Content != null)
                 {
 #if NET5_0_OR_GREATER
@@ -84,27 +115,14 @@ private async Task<HttpResponseMessage> SendRetryAsync(HttpResponseMessage respo
 
                 // general clone request with internal CloneAsync (see CloneAsync for details) extension method 
                 // do not dispose this request as that breaks the request cloning
-                var request = await response.RequestMessage.CloneAsync().ConfigureAwait(false);
-
+                request = await request.CloneAsync().ConfigureAwait(false);
                 // Increase retryCount and then update Retry-Attempt in request header if needed
                 retryCount++;
                 AddOrUpdateRetryAttempt(request, retryCount);
 
                 // Delay time
                 await delay.ConfigureAwait(false);
-
-                // Call base.SendAsync to send the request
-                response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
-
-                if (!ShouldRetry(response.StatusCode))
-                {
-                    return response;
-                }
-
             }
-
-            throw new ServiceException(ErrorType.TooManyRetries, (int)response.StatusCode,
-                string.Format(PnPCoreResources.Exception_ServiceException_MaxRetries, retryCount));
         }
 
         /// <summary>
@@ -126,10 +144,9 @@ private void AddOrUpdateRetryAttempt(HttpRequestMessage request, int retryCount)
 
         private Task Delay(HttpResponseMessage response, int retryCount, int delay, CancellationToken cancellationToken)
         {
-            HttpHeaders headers = response.Headers;
             double delayInSeconds = delay;
 
-            if (UseRetryAfterHeader && headers.TryGetValues(RETRY_AFTER, out IEnumerable<string> values))
+            if (UseRetryAfterHeader && response != null && response.Headers.TryGetValues(RETRY_AFTER, out IEnumerable<string> values))
             {
                 // Can we use the provided retry-after header?
                 string retryAfter = values.First();