diff --git a/ObsWebSocket.Core/ObsWebSocketClient.cs b/ObsWebSocket.Core/ObsWebSocketClient.cs index d01bc1e..f7a17a8 100644 --- a/ObsWebSocket.Core/ObsWebSocketClient.cs +++ b/ObsWebSocket.Core/ObsWebSocketClient.cs @@ -311,14 +311,21 @@ await currentInitialConnectionTcs catch (Exception ex) // Catches exceptions set on the TCS or cancellation { _logger.LogConnectasyncFailedToEstablishInitialConnection(ex); - // Ensure finalization happens if the TCS faulted, loop might still be running/cleaning up - if ( + + if (currentInitialConnectionTcs.Task.IsFaulted && loopTask is not null) + { + // The loop gave up and is finalizing on its own. Finalizing here as well raced it, + // and whichever got there first decided the reason Disconnected reported. Waiting + // also means Disconnected has been raised by the time this throws. + await loopTask.ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing); + } + else if ( _connectionState is not ConnectionState.Disconnected and not ConnectionState.Disconnecting ) { - // Don't wait indefinitely, trigger finalize and let it run + // Timed out or cancelled while the loop may still be retrying: stop it here. _ = FinalizeDisconnectionAsync( WebSocketCloseStatus.InternalServerError, "Initial connection failed.", diff --git a/ObsWebSocket.Tests/ClientMessageHandlingTests.cs b/ObsWebSocket.Tests/ClientMessageHandlingTests.cs index b18bf63..f4bdb53 100644 --- a/ObsWebSocket.Tests/ClientMessageHandlingTests.cs +++ b/ObsWebSocket.Tests/ClientMessageHandlingTests.cs @@ -1,4 +1,7 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using ObsWebSocket.Core; +using ObsWebSocket.Core.Networking; using ObsWebSocket.Core.Protocol.Generated; using ObsWebSocket.Core.Protocol.Responses; using ObsWebSocket.Tests.Fakes; @@ -323,6 +326,68 @@ public async Task ConnectionLoop_LostWithAutoReconnectOff_StaysDisconnected() Assert.IsFalse(fake.Client.IsConnected); } + /// Holds the connection loop at the point it gives up, to force the race below. + private sealed class StallOnGiveUp : ILoggerProvider, ILogger + { + public ILogger CreateLogger(string categoryName) => this; + + public IDisposable? BeginScope(TState state) + where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter + ) + { + // 26: the loop has given up and is about to report why. + if (eventId.Id == 26) + { + Thread.Sleep(300); + } + } + + public void Dispose() { } + } + + [TestMethod] + [Timeout(TestTimeout)] + public async Task ConnectAsync_FirstAttemptFailsWhileLoopIsSlow_ReportsTheLoopsReasonOnce() + { + ServiceCollection services = new(); + _ = services.AddLogging(builder => + builder.ClearProviders().AddProvider(new StallOnGiveUp()) + ); + _ = services.AddObsWebSocketClient(options => + { + options.ServerUri = new Uri("ws://fake-obs:4455"); + options.AutoReconnectEnabled = false; + }); + _ = services.AddSingleton( + new FakeObsServer { RefuseConnections = true } + ); + await using ServiceProvider provider = services.BuildServiceProvider(); + ObsWebSocketClient client = provider.GetRequiredService(); + + List reasons = []; + client.Disconnected += (_, e) => reasons.Add(e.ReasonException); + + _ = await Assert.ThrowsExactlyAsync(() => + client.ConnectAsync() + ); + + // Disconnected has fired by the time ConnectAsync throws, once, with the loop's account + // of how many attempts it made rather than whichever path got there first. + Assert.HasCount(1, reasons); + Assert.IsInstanceOfType(reasons[0]); + Assert.Contains("after 1 attempts", reasons[0]!.Message); + Assert.IsInstanceOfType(reasons[0]!.InnerException); + } + [TestMethod] [Timeout(TestTimeout)] public async Task DisconnectAsync_ThenRequest_RequestRefused()