Add redis startup error handling

Author: BrennanConroy

src/SignalR/server/StackExchangeRedis/src/Internal/RedisLog.cs

@@ -52,6 +52,9 @@ public static void ConnectingToEndpoints(ILogger logger, EndPointCollection endp
     [LoggerMessage(11, LogLevel.Warning, "Error processing message for internal server message.", EventName = "InternalMessageFailed")]
     public static partial void InternalMessageFailed(ILogger logger, Exception exception);
 
+    [LoggerMessage(12, 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)
     {

src/SignalR/server/StackExchangeRedis/src/RedisHubLifetimeManager.cs

@@ -83,6 +83,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);
 
@@ -105,11 +106,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;
@@ -550,7 +557,15 @@ 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)
+                    {
+                        RedisLog.ErrorConnecting(_logger, ex);
+                        throw;
+                    }
                     _bus = _redisServerConnection.GetSubscriber();
 
                     _redisServerConnection.ConnectionRestored += (_, e) =>

src/SignalR/server/StackExchangeRedis/test/RedisHubLifetimeManagerTests.cs

@@ -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;
@@ -83,6 +86,48 @@ 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();
+        // Two of the same error logs, one for the pre-emptive attempt to connect to Redis in the ctor, and one during the OnConnectedAsync for the connection
+        Assert.Equal(2, logs.Length);
+        Assert.Equal("Error connecting to Redis.", logs[0].Message);
+        Assert.Equal("throw from connect", logs[0].Exception.Message);
+        Assert.Equal("Error connecting to Redis.", logs[1].Message);
+        Assert.Equal("throw from connect", logs[1].Exception.Message);
+    }
+
     public override TestRedisServer CreateBackplane()
     {
         return new TestRedisServer();