Skip to content

Pass serialization exceptions to Hub disconnect #24408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2020
Merged

Pass serialization exceptions to Hub disconnect #24408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/SignalR/server/Core/src/HubConnectionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ internal StreamTracker StreamTracker

internal HubCallerContext HubCallerContext { get; }

internal Exception CloseException { get; private set; }

/// <summary>
/// Gets a <see cref="CancellationToken"/> that notifies when the connection is aborted.
/// </summary>
Expand Down Expand Up @@ -212,6 +214,7 @@ private ValueTask<FlushResult> WriteCore(HubMessage message, CancellationToken c
}
catch (Exception ex)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);

AbortAllowReconnect();
Expand All @@ -231,6 +234,7 @@ private ValueTask<FlushResult> WriteCore(SerializedHubMessage message, Cancellat
}
catch (Exception ex)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);

AbortAllowReconnect();
Expand All @@ -247,6 +251,7 @@ private async Task CompleteWriteAsync(ValueTask<FlushResult> task)
}
catch (Exception ex)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);

AbortAllowReconnect();
Expand Down Expand Up @@ -274,6 +279,7 @@ private async Task WriteSlowAsync(HubMessage message, CancellationToken cancella
}
catch (Exception ex)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);
AbortAllowReconnect();
}
Expand All @@ -299,6 +305,7 @@ private async Task WriteSlowAsync(SerializedHubMessage message, CancellationToke
}
catch (Exception ex)
{
CloseException = ex;
Log.FailedWritingMessage(_logger, ex);
AbortAllowReconnect();
}
Expand Down Expand Up @@ -336,6 +343,7 @@ private async Task TryWritePingSlowAsync()
}
catch (Exception ex)
{
CloseException = ex;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping failures too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this makes sense.

Log.FailedWritingMessage(_logger, ex);
AbortAllowReconnect();
}
Expand Down
2 changes: 1 addition & 1 deletion src/SignalR/server/Core/src/HubConnectionHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ private async Task RunHubAsync(HubConnectionContext connection)
return;
}

await HubOnDisconnectedAsync(connection, null);
await HubOnDisconnectedAsync(connection, connection.CloseException);
}

private async Task HubOnDisconnectedAsync(HubConnectionContext connection, Exception exception)
Expand Down
33 changes: 23 additions & 10 deletions src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,6 @@ public void InvalidArgument(CancellationToken token)
{
}

private class SelfRef
{
public SelfRef()
{
Self = this;
}

public SelfRef Self { get; set; }
}

public async Task<string> StreamingConcat(ChannelReader<string> source)
{
var sb = new StringBuilder();
Expand Down Expand Up @@ -331,6 +321,16 @@ public async Task BlockingMethod()
}
}

internal class SelfRef
{
public SelfRef()
{
Self = this;
}

public SelfRef Self { get; set; }
}

public abstract class TestHub : Hub
{
public override Task OnConnectedAsync()
Expand Down Expand Up @@ -1123,9 +1123,20 @@ public override Task OnConnectedAsync()
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);
}
Expand All @@ -1138,6 +1149,8 @@ public class ConnectionLifetimeState
public bool TokenStateInConnected { get; set; }

public bool TokenStateInDisconnected { get; set; }

public Exception DisconnectedException { get; set; }
}

public class CallerServiceHub : Hub
Expand Down
48 changes: 46 additions & 2 deletions src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@
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;
Expand Down Expand Up @@ -3342,6 +3340,52 @@ public async Task ConnectionAbortedIfSendFailsWithProtocolError()
}
}

[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<HubConnectionHandler<ConnectionLifetimeHub>>();

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<System.Text.Json.JsonException>(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<HubConnectionHandler<ConnectionLifetimeHub>>();

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<System.Text.Json.JsonException>(state.DisconnectedException);
}
}
}

[Fact(Skip = "Magic auto cast not supported")]
public async Task UploadStreamItemInvalidTypeAutoCasts()
{
Expand Down