diff --git a/src/sdk/CHANGELOG.md b/src/sdk/CHANGELOG.md index 674a170bf9..59a9f7ce89 100644 --- 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 index 0346ab0e06..eac36b4e20 100644 --- 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 log, PnPGlobalSettingsOptions internal int DelayInSeconds { get; set; } = 3; internal bool IncrementalDelay { get; set; } = true; - protected async override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + protected override async Task 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)); + } - /// - /// Retry sending the HTTP request - /// - /// The which is returned and includes the HTTP request needs to be retried. - /// The for the retry. - /// - private async Task 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 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)); } /// @@ -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 values)) + if (UseRetryAfterHeader && response != null && response.Headers.TryGetValues(RETRY_AFTER, out IEnumerable values)) { // Can we use the provided retry-after header? string retryAfter = values.First();