Log search service exceptions encountered during retry (#6232)

Author: Scott Bommarito

src/NuGet.Services.Search.Client/Client/RetryingHttpClientWrapper.cs

@@ -15,6 +15,7 @@ public sealed class RetryingHttpClientWrapper
     {
         private readonly HttpClient _httpClient;
         private readonly IEndpointHealthIndicatorStore _endpointHealthIndicatorStore;
+        private readonly Action<Exception> _onException;
 
         private static readonly int PeriodToDelayAlternateRequest = 3000;
         private static readonly IComparer<int> HealthComparer;
@@ -24,16 +25,16 @@ static RetryingHttpClientWrapper()
             HealthComparer = new WeightedRandomComparer();
         }
 
-        public RetryingHttpClientWrapper(HttpClient httpClient)
-            : this (httpClient, new BaseUrlHealthIndicatorStore(new NullHealthIndicatorLogger()))
+        public RetryingHttpClientWrapper(HttpClient httpClient, Action<Exception> onException)
+            : this (httpClient, new BaseUrlHealthIndicatorStore(new NullHealthIndicatorLogger()), onException)
         {
-            _httpClient = httpClient;
         }
 
-        public RetryingHttpClientWrapper(HttpClient httpClient, IEndpointHealthIndicatorStore endpointHealthIndicatorStore)
+        public RetryingHttpClientWrapper(HttpClient httpClient, IEndpointHealthIndicatorStore endpointHealthIndicatorStore, Action<Exception> onException)
         {
-            _httpClient = httpClient;
-            _endpointHealthIndicatorStore = endpointHealthIndicatorStore;
+            _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient));
+            _endpointHealthIndicatorStore = endpointHealthIndicatorStore ?? throw new ArgumentNullException(nameof(endpointHealthIndicatorStore));
+            _onException = onException ?? throw new ArgumentNullException(nameof(onException));
         }
 
         public async Task<string> GetStringAsync(IEnumerable<Uri> endpoints)
@@ -91,6 +92,11 @@ private async Task<HttpResponseMessage> GetWithRetry(IEnumerable<Uri> endpoints,
                 cancellationTokenSource.Cancel(false);
             }
 
+            foreach (var exception in exceptions)
+            {
+                _onException(exception);
+            }
+
             if (completedTask.IsFaulted || completedTask.IsCanceled)
             {
                 var exceptionsToThrow = exceptions.Where(e => !(e is TaskCanceledException || e.InnerException is TaskCanceledException)).ToList();

src/NuGet.Services.Search.Client/Client/SearchClient.cs

@@ -24,8 +24,8 @@ public class SearchClient
         /// </summary>
         /// <param name="baseUri">The URL to the root of the service</param>
         /// <param name="handlers">Handlers to apply to the request in order from first to last</param>
-        public SearchClient(Uri baseUri, params DelegatingHandler[] handlers)
-            : this(baseUri, "SearchGalleryQueryService/3.0.0-rc", null, new BaseUrlHealthIndicatorStore(new NullHealthIndicatorLogger()), handlers)
+        public SearchClient(Uri baseUri, Action<Exception> onException, params DelegatingHandler[] handlers)
+            : this(baseUri, "SearchGalleryQueryService/3.0.0-rc", null, new BaseUrlHealthIndicatorStore(new NullHealthIndicatorLogger()), onException, handlers)
         {
         }
 
@@ -37,7 +37,7 @@ public SearchClient(Uri baseUri, params DelegatingHandler[] handlers)
         /// <param name="credentials">The credentials to connect to the service with</param>
         /// <param name="healthIndicatorStore">Health indicator store</param>
         /// <param name="handlers">Handlers to apply to the request in order from first to last</param>
-        public SearchClient(Uri baseUri, string resourceType, ICredentials credentials, IEndpointHealthIndicatorStore healthIndicatorStore, params DelegatingHandler[] handlers)
+        public SearchClient(Uri baseUri, string resourceType, ICredentials credentials, IEndpointHealthIndicatorStore healthIndicatorStore, Action<Exception> onException, params DelegatingHandler[] handlers)
         {
             _resourceType = resourceType;
 
@@ -57,7 +57,7 @@ public SearchClient(Uri baseUri, string resourceType, ICredentials credentials,
 
             _httpClient = new HttpClient(handler, disposeHandler: true);
 
-            _retryingHttpClientWrapper = new RetryingHttpClientWrapper(_httpClient, healthIndicatorStore);
+            _retryingHttpClientWrapper = new RetryingHttpClientWrapper(_httpClient, healthIndicatorStore, onException);
             _discoveryClient = new ServiceDiscoveryClient(_httpClient, baseUri);
         }
 

src/NuGetGallery/Infrastructure/Lucene/ExternalSearchService.cs

@@ -52,7 +52,14 @@ public ExternalSearchService()
 
             if (_client == null)
             {
-                _client = new SearchClient(ServiceUri, "SearchGalleryQueryService/3.0.0-rc", null, _healthIndicatorStore, new TracingHttpHandler(Trace), new CorrelatingHttpClientHandler());
+                _client = new SearchClient(
+                    ServiceUri, 
+                    "SearchGalleryQueryService/3.0.0-rc", 
+                    null, 
+                    _healthIndicatorStore, 
+                    QuietLog.LogHandledException, 
+                    new TracingHttpHandler(Trace), 
+                    new CorrelatingHttpClientHandler());
             }
         }
 
@@ -90,7 +97,14 @@ public ExternalSearchService(IAppConfiguration config, IDiagnosticsService diagn
 
             if (_client == null)
             {
-                _client = new SearchClient(ServiceUri, config.SearchServiceResourceType, credentials, _healthIndicatorStore, new TracingHttpHandler(Trace), new CorrelatingHttpClientHandler());
+                _client = new SearchClient(
+                    ServiceUri, 
+                    config.SearchServiceResourceType, 
+                    credentials, 
+                    _healthIndicatorStore,
+                    QuietLog.LogHandledException,
+                    new TracingHttpHandler(Trace), 
+                    new CorrelatingHttpClientHandler());
             }
         }
 

src/NuGetGallery/Queries/AutoCompleteServiceQuery.cs

@@ -28,7 +28,7 @@ public AutoCompleteServiceQuery(IAppConfiguration configuration)
 
             _serviceDiscoveryClient = new ServiceDiscoveryClient(configuration.ServiceDiscoveryUri);
             _autocompleteServiceResourceType = configuration.AutocompleteServiceResourceType;
-            _httpClient = new RetryingHttpClientWrapper(new HttpClient());
+            _httpClient = new RetryingHttpClientWrapper(new HttpClient(), QuietLog.LogHandledException);
         }
 
         public async Task<IEnumerable<string>> RunServiceQuery(

tests/NuGetGallery.Facts/SearchClient/RetryingHttpClientWrapperFacts.cs

@@ -23,12 +23,12 @@ public class RetryingHttpClientWrapperFacts
 
         private RetryingHttpClientWrapper CreateWrapperClient(HttpMessageHandler handler)
         {
-            return new RetryingHttpClientWrapper(new HttpClient(handler));
+            return new RetryingHttpClientWrapper(new HttpClient(handler), (exception) => { });
         }
 
         private RetryingHttpClientWrapper CreateWrapperClient()
         {
-            return new RetryingHttpClientWrapper(new HttpClient());
+            return new RetryingHttpClientWrapper(new HttpClient(), (exception) => { });
         }
 
         [Fact]