dotnet / dotnet/aspnetcore

Raise severity of Kestrel's unexpected transport error logs

Open
#29,322 2 comments 0 reactions 0 assignees View on GitHub
area-networking feature-kestrel reliability
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

Arbitrary exceptions thrown from SocketConnection's receive and send loops are currently logged at the debug level.

https://github.com/dotnet/aspnetcore/blob/db6fbee1d7c5c2705940b7cc5f6abb1cee31b59f/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs#L174-L179

https://github.com/dotnet/aspnetcore/blob/db6fbee1d7c5c2705940b7cc5f6abb1cee31b59f/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketsTrace.cs#L29-L30

This was done so app developers aren't spammed with useless warnings, but really it just shows the lack of confidence that we properly handle every Exception on every platform.

This might have been prudent when Kestrel's Socket transport was also distributed as a NuGet package meaning we didn't have complete control over what implementation of System.Net.Sockets we might be running on, but that's no longer the case. We should make sure we understand every exception that can be thrown, and catch their specific types.

It's fine leaving the `catch (Exception ex)` block, but it should log a warning similar to what we do in Kestrel's HttpProtocol class.

https://github.com/dotnet/aspnetcore/blob/db6fbee1d7c5c2705940b7cc5f6abb1cee31b59f/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs#L589-L592

I think we should change this ASAP so we get feedback on previews. We should also verify that these unexpected connection error debug logs don't currently show up our functional tests.

I think we should do the same thing for unexpected errors in the QUIC transport.

https://github.com/dotnet/aspnetcore/blob/771bfb1976fa6cf829780840ac32b9ec719bca25/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicTrace.cs#L15-L18

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.