dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.35k stars 9.99k forks source link

Pipe should complete with exception if WebSockets closed prematurely #36137

Open zackliu opened 3 years ago

zackliu commented 3 years ago

Summary

Currently, if a WebSocket is closed prematurely, the application pipe will complete with no exception. As a server, we need to catch this kind of error in our higher layer codes and log it in our specific format or add some special logic. If the pipe closes very normally, we can't tell whether it's a real normal close or unexpected close.

https://github.com/dotnet/aspnetcore/blob/main/src/SignalR/common/Http.Connections/src/Internal/Transports/WebSocketsServerTransport.cs#L179

Motivation and goals

Unexpected close should not swallow in such low layer. It can throw rethrow a new exception such as ClosedPrematurelyException to tell application layer an unexpected/network issue. As a service provider, if we can't tell the close reason easily, we lose many opportunities.

davidfowl commented 3 years ago

Is there a reason the existing log doesn't work for you?

zackliu commented 3 years ago

It's inflexible. If we also want to provide the connection closed reason to customer in Azure, we need to obey some log format. And we also need to send metrics in the case. Anyway, we need to inject some our logic, so we need to catch it in our own codes instead of an underlayer log.

BrennanConroy commented 3 years ago

This looks related to https://github.com/dotnet/aspnetcore/issues/26701 We want to improve the experience a bit so that more exceptions are flowed to OnDisconnectedAsync so it's easier to tell if there was a graceful disconnect or not.