From 465a219694f0bf3ad3e011fc7b2f097e71c0177e Mon Sep 17 00:00:00 2001 From: Andrii Snihyr Date: Tue, 23 Feb 2021 10:08:06 -0800 Subject: [PATCH] Don't log a stack trace for AddressInUseException (#30154) Co-authored-by: Chris Ross --- .../Core/src/Internal/KestrelServerImpl.cs | 4 +-- .../Kestrel/Core/test/KestrelServerTests.cs | 8 +++--- .../TestApplicationErrorLoggerLoggedTest.cs | 6 +++++ .../BindTests/AddressRegistrationTests.cs | 25 ++++++++++++++----- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs index ebcdc25e7878..8458bf221f44 100644 --- a/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/KestrelServerImpl.cs @@ -205,9 +205,9 @@ async Task OnBind(ListenOptions options) await BindAsync(cancellationToken).ConfigureAwait(false); } - catch (Exception ex) + catch { - Trace.LogCritical(0, ex, "Unable to start Kestrel."); + // Don't log the error https://github.com/dotnet/aspnetcore/issues/29801 Dispose(); throw; } diff --git a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs index 23a85f5d2e76..cb705b770cd7 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerTests.cs @@ -50,7 +50,7 @@ public void StartWithInvalidAddressThrows() var exception = Assert.Throws(() => StartDummyApplication(server)); Assert.Contains("Invalid url", exception.Message); - Assert.Equal(1, testLogger.CriticalErrorsLogged); + Assert.Equal(0, testLogger.CriticalErrorsLogged); } } @@ -123,7 +123,7 @@ public void StartWithPathBaseInAddressThrows() Assert.Equal( $"A path base can only be configured using {nameof(IApplicationBuilder)}.UsePathBase().", exception.Message); - Assert.Equal(1, testLogger.CriticalErrorsLogged); + Assert.Equal(0, testLogger.CriticalErrorsLogged); } } @@ -171,7 +171,7 @@ public void StartWithMaxRequestBufferSizeLessThanMaxRequestLineSizeThrows(long m Assert.Equal( CoreStrings.FormatMaxRequestBufferSmallerThanRequestLineBuffer(maxRequestBufferSize, maxRequestLineSize), exception.Message); - Assert.Equal(1, testLogger.CriticalErrorsLogged); + Assert.Equal(0, testLogger.CriticalErrorsLogged); } } @@ -198,7 +198,7 @@ public void StartWithMaxRequestBufferSizeLessThanMaxRequestHeadersTotalSizeThrow Assert.Equal( CoreStrings.FormatMaxRequestBufferSmallerThanRequestHeaderBuffer(maxRequestBufferSize, maxRequestHeadersTotalSize), exception.Message); - Assert.Equal(1, testLogger.CriticalErrorsLogged); + Assert.Equal(0, testLogger.CriticalErrorsLogged); } } diff --git a/src/Servers/Kestrel/shared/test/TestApplicationErrorLoggerLoggedTest.cs b/src/Servers/Kestrel/shared/test/TestApplicationErrorLoggerLoggedTest.cs index 3ed79a090cbd..058c8e700342 100644 --- a/src/Servers/Kestrel/shared/test/TestApplicationErrorLoggerLoggedTest.cs +++ b/src/Servers/Kestrel/shared/test/TestApplicationErrorLoggerLoggedTest.cs @@ -17,6 +17,12 @@ public class TestApplicationErrorLoggerLoggedTest : LoggedTest public ConcurrentQueue LogMessages => TestApplicationErrorLogger.Messages; + public bool ThrowOnCriticalErrors + { + get => TestApplicationErrorLogger.ThrowOnCriticalErrors; + set => TestApplicationErrorLogger.ThrowOnCriticalErrors = value; + } + public bool ThrowOnUngracefulShutdown { get => TestApplicationErrorLogger.ThrowOnUngracefulShutdown; diff --git a/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs b/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs index 6eed911a0a7e..0313101d5667 100644 --- a/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs +++ b/src/Servers/Kestrel/test/BindTests/AddressRegistrationTests.cs @@ -549,6 +549,8 @@ private async Task RegisterDefaultServerAddresses_Success(IEnumerable ad [Fact] public async Task ThrowsWhenBindingToIPv4AddressInUse() { + ThrowOnCriticalErrors = false; + using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) { socket.Bind(new IPEndPoint(IPAddress.Loopback, 0)); @@ -562,13 +564,17 @@ public async Task ThrowsWhenBindingToIPv4AddressInUse() .UseKestrel() .UseUrls($"http://127.0.0.1:{port}") .Configure(ConfigureEchoAddress); - }); + }) + .ConfigureServices(AddTestLogging); using (var host = hostBuilder.Build()) { var exception = Assert.Throws(() => host.Start()); - Assert.Equal(CoreStrings.FormatEndpointAlreadyInUse($"http://127.0.0.1:{port}"), exception.Message); - + var expectedMessage = CoreStrings.FormatEndpointAlreadyInUse($"http://127.0.0.1:{port}"); + Assert.Equal(expectedMessage, exception.Message); + Assert.Equal(0, LogMessages.Count(log => log.LogLevel == LogLevel.Critical && + log.Exception is null && + log.Message.EndsWith(expectedMessage, StringComparison.Ordinal))); await host.StopAsync(); } } @@ -578,7 +584,7 @@ public async Task ThrowsWhenBindingToIPv4AddressInUse() [IPv6SupportedCondition] public async Task ThrowsWhenBindingToIPv6AddressInUse() { - IgnoredCriticalLogExceptions.Add(typeof(IOException)); + ThrowOnCriticalErrors = false; using (var socket = new Socket(AddressFamily.InterNetworkV6, SocketType.Stream, ProtocolType.Tcp)) { @@ -599,7 +605,11 @@ public async Task ThrowsWhenBindingToIPv6AddressInUse() using (var host = hostBuilder.Build()) { var exception = Assert.Throws(() => host.Start()); - Assert.Equal(CoreStrings.FormatEndpointAlreadyInUse($"http://[::1]:{port}"), exception.Message); + var expectedMessage = CoreStrings.FormatEndpointAlreadyInUse($"http://[::1]:{port}"); + Assert.Equal(expectedMessage, exception.Message); + Assert.Equal(0, LogMessages.Count(log => log.LogLevel == LogLevel.Critical && + log.Exception is null && + log.Message.EndsWith(expectedMessage, StringComparison.Ordinal))); await host.StopAsync(); } @@ -931,7 +941,7 @@ public async Task EndpointDefaultsConfig_CanSetProtocolForUrlsConfig(string inpu private void ThrowsWhenBindingLocalhostToAddressInUse(AddressFamily addressFamily) { - IgnoredCriticalLogExceptions.Add(typeof(IOException)); + ThrowOnCriticalErrors = false; var addressInUseCount = 0; var wrongMessageCount = 0; @@ -990,6 +1000,9 @@ private void ThrowsWhenBindingLocalhostToAddressInUse(AddressFamily addressFamil } Assert.Equal(CoreStrings.FormatEndpointAlreadyInUse(thisAddressString), exception.Message); + Assert.Equal(0, LogMessages.Count(log => log.LogLevel == LogLevel.Critical && + log.Exception is null && + log.Message.EndsWith(CoreStrings.FormatEndpointAlreadyInUse(thisAddressString), StringComparison.Ordinal))); break; } }