Missing overrides in LoggingHttpMessageHandler and LoggingScopeHttpMessageHandler (dotnet#85143)

mphelt · web-flow · commit 4b104fbad4eb · 2023-05-04T15:49:36.000+02:00
* add missing overrides in LoggingHttpMessageHandler and LoggingScopeHttpMessageHandler (dotnet#85104) * Update LoggingUriOutputTests.cs * Update LoggingHttpMessageHandler.cs * Update LoggingScopeHttpMessageHandler.cs * Update LoggingScopeHttpMessageHandler.cs * Update LoggingUriOutputTests.cs * Update LoggingScopeHttpMessageHandler.cs * Update LoggingUriOutputTests.cs * Update LoggingUriOutputTests.cs * Update LoggingUriOutputTests.cs * Update LoggingUriOutputTests.cs * Update LoggingHttpMessageHandler.cs * Update LoggingScopeHttpMessageHandler.cs * Update LoggingHttpMessageHandler.cs * Update LoggingScopeHttpMessageHandler.cs * Update LoggingHttpMessageHandler.cs * Update LoggingHttpMessageHandler.cs * Update LoggingScopeHttpMessageHandler.cs * Code style update * back to private methods * merge with dotnet/runtime (#7)
diff --git a/src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs b/src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs
@@ -47,9 +47,7 @@ public LoggingHttpMessageHandler(ILogger logger, HttpClientFactoryOptions option
             _options = options;
         }
 
-        /// <inheritdoc />
-        /// <remarks>Loggs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
-        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        private Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken)
         {
             ThrowHelper.ThrowIfNull(request);
             return Core(request, cancellationToken);
@@ -62,13 +60,31 @@ async Task<HttpResponseMessage> Core(HttpRequestMessage request, CancellationTok
                 // not really anything to surround.
                 Log.RequestStart(_logger, request, shouldRedactHeaderValue);
                 var stopwatch = ValueStopwatch.StartNew();
-                HttpResponseMessage response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
+                HttpResponseMessage response = useAsync
+                    ? await base.SendAsync(request, cancellationToken).ConfigureAwait(false)
+#if NET5_0_OR_GREATER
+                    : base.Send(request, cancellationToken);
+#else
+                    : throw new NotImplementedException("Unreachable code");
+#endif
                 Log.RequestEnd(_logger, response, stopwatch.GetElapsedTime(), shouldRedactHeaderValue);
 
                 return response;
             }
         }
 
+        /// <inheritdoc />
+        /// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
+        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+            => SendCoreAsync(request, useAsync: true, cancellationToken);
+
+#if NET5_0_OR_GREATER
+        /// <inheritdoc />
+        /// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
+        protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
+            => SendCoreAsync(request, useAsync: false, cancellationToken).GetAwaiter().GetResult();
+#endif
+
         // Used in tests.
         internal static class Log
         {
diff --git a/src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingScopeHttpMessageHandler.cs b/src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingScopeHttpMessageHandler.cs
@@ -47,9 +47,7 @@ public LoggingScopeHttpMessageHandler(ILogger logger, HttpClientFactoryOptions o
             _options = options;
         }
 
-        /// <inheritdoc />
-        /// <remarks>Loggs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
-        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+        private Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken)
         {
             ThrowHelper.ThrowIfNull(request);
             return Core(request, cancellationToken);
@@ -63,14 +61,32 @@ async Task<HttpResponseMessage> Core(HttpRequestMessage request, CancellationTok
                 using (Log.BeginRequestPipelineScope(_logger, request))
                 {
                     Log.RequestPipelineStart(_logger, request, shouldRedactHeaderValue);
-                    HttpResponseMessage response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
+                    HttpResponseMessage response = useAsync
+                        ? await base.SendAsync(request, cancellationToken).ConfigureAwait(false)
+#if NET5_0_OR_GREATER
+                        : base.Send(request, cancellationToken);
+#else
+                        : throw new NotImplementedException("Unreachable code");
+#endif
                     Log.RequestPipelineEnd(_logger, response, stopwatch.GetElapsedTime(), shouldRedactHeaderValue);
 
                     return response;
                 }
             }
         }
 
+        /// <inheritdoc />
+        /// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
+        protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
+            => SendCoreAsync(request, useAsync: true, cancellationToken);
+
+#if NET5_0_OR_GREATER
+        /// <inheritdoc />
+        /// <remarks>Logs the request to and response from the sent <see cref="HttpRequestMessage"/>.</remarks>
+        protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
+            => SendCoreAsync(request, useAsync: false, cancellationToken).GetAwaiter().GetResult();
+#endif
+
         // Used in tests
         internal static class Log
         {
diff --git a/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/Logging/LoggingUriOutputTests.cs b/src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/Logging/LoggingUriOutputTests.cs
@@ -1,6 +1,7 @@
-﻿// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Linq;
 using System.Net.Http;
 using System.Threading;
@@ -90,6 +91,83 @@ public async Task LoggingScopeHttpMessageHandler_LogsAbsoluteUri()
             Assert.Equal("HTTP GET http://api.example.com/search?term=Western%20Australia", message.Scope.ToString());
         }
 
+#if NET5_0_OR_GREATER
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
+        public void LoggingHttpMessageHandler_LogsAbsoluteUri_Sync()
+        {
+            // Arrange
+            var sink = new TestSink();
+
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddLogging();
+            serviceCollection.AddSingleton<ILoggerFactory>(new TestLoggerFactory(sink, enabled: true));
+
+            serviceCollection
+                .AddHttpClient("test")
+                .ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler());
+
+            var services = serviceCollection.BuildServiceProvider();
+
+            var client = services.GetRequiredService<IHttpClientFactory>().CreateClient("test");
+
+
+            // Act
+            var request = new HttpRequestMessage(HttpMethod.Get, "http://api.example.com/search?term=Western%20Australia");
+
+            client.Send(request);
+
+            // Assert
+            var messages = sink.Writes.ToArray();
+
+            var message = Assert.Single(messages.Where(m =>
+            {
+                return
+                    m.EventId == LoggingHttpMessageHandler.Log.EventIds.RequestStart &&
+                    m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
+            }));
+
+            Assert.Equal("Sending HTTP request GET http://api.example.com/search?term=Western%20Australia", message.Message);
+        }
+
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNetCore))]
+        public void LoggingScopeHttpMessageHandler_LogsAbsoluteUri_Sync()
+        {
+            // Arrange
+            var sink = new TestSink();
+
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddLogging();
+            serviceCollection.AddSingleton<ILoggerFactory>(new TestLoggerFactory(sink, enabled: true));
+
+            serviceCollection
+                .AddHttpClient("test")
+                .ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler());
+
+            var services = serviceCollection.BuildServiceProvider();
+
+            var client = services.GetRequiredService<IHttpClientFactory>().CreateClient("test");
+
+
+            // Act
+            var request = new HttpRequestMessage(HttpMethod.Get, "http://api.example.com/search?term=Western%20Australia");
+
+            client.Send(request);
+
+            // Assert
+            var messages = sink.Writes.ToArray();
+
+            var message = Assert.Single(messages.Where(m =>
+            {
+                return
+                    m.EventId == LoggingScopeHttpMessageHandler.Log.EventIds.PipelineStart &&
+                    m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
+            }));
+
+            Assert.Equal("Start processing HTTP request GET http://api.example.com/search?term=Western%20Australia", message.Message);
+            Assert.Equal("HTTP GET http://api.example.com/search?term=Western%20Australia", message.Scope.ToString());
+        }
+#endif
+
         private class TestMessageHandler : HttpClientHandler
         {
             protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
@@ -98,6 +176,10 @@ protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage reques
 
                 return Task.FromResult(response);
             }
+
+#if NET5_0_OR_GREATER
+            protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) => new();
+#endif
         }
     }
 }
