From 4290bcc78235daa3b274709190c9970d37d54363 Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 31 Jul 2020 10:13:28 -0700 Subject: [PATCH] Pass serialization exceptions to Hub disconnect (#24408) --- .../server/Core/src/HubConnectionContext.cs | 8 ++++ .../server/Core/src/HubConnectionHandler.cs | 2 +- .../HubConnectionHandlerTestUtils/Hubs.cs | 33 +++++++++---- .../SignalR/test/HubConnectionHandlerTests.cs | 48 ++++++++++++++++++- 4 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/SignalR/server/Core/src/HubConnectionContext.cs b/src/SignalR/server/Core/src/HubConnectionContext.cs index 01d07d2e8a..e0a6d758f6 100644 --- a/src/SignalR/server/Core/src/HubConnectionContext.cs +++ b/src/SignalR/server/Core/src/HubConnectionContext.cs @@ -91,6 +91,8 @@ namespace Microsoft.AspNetCore.SignalR internal HubCallerContext HubCallerContext { get; } + internal Exception CloseException { get; private set; } + /// /// Gets a that notifies when the connection is aborted. /// @@ -212,6 +214,7 @@ namespace Microsoft.AspNetCore.SignalR } catch (Exception ex) { + CloseException = ex; Log.FailedWritingMessage(_logger, ex); AbortAllowReconnect(); @@ -231,6 +234,7 @@ namespace Microsoft.AspNetCore.SignalR } catch (Exception ex) { + CloseException = ex; Log.FailedWritingMessage(_logger, ex); AbortAllowReconnect(); @@ -247,6 +251,7 @@ namespace Microsoft.AspNetCore.SignalR } catch (Exception ex) { + CloseException = ex; Log.FailedWritingMessage(_logger, ex); AbortAllowReconnect(); @@ -274,6 +279,7 @@ namespace Microsoft.AspNetCore.SignalR } catch (Exception ex) { + CloseException = ex; Log.FailedWritingMessage(_logger, ex); AbortAllowReconnect(); } @@ -299,6 +305,7 @@ namespace Microsoft.AspNetCore.SignalR } catch (Exception ex) { + CloseException = ex; Log.FailedWritingMessage(_logger, ex); AbortAllowReconnect(); } @@ -336,6 +343,7 @@ namespace Microsoft.AspNetCore.SignalR } catch (Exception ex) { + CloseException = ex; Log.FailedWritingMessage(_logger, ex); AbortAllowReconnect(); } diff --git a/src/SignalR/server/Core/src/HubConnectionHandler.cs b/src/SignalR/server/Core/src/HubConnectionHandler.cs index 403a03e8ae..6ef96034ad 100644 --- a/src/SignalR/server/Core/src/HubConnectionHandler.cs +++ b/src/SignalR/server/Core/src/HubConnectionHandler.cs @@ -182,7 +182,7 @@ namespace Microsoft.AspNetCore.SignalR return; } - await HubOnDisconnectedAsync(connection, null); + await HubOnDisconnectedAsync(connection, connection.CloseException); } private async Task HubOnDisconnectedAsync(HubConnectionContext connection, Exception exception) diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs index 3031ee9ffd..c2a4893fd4 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs @@ -189,16 +189,6 @@ namespace Microsoft.AspNetCore.SignalR.Tests { } - private class SelfRef - { - public SelfRef() - { - Self = this; - } - - public SelfRef Self { get; set; } - } - public async Task StreamingConcat(ChannelReader source) { var sb = new StringBuilder(); @@ -331,6 +321,16 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } + internal class SelfRef + { + public SelfRef() + { + Self = this; + } + + public SelfRef Self { get; set; } + } + public abstract class TestHub : Hub { public override Task OnConnectedAsync() @@ -1123,9 +1123,20 @@ namespace Microsoft.AspNetCore.SignalR.Tests return base.OnConnectedAsync(); } + public Task ProtocolErrorSelf() + { + return Clients.Caller.SendAsync("Send", new SelfRef()); + } + + public Task ProtocolErrorAll() + { + return Clients.All.SendAsync("Send", new SelfRef()); + } + public override Task OnDisconnectedAsync(Exception exception) { _state.TokenStateInDisconnected = Context.ConnectionAborted.IsCancellationRequested; + _state.DisconnectedException = exception; return base.OnDisconnectedAsync(exception); } @@ -1138,6 +1149,8 @@ namespace Microsoft.AspNetCore.SignalR.Tests public bool TokenStateInConnected { get; set; } public bool TokenStateInDisconnected { get; set; } + + public Exception DisconnectedException { get; set; } } public class CallerServiceHub : Hub diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs index ceab4322be..32d198d2fd 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs @@ -19,10 +19,8 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Connections.Features; -using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; -using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; @@ -3342,6 +3340,52 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } + [Fact] + public async Task SerializationExceptionsSendSelfArePassedToOnDisconnectedAsync() + { + using (StartVerifiableLog(write => write.EventId.Name == "FailedWritingMessage")) + { + var state = new ConnectionLifetimeState(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(s => s.AddSingleton(state), LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler); + + // Test HubConnectionContext.WriteCore(HubMessage) codepath + await client.SendInvocationAsync(nameof(ConnectionLifetimeHub.ProtocolErrorSelf)).OrTimeout(); + + await connectionHandlerTask.OrTimeout(); + + Assert.IsType(state.DisconnectedException); + } + } + } + + [Fact] + public async Task SerializationExceptionsSendAllArePassedToOnDisconnectedAsync() + { + using (StartVerifiableLog(write => write.EventId.Name == "FailedWritingMessage")) + { + var state = new ConnectionLifetimeState(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(s => s.AddSingleton(state), LoggerFactory); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler); + + // Test HubConnectionContext.WriteCore(SerializedHubMessage) codepath + await client.SendInvocationAsync(nameof(ConnectionLifetimeHub.ProtocolErrorAll)).OrTimeout(); + + await connectionHandlerTask.OrTimeout(); + + Assert.IsType(state.DisconnectedException); + } + } + } + [Fact(Skip = "Magic auto cast not supported")] public async Task UploadStreamItemInvalidTypeAutoCasts() {