Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ public static void ConnectingToEndpoints(ILogger logger, EndPointCollection endp
[LoggerMessage(13, LogLevel.Error, "Error forwarding client result with ID '{InvocationID}' to server.", EventName = "ErrorForwardingResult")]
public static partial void ErrorForwardingResult(ILogger logger, string invocationId, Exception ex);

[LoggerMessage(14, LogLevel.Error, "Error connecting to Redis.", EventName = "ErrorConnecting")]
public static partial void ErrorConnecting(ILogger logger, Exception ex);

// This isn't DefineMessage-based because it's just the simple TextWriter logging from ConnectionMultiplexer
public static void ConnectionMultiplexerMessage(ILogger logger, string? message)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class RedisHubLifetimeManager<THub> : HubLifetimeManager<THub>, IDisposab
private readonly SemaphoreSlim _connectionLock = new SemaphoreSlim(1);
private readonly IHubProtocolResolver _hubProtocolResolver;
private readonly ClientResultsManager _clientResultsManager = new();
private bool _redisConnectErrorLogged;

private readonly AckHandler _ackHandler;
private int _internalAckId;
Expand Down Expand Up @@ -90,6 +91,7 @@ public RedisHubLifetimeManager(ILogger<RedisHubLifetimeManager<THub>> logger,
public override async Task OnConnectedAsync(HubConnectionContext connection)
{
await EnsureRedisServerConnection();

var feature = new RedisFeature();
connection.Features.Set<IRedisFeature>(feature);

Expand All @@ -112,11 +114,17 @@ public override Task OnDisconnectedAsync(HubConnectionContext connection)
{
_connections.Remove(connection);

var tasks = new List<Task>();
// If the bus is null then the Redis connection failed to be established and none of the other connection setup ran
if (_bus is null)
{
return Task.CompletedTask;
}

var connectionChannel = _channels.Connection(connection.ConnectionId);
var tasks = new List<Task>();

RedisLog.Unsubscribe(_logger, connectionChannel);
tasks.Add(_bus!.UnsubscribeAsync(connectionChannel));
tasks.Add(_bus.UnsubscribeAsync(connectionChannel));

var feature = connection.Features.GetRequiredFeature<IRedisFeature>();
var groupNames = feature.Groups;
Expand Down Expand Up @@ -704,7 +712,21 @@ private async Task EnsureRedisServerConnection()
if (_redisServerConnection == null)
{
var writer = new LoggerTextWriter(_logger);
_redisServerConnection = await _options.ConnectAsync(writer);
try
{
_redisServerConnection = await _options.ConnectAsync(writer);
}
catch (Exception ex)
{
// If the connection hasn't been established yet we shouldn't keep logging the same error over and over
// for every new client connection.
if (!_redisConnectErrorLogged)
{
RedisLog.ErrorConnecting(_logger, ex);
_redisConnectErrorLogged = true;
}
throw;
}
_bus = _redisServerConnection.GetSubscriber();

_redisServerConnection.ConnectionRestored += (_, e) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
using Microsoft.AspNetCore.SignalR.Specification.Tests;
using Microsoft.AspNetCore.SignalR.Tests;
using Microsoft.AspNetCore.Testing;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Logging.Testing;
using Microsoft.Extensions.Options;
using Moq;
using Newtonsoft.Json.Linq;
using Newtonsoft.Json.Serialization;
using Xunit;
Expand Down Expand Up @@ -83,6 +86,45 @@ public async Task CamelCasedJsonIsPreservedAcrossRedisBoundary()
}
}

[Fact]
public async Task ErrorFromConnectionFactoryLogsAndAllowsDisconnect()
{
var server = new TestRedisServer();

var testSink = new TestSink();
var logger = new TestLogger("", testSink, true);
var mockLoggerFactory = new Mock<ILoggerFactory>();
mockLoggerFactory
.Setup(m => m.CreateLogger(It.IsAny<string>()))
.Returns((string categoryName) => (ILogger)logger);
var loggerT = mockLoggerFactory.Object.CreateLogger<RedisHubLifetimeManager<Hub>>();

var manager = new RedisHubLifetimeManager<Hub>(
loggerT,
Options.Create(new RedisOptions()
{
ConnectionFactory = _ => throw new Exception("throw from connect")
}),
new DefaultHubProtocolResolver(new IHubProtocol[]
{
}, NullLogger<DefaultHubProtocolResolver>.Instance));

using (var client = new TestClient())
{
var connection = HubConnectionContextUtils.Create(client.Connection);

var ex = await Assert.ThrowsAsync<Exception>(() => manager.OnConnectedAsync(connection)).DefaultTimeout();
Assert.Equal("throw from connect", ex.Message);

await manager.OnDisconnectedAsync(connection).DefaultTimeout();
}

var logs = testSink.Writes.ToArray();
Assert.Single(logs);
Assert.Equal("Error connecting to Redis.", logs[0].Message);
Assert.Equal("throw from connect", logs[0].Exception.Message);
}

public override TestRedisServer CreateBackplane()
{
return new TestRedisServer();
Expand Down