Retry policies are now per request instead of per HttpManager (#5252)

Author: Robbie-Microsoft

src/client/Microsoft.Identity.Client/AppConfig/ApplicationConfiguration.cs

@@ -114,9 +114,8 @@ public string ClientVersion
         public bool CacheSynchronizationEnabled { get; internal set; } = true;
         public bool MultiCloudSupportEnabled { get; set; } = false;
 
-        public bool RetryOnServerErrors { get; set; } = true;
-
         public ManagedIdentityId ManagedIdentityId { get; internal set; }
+        public bool DisableInternalRetries { get; internal set; } = false;
 
         public bool IsManagedIdentity { get; }
         public bool IsConfidentialClient { get; }

src/client/Microsoft.Identity.Client/AppConfig/BaseAbstractApplicationBuilder.cs

@@ -61,7 +61,8 @@ public T WithHttpClientFactory(IMsalHttpClientFactory httpClientFactory)
         /// </summary>
         /// <param name="httpClientFactory">HTTP client factory</param>
         /// <param name="retryOnceOn5xx">Configures MSAL to retry on 5xx server errors. When enabled (on by default), MSAL will wait 1 second after receiving
-        /// a 5xx error and then retry the http request again.</param>
+        /// a 5xx error and then retry the http request again.
+        /// When disabled, the developer will be responsible for configuring their own retry policy in their custom IMsalHttpClientFactory.</param>
         /// <remarks>MSAL does not guarantee that it will not modify the HttpClient, for example by adding new headers.
         /// Prior to the changes needed in order to make MSAL's httpClients thread safe (https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/pull/2046/files),
         /// the httpClient had the possibility of throwing an exception stating "Properties can only be modified before sending the first request".
@@ -73,7 +74,7 @@ public T WithHttpClientFactory(IMsalHttpClientFactory httpClientFactory)
         public T WithHttpClientFactory(IMsalHttpClientFactory httpClientFactory, bool retryOnceOn5xx)
         {
             Config.HttpClientFactory = httpClientFactory;
-            Config.RetryOnServerErrors = retryOnceOn5xx;
+            Config.DisableInternalRetries = !retryOnceOn5xx;
             return (T)this;
         }
 

src/client/Microsoft.Identity.Client/Http/HttpManager.cs

@@ -27,21 +27,29 @@ namespace Microsoft.Identity.Client.Http
     internal class HttpManager : IHttpManager
     {
         protected readonly IMsalHttpClientFactory _httpClientFactory;
-        private readonly IRetryPolicy _retryPolicy;
+        private readonly bool _disableInternalRetries;
         public long LastRequestDurationInMs { get; private set; }
 
         /// <summary>
-        /// A new instance of the HTTP manager with a retry *condition*. The retry policy hardcodes: 
-        /// - the number of retries (1)
-        /// - the delay between retries (1 second)
+        /// Initializes a new instance of the <see cref="HttpManager"/> class.
         /// </summary>
+        /// <param name="httpClientFactory">
+        /// An instance of <see cref="IMsalHttpClientFactory"/> used to create and manage <see cref="HttpClient"/> instances.
+        /// This factory ensures proper reuse of <see cref="HttpClient"/> to avoid socket exhaustion.
+        /// </param>
+        /// <param name="disableInternalRetries">
+        /// A boolean flag indicating whether the HTTP manager should enable retry logic for transient failures.
+        /// </param>
+        /// <exception cref="ArgumentNullException">
+        /// Thrown when <paramref name="httpClientFactory"/> is null.
+        /// </exception>
         public HttpManager(
             IMsalHttpClientFactory httpClientFactory,
-            IRetryPolicy retryPolicy)
+            bool disableInternalRetries)
         {
             _httpClientFactory = httpClientFactory ??
                 throw new ArgumentNullException(nameof(httpClientFactory));
-            _retryPolicy = retryPolicy;
+            _disableInternalRetries = disableInternalRetries;
         }
 
         public async Task<HttpResponse> SendRequestAsync(
@@ -54,6 +62,7 @@ public async Task<HttpResponse> SendRequestAsync(
             X509Certificate2 bindingCertificate,
             Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> validateServerCert,
             CancellationToken cancellationToken,
+            IRetryPolicy retryPolicy,
             int retryCount = 0)
         {
             Exception timeoutException = null;
@@ -102,9 +111,9 @@ public async Task<HttpResponse> SendRequestAsync(
                 timeoutException = exception;
             }
 
-            while (_retryPolicy.pauseForRetry(response, timeoutException, retryCount))
+            while (!_disableInternalRetries && retryPolicy.PauseForRetry(response, timeoutException, retryCount))
             {
-                logger.Warning($"Retry condition met. Retry count: {retryCount++} after waiting {_retryPolicy.DelayInMilliseconds}ms.");
+                logger.Warning($"Retry condition met. Retry count: {retryCount++} after waiting {retryPolicy.DelayInMilliseconds}ms.");
                 return await SendRequestAsync(
                     endpoint,
                     headers,
@@ -113,8 +122,10 @@ public async Task<HttpResponse> SendRequestAsync(
                     logger,
                     doNotThrow,
                     bindingCertificate,
-                    validateServerCert, cancellationToken: cancellationToken,
-                    retryCount: retryCount) // Pass the updated retry count
+                    validateServerCert,
+                    cancellationToken,
+                    retryPolicy,
+                    retryCount) // Pass the updated retry count
                     .ConfigureAwait(false);
             }
 

src/client/Microsoft.Identity.Client/Http/HttpManagerFactory.cs

@@ -1,32 +1,18 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-
 namespace Microsoft.Identity.Client.Http
 {
     /// <summary>
-    /// Factory to return the instance of HttpManager based on retry configuration and type of MSAL application.
+    /// Factory to return the instance of HttpManager based on type of MSAL application.
     /// </summary>
     internal sealed class HttpManagerFactory
     {
         public static IHttpManager GetHttpManager(
             IMsalHttpClientFactory httpClientFactory,
-            bool withRetry,
-            bool isManagedIdentity)
+            bool disableInternalRetries = false)
         {
-            if (!withRetry)
-            {
-                return new HttpManager(httpClientFactory, new NoRetryPolicy());
-            }
-
-            return isManagedIdentity ?
-                new HttpManager(httpClientFactory, new LinearRetryPolicy(1000, 3, HttpRetryConditions.ManagedIdentity)) :
-                new HttpManager(httpClientFactory, new LinearRetryPolicy(1000, 1, HttpRetryConditions.Sts));
+            return new HttpManager(httpClientFactory, disableInternalRetries);
         }
     }
 }

src/client/Microsoft.Identity.Client/Http/IHttpManager.cs

@@ -28,6 +28,7 @@ internal interface IHttpManager
         /// <param name="mtlsCertificate">Certificate used for MTLS authentication.</param>
         /// <param name="validateServerCertificate">Callback to validate the server cert for service fabric managed identity flow.</param>
         /// <param name="cancellationToken"></param>
+        /// <param name="retryPolicy">Retry policy to be used for the request.</param>
         /// <param name="retryCount">Number of retries to be attempted in case of retriable status codes.</param>
         /// <returns></returns>
         Task<HttpResponse> SendRequestAsync(
@@ -40,6 +41,7 @@ Task<HttpResponse> SendRequestAsync(
            X509Certificate2 mtlsCertificate,
            Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> validateServerCertificate,
            CancellationToken cancellationToken,
+           IRetryPolicy retryPolicy,
            int retryCount = 0);
     }
 }

src/client/Microsoft.Identity.Client/Http/IRetryPolicy.cs

@@ -12,6 +12,6 @@ namespace Microsoft.Identity.Client.Http
     internal interface IRetryPolicy
     {
         int DelayInMilliseconds { get; }
-        bool pauseForRetry(HttpResponse response, Exception exception, int retryCount);
+        bool PauseForRetry(HttpResponse response, Exception exception, int retryCount);
     }
 }

src/client/Microsoft.Identity.Client/Http/LinearRetryPolicy.cs

@@ -9,7 +9,12 @@ namespace Microsoft.Identity.Client.Http
 {
     internal class LinearRetryPolicy : IRetryPolicy
     {
-        
+        // referenced in unit tests, cannot be private
+        public static int numRetries { get; private set; } = 0;
+        public const int DefaultStsMaxRetries = 1;
+        // this will be overridden in the unit tests so that they run faster
+        public static int DefaultStsRetryDelayMs { get; set; } = 1000;
+
         private int _maxRetries;
         private readonly Func<HttpResponse, Exception, bool> _retryCondition;
         public int DelayInMilliseconds { private set; get; }
@@ -21,8 +26,11 @@ public LinearRetryPolicy(int delayMilliseconds, int maxRetries, Func<HttpRespons
             _retryCondition = retryCondition;
         }
 
-        public bool pauseForRetry(HttpResponse response, Exception exception, int retryCount)
+        public bool PauseForRetry(HttpResponse response, Exception exception, int retryCount)
         {
+            // referenced in the unit tests
+            numRetries = retryCount + 1;
+
             return retryCount < _maxRetries && _retryCondition(response, exception);
         }
     }

src/client/Microsoft.Identity.Client/Http/NoRetryPolicy.cs

@@ -46,6 +46,11 @@ public RegionInfo(string region, RegionAutodetectionSource regionSource, string
         private static bool s_failedAutoDiscovery = false;
         private static string s_regionDiscoveryDetails;
 
+        private readonly LinearRetryPolicy _linearRetryPolicy = new LinearRetryPolicy(
+            LinearRetryPolicy.DefaultStsRetryDelayMs,
+            LinearRetryPolicy.DefaultStsMaxRetries,
+            HttpRetryConditions.Sts);
+
         public RegionManager(
             IHttpManager httpManager,
             int imdsCallTimeout = 2000,
@@ -206,7 +211,9 @@ private async Task<RegionInfo> DiscoverAsync(ILoggerAdapter logger, Cancellation
                                     logger: logger,
                                     doNotThrow: false,
                                     mtlsCertificate: null,
-                                    validateServerCertificate: null, cancellationToken: GetCancellationToken(requestCancellationToken))
+                                    validateServerCertificate: null,
+                                    cancellationToken: GetCancellationToken(requestCancellationToken),
+                                    retryPolicy: _linearRetryPolicy)
                                 .ConfigureAwait(false);
 
                             // A bad request occurs when the version in the IMDS call is no longer supported.
@@ -222,7 +229,9 @@ private async Task<RegionInfo> DiscoverAsync(ILoggerAdapter logger, Cancellation
                                     logger: logger,
                                     doNotThrow: false,
                                     mtlsCertificate: null,
-                                    validateServerCertificate: null, cancellationToken: GetCancellationToken(requestCancellationToken))
+                                    validateServerCertificate: null,
+                                    cancellationToken: GetCancellationToken(requestCancellationToken),
+                                    retryPolicy: _linearRetryPolicy)
                                     .ConfigureAwait(false); // Call again with updated version
                             }
 
@@ -324,7 +333,8 @@ private async Task<string> GetImdsUriApiVersionAsync(ILoggerAdapter logger, Dict
                     doNotThrow: false,
                     mtlsCertificate: null,
                     validateServerCertificate: null,
-                    cancellationToken: GetCancellationToken(userCancellationToken))
+                    cancellationToken: GetCancellationToken(userCancellationToken),
+                    retryPolicy: _linearRetryPolicy)
                 .ConfigureAwait(false);
 
             // When IMDS endpoint is called without the api version query param, bad request response comes back with latest version.

src/client/Microsoft.Identity.Client/Instance/Region/RegionManager.cs

@@ -6,6 +6,7 @@
 using System.Net;
 using System.Threading.Tasks;
 using Microsoft.Identity.Client.Core;
+using Microsoft.Identity.Client.Http;
 using Microsoft.Identity.Client.Internal;
 using Microsoft.Identity.Client.OAuth2;
 
@@ -28,6 +29,11 @@ public async Task ValidateAuthorityAsync(
                 var resource = $"https://{authorityInfo.Host}";
                 string webFingerUrl = Constants.FormatAdfsWebFingerUrl(authorityInfo.Host, resource);
 
+                LinearRetryPolicy _linearRetryPolicy = new LinearRetryPolicy(
+                    LinearRetryPolicy.DefaultStsRetryDelayMs,
+                    LinearRetryPolicy.DefaultStsMaxRetries,
+                    HttpRetryConditions.Sts);
+
                 Http.HttpResponse httpResponse = await _requestContext.ServiceBundle.HttpManager.SendRequestAsync(
                     new Uri(webFingerUrl),
                     null,
@@ -36,7 +42,10 @@ public async Task ValidateAuthorityAsync(
                     logger: _requestContext.Logger,
                     doNotThrow: false,
                     mtlsCertificate: null,
-                    validateServerCertificate: null, cancellationToken: _requestContext.UserCancellationToken)
+                    validateServerCertificate: null,
+                    cancellationToken: _requestContext.UserCancellationToken,
+                    retryPolicy: _linearRetryPolicy
+                    )
                     .ConfigureAwait(false);
 
                 if (httpResponse.StatusCode != HttpStatusCode.OK)