From 4b104fbad4eb79c378a53c4fb10490114a7ac7b8 Mon Sep 17 00:00:00 2001 From: mphelt <25581948+mphelt@users.noreply.github.com> Date: Thu, 4 May 2023 15:49:36 +0200 Subject: [PATCH] Missing overrides in LoggingHttpMessageHandler and LoggingScopeHttpMessageHandler (#85143) * add missing overrides in LoggingHttpMessageHandler and LoggingScopeHttpMessageHandler (https://github.com/dotnet/runtime/issues/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) --- .../src/Logging/LoggingHttpMessageHandler.cs | 24 +++++- .../Logging/LoggingScopeHttpMessageHandler.cs | 24 +++++- .../Logging/LoggingUriOutputTests.cs | 84 ++++++++++++++++++- 3 files changed, 123 insertions(+), 9 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs b/src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs index 62ec57cbc71f3..c91bc63b82396 100644 --- 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; } - /// - /// Loggs the request to and response from the sent . - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + private Task SendCoreAsync(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken) { ThrowHelper.ThrowIfNull(request); return Core(request, cancellationToken); @@ -62,13 +60,31 @@ async Task 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; } } + /// + /// Logs the request to and response from the sent . + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => SendCoreAsync(request, useAsync: true, cancellationToken); + +#if NET5_0_OR_GREATER + /// + /// Logs the request to and response from the sent . + 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 index a2cadee1bcf81..d111c04c5c71d 100644 --- 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; } - /// - /// Loggs the request to and response from the sent . - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + private Task SendCoreAsync(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken) { ThrowHelper.ThrowIfNull(request); return Core(request, cancellationToken); @@ -63,7 +61,13 @@ async Task 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; @@ -71,6 +75,18 @@ async Task Core(HttpRequestMessage request, CancellationTok } } + /// + /// Logs the request to and response from the sent . + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => SendCoreAsync(request, useAsync: true, cancellationToken); + +#if NET5_0_OR_GREATER + /// + /// Logs the request to and response from the sent . + 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 index 9f5588a3f6105..c7a30a591351e 100644 --- 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(new TestLoggerFactory(sink, enabled: true)); + + serviceCollection + .AddHttpClient("test") + .ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler()); + + var services = serviceCollection.BuildServiceProvider(); + + var client = services.GetRequiredService().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(new TestLoggerFactory(sink, enabled: true)); + + serviceCollection + .AddHttpClient("test") + .ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler()); + + var services = serviceCollection.BuildServiceProvider(); + + var client = services.GetRequiredService().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 SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) @@ -98,6 +176,10 @@ protected override Task SendAsync(HttpRequestMessage reques return Task.FromResult(response); } + +#if NET5_0_OR_GREATER + protected override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) => new(); +#endif } } }