From 90593f4fb25dd4840d349d242e25b422a4be6e4b Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 20 May 2023 19:07:53 -0400 Subject: [PATCH 01/41] Clear DMAPI channels cache on TGS detach --- build/Version.props | 2 +- src/DMAPI/tgs.dm | 2 +- src/DMAPI/tgs/v5/topic.dm | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/build/Version.props b/build/Version.props index 7929f4741d..1a4eb38d8a 100644 --- a/build/Version.props +++ b/build/Version.props @@ -8,7 +8,7 @@ 9.10.2 10.4.1 11.4.2 - 6.4.3 + 6.4.4 5.6.0 1.2.2 1.2.1 diff --git a/src/DMAPI/tgs.dm b/src/DMAPI/tgs.dm index c562224c73..ab2d565991 100644 --- a/src/DMAPI/tgs.dm +++ b/src/DMAPI/tgs.dm @@ -1,6 +1,6 @@ // tgstation-server DMAPI -#define TGS_DMAPI_VERSION "6.4.3" +#define TGS_DMAPI_VERSION "6.4.4" // All functions and datums outside this document are subject to change with any version and should not be relied on. diff --git a/src/DMAPI/tgs/v5/topic.dm b/src/DMAPI/tgs/v5/topic.dm index 28fcc14aef..3779db6237 100644 --- a/src/DMAPI/tgs/v5/topic.dm +++ b/src/DMAPI/tgs/v5/topic.dm @@ -71,6 +71,7 @@ var/list/event_call = list(event_type) if (event_type == TGS_EVENT_WATCHDOG_DETACH) detached = TRUE + chat_channels.Cut() // https://github.com/tgstation/tgstation-server/issues/1490 if(event_parameters) event_call += event_parameters From 5967c178d412588f424d50cbbeafba6c4d8b738b Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 20 May 2023 19:15:31 -0400 Subject: [PATCH 02/41] Update chat tracking contexts after reattaching Fixes #1490 --- .../Components/Chat/ChatManager.cs | 22 +++++++++++-------- .../Components/Chat/IChatManager.cs | 7 ++++++ .../Components/Watchdog/WatchdogBase.cs | 3 +++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index ddd0aff92b..a1623b312c 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -232,7 +232,6 @@ namespace Tgstation.Server.Host.Components.Chat channelIdCounter += (ulong)results.Count; } - Task trackingContextUpdateTask; lock (mappedChannels) { lock (providers) @@ -245,16 +244,9 @@ namespace Tgstation.Server.Host.Components.Chat mappedChannels.Add(newId, newMapping); newMapping.Channel.RealId = newId; } - - lock (trackingContexts) - trackingContextUpdateTask = Task.WhenAll( - trackingContexts.Select( - x => x.UpdateChannels( - mappedChannels.Select(y => y.Value.Channel).ToList(), - cancellationToken))); } - await trackingContextUpdateTask; + await UpdateTrackingContexts(cancellationToken); } finally { @@ -472,6 +464,18 @@ namespace Tgstation.Server.Host.Components.Chat return context; } + /// + public Task UpdateTrackingContexts(CancellationToken cancellationToken) + { + lock (mappedChannels) + lock (trackingContexts) + return Task.WhenAll( + trackingContexts.Select( + x => x.UpdateChannels( + mappedChannels.Select(y => y.Value.Channel).ToList(), + cancellationToken))); + } + /// public void RegisterCommandHandler(ICustomCommandHandler customCommandHandler) { diff --git a/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs index cb0265c334..6518f1c894 100644 --- a/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs @@ -84,5 +84,12 @@ namespace Tgstation.Server.Host.Components.Chat /// /// A new . IChatTrackingContext CreateTrackingContext(); + + /// + /// Force an update with the active channels on all active s. + /// + /// The for the operation. + /// A representing the running operation. + Task UpdateTrackingContexts(CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index e2b1a6b7dc..5188d636b3 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -384,8 +384,11 @@ namespace Tgstation.Server.Host.Components.Watchdog { if (core.Watchdog != this) throw new InvalidOperationException(Instance.DifferentCoreExceptionMessage); + using (await SemaphoreSlimContext.Lock(synchronizationSemaphore, ct)) await LaunchNoLock(true, true, true, reattachInfo, ct); + + await Chat.UpdateTrackingContexts(ct); }, cancellationToken) ; From ddc71308871f4737538a39a6e95bd15b9b5904c1 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 20 May 2023 19:23:32 -0400 Subject: [PATCH 03/41] Fix a lint --- tests/Tgstation.Server.Tests/Live/TestLiveServer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 096b4e8b12..abc6f25d6b 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -44,7 +44,7 @@ namespace Tgstation.Server.Tests.Live public static ushort DDPort { get; } = FreeTcpPort(); public static ushort DMPort { get; } = GetDMPort(); - readonly Version TestUpdateVersion = new Version(5, 11, 0); + readonly Version TestUpdateVersion = new(5, 11, 0); readonly IServerClientFactory clientFactory = new ServerClientFactory(new ProductHeaderValue(Assembly.GetExecutingAssembly().GetName().Name, Assembly.GetExecutingAssembly().GetName().Version.ToString())); From 1214cfbbfd9899b8a3fc53c7ac95defe5de31f5f Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 20 May 2023 19:30:18 -0400 Subject: [PATCH 04/41] Regression tests for #1490 --- tests/DMAPI/LongRunning/Test.dm | 20 +++++++++++++++++++ .../Live/TestLiveServer.cs | 19 ++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/tests/DMAPI/LongRunning/Test.dm b/tests/DMAPI/LongRunning/Test.dm index 14b927c7e8..e952d80426 100644 --- a/tests/DMAPI/LongRunning/Test.dm +++ b/tests/DMAPI/LongRunning/Test.dm @@ -35,6 +35,12 @@ for(var/i in 1 to 10000000) dab() TgsNew(new /datum/tgs_event_handler/impl, TGS_SECURITY_SAFE) + + var/list/channels = TgsChatChannelInfo() + if(!length(channels)) + text2file("Expected some chat channels!", "test_fail_reason.txt") + del(world) + StartAsync() /proc/dab() @@ -133,9 +139,17 @@ var/run_bridge_test // Bridge response queuing var/tactics6 = data["tgs_integration_test_tactics6"] if(tactics6) + if (length(world.TgsChatChannelInfo())) + return "channels_present!" + DetachedChatMessageQueuing() return "queued" + var/tactics7 = data["tgs_integration_test_tactics7"] + if(tactics7) + var/list/channels = TgsChatChannelInfo() + return "[length(channels)]" + TgsChatBroadcast(new /datum/tgs_message_content("Recieved non-tgs topic: `[T]`")) return "feck" @@ -162,6 +176,12 @@ var/run_bridge_test /datum/tgs_event_handler/impl/HandleEvent(event_code, ...) set waitfor = FALSE + if(event_code == TGS_EVENT_WATCHDOG_DETACH) + var/list/channels = world.TgsChatChannelInfo() + if(length(channels)) + text2file("Expected no chat channels after detach!", "test_fail_reason.txt") + del(world) + world.TgsChatBroadcast(new /datum/tgs_message_content("Recieved event: `[json_encode(args)]`")) /world/Export(url) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index abc6f25d6b..1d36255826 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -840,6 +840,25 @@ namespace Tgstation.Server.Tests.Live var dd = await instanceClient.DreamDaemon.Read(cancellationToken); Assert.AreEqual(WatchdogStatus.Online, dd.Status.Value); + var chatReadTask = instanceClient.ChatBots.List(null, cancellationToken); + + topicRequestResult = await WatchdogTest.TopicClient.SendTopic( + IPAddress.Loopback, + $"tgs_integration_test_tactics7=1", + DDPort, + cancellationToken); + + Assert.IsNotNull(topicRequestResult); + if(!Int32.TryParse(topicRequestResult.StringData, out var channelsPresent)) + { + Assert.Fail("Expected DD to send us an int!"); + } + + var currentChatBots = await chatReadTask; + var connectedChannelCount = currentChatBots.Where(x => x.Enabled.Value).SelectMany(x => x.Channels).Count(); + + Assert.AreEqual(connectedChannelCount, channelsPresent); + await instanceClient.DreamDaemon.Shutdown(cancellationToken); dd = await instanceClient.DreamDaemon.Update(new DreamDaemonRequest { From 95443b42aaa5e15db1e09bbef05125c059443980 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 20 May 2023 19:43:54 -0400 Subject: [PATCH 05/41] Add a test assert --- tests/Tgstation.Server.Tests/Live/TestLiveServer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 1d36255826..75caa1bcc8 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -732,6 +732,7 @@ namespace Tgstation.Server.Tests.Live using var httpClient = new HttpClient(); var webRequestTask = httpClient.GetAsync(server.Url.ToString() + "swagger/v1/swagger.json"); using var response = await webRequestTask; + response.EnsureSuccessStatusCode(); using var content = await response.Content.ReadAsStreamAsync(); using var output = new FileStream(@"C:\swagger.json", FileMode.Create); await content.CopyToAsync(output); From 51eb8348aebe3aef69c0c4eec56afa1733703d59 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 20 May 2023 20:17:25 -0400 Subject: [PATCH 06/41] Version bump to 5.12.2 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 1a4eb38d8a..bb6a17d416 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 5.12.1 + 5.12.2 4.6.0 9.10.2 10.4.1 From ee1d752f4dec2930c5006bb4fbc356c8329837a8 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 00:59:30 -0400 Subject: [PATCH 07/41] Do not let chat message queuing hold up the watchdog --- .../Components/Chat/ChatManager.cs | 55 +++++++++---- .../Components/Chat/IChatManager.cs | 4 +- .../Components/Watchdog/BasicWatchdog.cs | 36 ++++----- .../Components/Watchdog/WatchdogBase.cs | 81 +++++++------------ 4 files changed, 88 insertions(+), 88 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index a1623b312c..1869a4bb95 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -325,35 +325,33 @@ namespace Tgstation.Server.Host.Components.Chat if (channelIds == null) throw new ArgumentNullException(nameof(channelIds)); - var task = SendMessage( - channelIds, - null, - message, - handlerCts.Token); - AddMessageTask(task); + QueueMessageInternal(message, channelIds, false); } /// - public async Task QueueWatchdogMessage(string message, CancellationToken cancellationToken) + public void QueueWatchdogMessage(string message) { + if (message == null) + throw new ArgumentNullException(nameof(message)); + List wdChannels = null; message = String.Format(CultureInfo.InvariantCulture, "WD: {0}", message); if (!initialProviderConnectionsTask.IsCompleted) logger.LogTrace("Waiting for initial provider connections before sending watchdog message..."); - await initialProviderConnectionsTask.WithToken(cancellationToken); - // so it doesn't change while we're using it lock (mappedChannels) wdChannels = mappedChannels.Where(x => x.Value.IsWatchdogChannel).Select(x => x.Key).ToList(); - QueueMessage( + // Reimplementing QueueMessage + QueueMessageInternal( new MessageContent { Text = message, }, - wdChannels); + wdChannels, + true); } /// @@ -467,13 +465,16 @@ namespace Tgstation.Server.Host.Components.Chat /// public Task UpdateTrackingContexts(CancellationToken cancellationToken) { + async Task UpdateTrackingContext(IChannelSink channelSink, IEnumerable channels) + { + await initialProviderConnectionsTask.WithToken(cancellationToken); + await channelSink.UpdateChannels(channels, cancellationToken); + } + lock (mappedChannels) lock (trackingContexts) return Task.WhenAll( - trackingContexts.Select( - x => x.UpdateChannels( - mappedChannels.Select(y => y.Value.Channel).ToList(), - cancellationToken))); + trackingContexts.Select(x => UpdateTrackingContext(x, mappedChannels.Select(y => y.Value.Channel).ToList()))); } /// @@ -1014,5 +1015,29 @@ namespace Tgstation.Server.Host.Components.Chat lock (handlerCts) messageSendTask = Wrap(messageSendTask); } + + /// + /// Adds a given to the send queue. + /// + /// The being sent. + /// The s of the s to send to. + /// If , the message send will wait for to complete before running. + void QueueMessageInternal(MessageContent message, IEnumerable channelIds, bool waitForConnections) + { + async Task SendMessageTask() + { + var cancellationToken = handlerCts.Token; + if (waitForConnections) + await initialProviderConnectionsTask.WithToken(cancellationToken); + + await SendMessage( + channelIds, + null, + message, + cancellationToken); + } + + AddMessageTask(SendMessageTask()); + } } } diff --git a/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs index 6518f1c894..59f3e2b1c5 100644 --- a/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/IChatManager.cs @@ -57,9 +57,7 @@ namespace Tgstation.Server.Host.Components.Chat /// Queue a chat to configured watchdog channels. /// /// The message being sent. - /// The for the operation. - /// A representing the running operation. - Task QueueWatchdogMessage(string message, CancellationToken cancellationToken); + void QueueWatchdogMessage(string message); /// /// Send the message for a deployment to configured deployment channels. diff --git a/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs index 5445ae311c..b8f23b2339 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs @@ -118,22 +118,19 @@ namespace Tgstation.Server.Host.Components.Watchdog if (Server.RebootState == Session.RebootState.Shutdown) { // the time for graceful shutdown is now - await Chat.QueueWatchdogMessage( + Chat.QueueWatchdogMessage( String.Format( CultureInfo.InvariantCulture, "Server {0}! Shutting down due to graceful termination request...", - exitWord), - cancellationToken) - ; + exitWord)); return MonitorAction.Exit; } - await Chat.QueueWatchdogMessage( + Chat.QueueWatchdogMessage( String.Format( CultureInfo.InvariantCulture, "Server {0}! Rebooting...", - exitWord), - cancellationToken); + exitWord)); return MonitorAction.Restart; case MonitorActivationReason.ActiveServerRebooted: var rebootState = Server.RebootState; @@ -156,10 +153,8 @@ namespace Tgstation.Server.Host.Components.Watchdog return MonitorAction.Restart; case Session.RebootState.Shutdown: // graceful shutdown time - await Chat.QueueWatchdogMessage( - "Active server rebooted! Shutting down due to graceful termination request...", - cancellationToken) - ; + Chat.QueueWatchdogMessage( + "Active server rebooted! Shutting down due to graceful termination request..."); return MonitorAction.Exit; default: throw new InvalidOperationException($"Invalid reboot state: {rebootState}"); @@ -200,7 +195,7 @@ namespace Tgstation.Server.Host.Components.Watchdog /// protected override async Task InitController( - Task chatTask, + Task eventTask, ReattachInformation reattachInfo, CancellationToken cancellationToken) { @@ -221,7 +216,7 @@ namespace Tgstation.Server.Host.Components.Watchdog await BeforeApplyDmb(dmbToUse.CompileJob, cancellationToken); dmbToUse = await PrepServerForLaunch(dmbToUse, cancellationToken); - await chatTask; + await eventTask; serverLaunchTask = SessionControllerFactory.LaunchNew( dmbToUse, null, @@ -231,7 +226,7 @@ namespace Tgstation.Server.Host.Components.Watchdog } else { - await chatTask; + await eventTask; serverLaunchTask = SessionControllerFactory.Reattach(reattachInfo, cancellationToken); } @@ -299,14 +294,17 @@ namespace Tgstation.Server.Host.Components.Watchdog /// /// The for the operation. /// A representing the running operation. - protected virtual Task HandleNewDmbAvailable(CancellationToken cancellationToken) + protected virtual async Task HandleNewDmbAvailable(CancellationToken cancellationToken) { gracefulRebootRequired = true; if (Server.CompileJob.DMApiVersion == null) - return Chat.QueueWatchdogMessage( - "A new deployment has been made but cannot be applied automatically as the currently running server has no DMAPI. Please manually reboot the server to apply the update.", - cancellationToken); - return Server.SetRebootState(Session.RebootState.Restart, cancellationToken); + { + Chat.QueueWatchdogMessage( + "A new deployment has been made but cannot be applied automatically as the currently running server has no DMAPI. Please manually reboot the server to apply the update."); + return; + } + + await Server.SetRebootState(Session.RebootState.Restart, cancellationToken); } /// diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 5188d636b3..aceb6c0fcf 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -347,10 +347,9 @@ namespace Tgstation.Server.Host.Components.Watchdog { if (!graceful) { - var chatTask = Chat.QueueWatchdogMessage("Manual restart triggered...", cancellationToken); + Chat.QueueWatchdogMessage("Manual restart triggered..."); await TerminateNoLock(false, false, cancellationToken); await LaunchNoLock(true, false, true, null, cancellationToken); - await chatTask; return; } @@ -425,7 +424,7 @@ namespace Tgstation.Server.Host.Components.Watchdog releaseServers = true; if (Status == WatchdogStatus.Online) - await Chat.QueueWatchdogMessage("Detaching...", cancellationToken); + Chat.QueueWatchdogMessage("Detaching..."); else Logger.LogTrace("Not sending detach chat message as status is: {status}", Status); } @@ -477,11 +476,11 @@ namespace Tgstation.Server.Host.Components.Watchdog /// /// Starts all s. /// - /// A, possibly active, for an outgoing chat message. + /// A, possibly active, for an event that's running. /// to use, if any. /// The for the operation. /// A representing the running operation. - protected abstract Task InitController(Task chatTask, ReattachInformation reattachInfo, CancellationToken cancellationToken); + protected abstract Task InitController(Task eventTask, ReattachInformation reattachInfo, CancellationToken cancellationToken); /// /// Launches the watchdog. @@ -507,21 +506,16 @@ namespace Tgstation.Server.Host.Components.Watchdog throw new JobException(ErrorCode.WatchdogCompileJobCorrupted); // this is necessary, the monitor could be in it's sleep loop trying to restart, if so cancel THAT monitor and start our own with blackjack and hookers - Task announceTask; + var eventTask = Task.CompletedTask; if (announce) { - announceTask = Chat.QueueWatchdogMessage( + Chat.QueueWatchdogMessage( reattachInfo == null ? "Launching..." - : "Reattaching...", - cancellationToken); // simple announce + : "Reattaching..."); // simple announce if (reattachInfo == null) - announceTask = Task.WhenAll( - HandleEvent(EventType.WatchdogLaunch, Enumerable.Empty(), false, cancellationToken), - announceTask); + eventTask = HandleEvent(EventType.WatchdogLaunch, Enumerable.Empty(), false, cancellationToken); } - else - announceTask = Task.CompletedTask; // no announce // since neither server is running, this is safe to do LastLaunchParameters = ActiveLaunchParameters; @@ -529,7 +523,7 @@ namespace Tgstation.Server.Host.Components.Watchdog try { - await InitController(announceTask, reattachInfo, cancellationToken); + await InitController(eventTask, reattachInfo, cancellationToken); } catch (OperationCanceledException ex) { @@ -539,15 +533,15 @@ namespace Tgstation.Server.Host.Components.Watchdog catch (Exception e) { Logger.LogWarning(e, "Failed to start watchdog!"); - var originalChatTask = announceTask; - async Task ChainChatTaskWithErrorMessage() + var originalChatTask = eventTask; + async Task ChainEventTaskWithErrorMessage() { await originalChatTask; if (announceFailure) - await Chat.QueueWatchdogMessage("Startup failed!", cancellationToken); + Chat.QueueWatchdogMessage("Startup failed!"); } - announceTask = ChainChatTaskWithErrorMessage(); + eventTask = ChainEventTaskWithErrorMessage(); throw; } finally @@ -555,7 +549,7 @@ namespace Tgstation.Server.Host.Components.Watchdog // finish the chat task that's in flight try { - await announceTask; + await eventTask; } catch (OperationCanceledException ex) { @@ -626,8 +620,8 @@ namespace Tgstation.Server.Host.Components.Watchdog const string FailReattachMessage = "Unable to properly reattach to server! Restarting watchdog..."; Logger.LogWarning(FailReattachMessage); - var chatTask = Chat.QueueWatchdogMessage(FailReattachMessage, cancellationToken); - await InitController(chatTask, null, cancellationToken); + Chat.QueueWatchdogMessage(FailReattachMessage); + await InitController(Task.CompletedTask, null, cancellationToken); } /// @@ -730,7 +724,6 @@ namespace Tgstation.Server.Host.Components.Watchdog await DisposeAndNullControllers(cancellationToken); - var chatTask = Task.CompletedTask; for (var retryAttempts = 1; ; ++retryAttempts) { Status = WatchdogStatus.Restoring; @@ -748,10 +741,6 @@ namespace Tgstation.Server.Host.Components.Watchdog { launchException = e; } - finally - { - await chatTask; - } Logger.LogWarning(launchException, "Failed to automatically restart the watchdog! Attempt: {attemptNumber}", retryAttempts); Status = WatchdogStatus.DelayedRestart; @@ -761,16 +750,12 @@ namespace Tgstation.Server.Host.Components.Watchdog Math.Pow(2, retryAttempts)), TimeSpan.FromHours(1).TotalSeconds); // max of one hour, increasing by a power of 2 each time - chatTask = Chat.QueueWatchdogMessage( - $"Failed to restart (Attempt: {retryAttempts}), retrying in {retryDelay}s...", - cancellationToken); + Chat.QueueWatchdogMessage( + $"Failed to restart (Attempt: {retryAttempts}), retrying in {retryDelay}s..."); - await Task.WhenAll( - AsyncDelayer.Delay( - TimeSpan.FromSeconds(retryDelay), - cancellationToken), - chatTask) - ; + await AsyncDelayer.Delay( + TimeSpan.FromSeconds(retryDelay), + cancellationToken); } } @@ -966,9 +951,8 @@ namespace Tgstation.Server.Host.Components.Watchdog var nextActionMessage = nextAction != MonitorAction.Exit ? "Recovering" : "Shutting down"; - var chatTask = Chat.QueueWatchdogMessage( - $"Monitor crashed, this should NEVER happen! Please report this, full details in logs! {nextActionMessage}. Error: {e.Message}", - cancellationToken); + Chat.QueueWatchdogMessage( + $"Monitor crashed, this should NEVER happen! Please report this, full details in logs! {nextActionMessage}. Error: {e.Message}"); if (disposed) nextAction = MonitorAction.Exit; @@ -980,8 +964,6 @@ namespace Tgstation.Server.Host.Components.Watchdog Logger.LogDebug("Server seems to be okay, not restarting"); nextAction = MonitorAction.Continue; } - - await chatTask; } } catch (OperationCanceledException) @@ -1026,15 +1008,14 @@ namespace Tgstation.Server.Host.Components.Watchdog releaseServers, cancellationToken); - var chatTask = announce ? Chat.QueueWatchdogMessage("Shutting down...", cancellationToken) : Task.CompletedTask; + if (announce) + Chat.QueueWatchdogMessage("Shutting down..."); await eventTask; await StopMonitor(); LastLaunchParameters = null; - - await chatTask; return; } @@ -1071,7 +1052,7 @@ namespace Tgstation.Server.Host.Components.Watchdog case 2: const string message2 = "DEFCON 3: DreamDaemon has missed 2 heartbeats!"; Logger.LogInformation(message2); - await Chat.QueueWatchdogMessage(message2, cancellationToken); + Chat.QueueWatchdogMessage(message2); break; case 3: var actionToTake = shouldShutdown @@ -1079,12 +1060,11 @@ namespace Tgstation.Server.Host.Components.Watchdog : "be restarted"; const string logTemplate1 = "DEFCON 2: DreamDaemon has missed 3 heartbeats! If it does not respond to the next one, the watchdog will {actionToTake}!"; Logger.LogWarning(logTemplate1, actionToTake); - await Chat.QueueWatchdogMessage( + Chat.QueueWatchdogMessage( logTemplate1.Replace( "{actionToTake}", actionToTake, - StringComparison.Ordinal), - cancellationToken); + StringComparison.Ordinal)); break; case 4: var actionTaken = shouldShutdown @@ -1092,12 +1072,11 @@ namespace Tgstation.Server.Host.Components.Watchdog : "Restarting"; const string logTemplate2 = "DEFCON 1: Four heartbeats have been missed! {actionTaken}..."; Logger.LogWarning(logTemplate2, actionTaken); - await Chat.QueueWatchdogMessage( + Chat.QueueWatchdogMessage( logTemplate2.Replace( "{actionTaken}", actionTaken, - StringComparison.Ordinal), - cancellationToken); + StringComparison.Ordinal)); if (ActiveLaunchParameters.DumpOnHeartbeatRestart.Value) { From 42a7dab56af5e93928875ea8d9b88e163976a054 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 01:44:15 -0400 Subject: [PATCH 08/41] ChatChannelInfo will now try to delay until the reattach updates it --- src/DMAPI/tgs/v5/api.dm | 1 + src/DMAPI/tgs/v5/bridge.dm | 10 +++++++--- tests/DMAPI/LongRunning/Test.dm | 9 ++++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/DMAPI/tgs/v5/api.dm b/src/DMAPI/tgs/v5/api.dm index 517240f12f..25bffb7850 100644 --- a/src/DMAPI/tgs/v5/api.dm +++ b/src/DMAPI/tgs/v5/api.dm @@ -199,6 +199,7 @@ /datum/tgs_api/v5/ChatChannelInfo() RequireInitialBridgeResponse() + WaitForReattach(TRUE) return chat_channels.Copy() /datum/tgs_api/v5/proc/DecodeChannels(chat_update_json) diff --git a/src/DMAPI/tgs/v5/bridge.dm b/src/DMAPI/tgs/v5/bridge.dm index b3cf775939..37f58bcdf6 100644 --- a/src/DMAPI/tgs/v5/bridge.dm +++ b/src/DMAPI/tgs/v5/bridge.dm @@ -59,18 +59,22 @@ var/json = json_encode(data) return json -/datum/tgs_api/v5/proc/PerformBridgeRequest(bridge_request) +/datum/tgs_api/v5/proc/WaitForReattach(require_channels = FALSE) if(detached) // Wait up to one minute for(var/i in 1 to 600) sleep(1) - if(!detached) + if(!detached && (!require_channels || length(chat_channels))) break - // dad went out for milk cigarettes 20 years ago... + // dad went out for milk and cigarettes 20 years ago... + // yes, this affects all other waiters, intentional if(i == 600) detached = FALSE +/datum/tgs_api/v5/proc/PerformBridgeRequest(bridge_request) + WaitForReattach(FALSE) + // This is an infinite sleep until we get a response var/export_response = world.Export(bridge_request) if(!export_response) diff --git a/tests/DMAPI/LongRunning/Test.dm b/tests/DMAPI/LongRunning/Test.dm index e952d80426..bdf85acc45 100644 --- a/tests/DMAPI/LongRunning/Test.dm +++ b/tests/DMAPI/LongRunning/Test.dm @@ -139,7 +139,9 @@ var/run_bridge_test // Bridge response queuing var/tactics6 = data["tgs_integration_test_tactics6"] if(tactics6) - if (length(world.TgsChatChannelInfo())) + // hack hack, calling world.TgsChatChannelInfo() will try to delay until the channels come back + var/datum/tgs_api/v5/api = TGS_READ_GLOBAL(tgs) + if (length(api.chat_channels)) return "channels_present!" DetachedChatMessageQueuing() @@ -177,8 +179,9 @@ var/run_bridge_test set waitfor = FALSE if(event_code == TGS_EVENT_WATCHDOG_DETACH) - var/list/channels = world.TgsChatChannelInfo() - if(length(channels)) + // hack hack, calling world.TgsChatChannelInfo() will try to delay until the channels come back + var/datum/tgs_api/v5/api = TGS_READ_GLOBAL(tgs) + if(length(api.chat_channels)) text2file("Expected no chat channels after detach!", "test_fail_reason.txt") del(world) From 65a4d929b1dab201034e8aeb874ef0ff3d66bedb Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 02:14:53 -0400 Subject: [PATCH 09/41] Informative comment --- tests/Tgstation.Server.Tests/Live/TestLiveServer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 75caa1bcc8..1734afef0f 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -843,6 +843,7 @@ namespace Tgstation.Server.Tests.Live var chatReadTask = instanceClient.ChatBots.List(null, cancellationToken); + // Check the DMAPI got the channels again https://github.com/tgstation/tgstation-server/issues/1490 topicRequestResult = await WatchdogTest.TopicClient.SendTopic( IPAddress.Loopback, $"tgs_integration_test_tactics7=1", From 522df7b9d986f226564e9f51d7c82e7e74bbe89c Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 02:15:05 -0400 Subject: [PATCH 10/41] More logging for chat context updates --- .../Components/Chat/ChatManager.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 1869a4bb95..7b518ab2dc 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -465,16 +465,25 @@ namespace Tgstation.Server.Host.Components.Chat /// public Task UpdateTrackingContexts(CancellationToken cancellationToken) { - async Task UpdateTrackingContext(IChannelSink channelSink, IEnumerable channels) + var logMessageSent = 0; + async Task UpdateTrackingContext(IChatTrackingContext channelSink, IEnumerable channels) { await initialProviderConnectionsTask.WithToken(cancellationToken); + if (Interlocked.Exchange(ref logMessageSent, 1) == 0) + logger.LogTrace("Updating chat tracking contexts..."); + await channelSink.UpdateChannels(channels, cancellationToken); } + List tasks; lock (mappedChannels) lock (trackingContexts) - return Task.WhenAll( - trackingContexts.Select(x => UpdateTrackingContext(x, mappedChannels.Select(y => y.Value.Channel).ToList()))); + tasks = trackingContexts.Select(x => UpdateTrackingContext(x, mappedChannels.Select(y => y.Value.Channel).ToList())).ToList(); + + if (tasks.Count > 0 && !initialProviderConnectionsTask.IsCompleted) + logger.LogTrace("Waiting for initial chat bot connections before updating tracking contexts..."); + + return Task.WhenAll(tasks); } /// From 0eb2fa7dbce8756a864858d0f08c94c428255b2c Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 02:19:34 -0400 Subject: [PATCH 11/41] Fix location of the call site to UpdateTrackingContexts in ChatManager --- src/Tgstation.Server.Host/Components/Chat/ChatManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 7b518ab2dc..bb17abfcfc 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -245,13 +245,13 @@ namespace Tgstation.Server.Host.Components.Chat newMapping.Channel.RealId = newId; } } - - await UpdateTrackingContexts(cancellationToken); } finally { provider.InitialMappingComplete(); } + + await UpdateTrackingContexts(cancellationToken); } /// From dd7eead686fad1fd694278fe2f81b0e198106b98 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 04:17:53 -0400 Subject: [PATCH 12/41] Try fixing the order of operations again --- .../Components/Chat/ChatManager.cs | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index bb17abfcfc..05bc3f3c3f 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -245,13 +245,17 @@ namespace Tgstation.Server.Host.Components.Chat newMapping.Channel.RealId = newId; } } + + // we only want to update contexts if everything at startup has connected once already + // otherwise we could send an incomplete channel set to the DMAPI, which will then spout all its queued messages into it instead of all relevant chatbots + // The watchdog can call this if it needs to after starting up + if (initialProviderConnectionsTask.IsCompleted) + await UpdateTrackingContexts(cancellationToken); } finally { provider.InitialMappingComplete(); } - - await UpdateTrackingContexts(cancellationToken); } /// @@ -463,27 +467,35 @@ namespace Tgstation.Server.Host.Components.Chat } /// - public Task UpdateTrackingContexts(CancellationToken cancellationToken) + public async Task UpdateTrackingContexts(CancellationToken cancellationToken) { var logMessageSent = 0; async Task UpdateTrackingContext(IChatTrackingContext channelSink, IEnumerable channels) { - await initialProviderConnectionsTask.WithToken(cancellationToken); if (Interlocked.Exchange(ref logMessageSent, 1) == 0) - logger.LogTrace("Updating chat tracking contexts..."); await channelSink.UpdateChannels(channels, cancellationToken); } + var waitingForInitialConnection = !initialProviderConnectionsTask.IsCompleted; + if (waitingForInitialConnection) + { + logger.LogTrace("Waiting for initial chat bot connections before updating tracking contexts..."); + await initialProviderConnectionsTask.WithToken(cancellationToken); + } + List tasks; lock (mappedChannels) lock (trackingContexts) - tasks = trackingContexts.Select(x => UpdateTrackingContext(x, mappedChannels.Select(y => y.Value.Channel).ToList())).ToList(); + tasks = trackingContexts.Select(x => UpdateTrackingContext(x, mappedChannels.Select(y => y.Value.Channel))).ToList(); - if (tasks.Count > 0 && !initialProviderConnectionsTask.IsCompleted) - logger.LogTrace("Waiting for initial chat bot connections before updating tracking contexts..."); + if (waitingForInitialConnection) + if (tasks.Count > 0) + logger.LogTrace("Updating chat tracking contexts..."); + else + logger.LogTrace("No chat tracking contexts to update"); - return Task.WhenAll(tasks); + await Task.WhenAll(tasks); } /// From 328d41fbe1be033ce060ebb77d36e7d08756af4b Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 11:46:06 -0400 Subject: [PATCH 13/41] Minor lint fix --- .../Components/Chat/ChatTrackingContext.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs b/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs index 8239bdb0e1..119fb5573f 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs @@ -21,7 +21,11 @@ namespace Tgstation.Server.Host.Components.Chat { if (active == value) return; - logger.LogTrace(value ? "Activated" : "Deactivated"); + if (value) + logger.LogTrace("Activated"); + else + logger.LogTrace("Deactivated"); + active = value; } } From fda5713dbdf7444753f4f246bdc2a988bb2cf977 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sun, 21 May 2023 13:34:25 -0400 Subject: [PATCH 14/41] Generate more bot tokens for better chat testing --- .github/workflows/ci-suite.yml | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci-suite.yml b/.github/workflows/ci-suite.yml index 9253d15ddd..cd9e7b5efa 100644 --- a/.github/workflows/ci-suite.yml +++ b/.github/workflows/ci-suite.yml @@ -18,9 +18,7 @@ on: env: TGS_DOTNET_VERSION: 6.0.x TGS_TEST_DISCORD_CHANNEL: ${{ secrets.DISCORD_CHANNEL_ID }} - TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} TGS_TEST_IRC_CHANNEL: ${{ secrets.IRC_CHANNEL }} - TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} TGS_TEST_GITHUB_TOKEN: ${{ secrets.LIVE_TESTS_TOKEN }} TGS_RELEASE_NOTES_TOKEN: ${{ secrets.DEV_PUSH_TOKEN }} @@ -308,11 +306,11 @@ jobs: needs: dmapi-build if: "!(cancelled() || failure()) && needs.dmapi-build.result == 'success'" env: - TGS_TEST_DATABASE_TYPE: SqlServer TGS_TEST_DUMP_API_SPEC: yes strategy: fail-fast: false matrix: + database-type: [ 'SqlServer' ] watchdog-type: [ 'Basic', 'System' ] configuration: [ 'Debug', 'Release' ] runs-on: windows-2019 @@ -329,11 +327,15 @@ jobs: if: ${{ matrix.watchdog-type == 'Basic' }} run: echo "General__UseBasicWatchdog=true" >> $Env:GITHUB_ENV - - name: Set TGS_TEST_CONNECTION_STRING + - name: Set SqlServer Connection Info + if: ${{ matrix.database-type == 'SqlServer' }} shell: bash run: | TGS_CONNSTRING_VALUE="Server=(localdb)\MSSQLLocalDB;Integrated Security=true;Initial Catalog=TGS_${{ matrix.watchdog-type }}_${{ matrix.configuration }};Application Name=tgstation-server" echo "TGS_TEST_CONNECTION_STRING=$(echo $TGS_CONNSTRING_VALUE)" >> $GITHUB_ENV + echo "TGS_TEST_DATABASE_TYPE=SqlServer" >> $GITHUB_ENV + echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_WINDOWS_SQLSERVER }}" >> $GITHUB_ENV + echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_WINDOWS_SQLSERVER }}" >> $GITHUB_ENV - name: Checkout (Branch Push) uses: actions/checkout@v3 @@ -361,7 +363,7 @@ jobs: path: ./TestResults/ - name: Store OpenAPI Spec - if: ${{ matrix.configuration == 'Release' && matrix.watchdog-type == 'System' }} + if: ${{ matrix.configuration == 'Release' && matrix.watchdog-type == 'System' && matrix.database-type == 'SqlServer' }} uses: actions/upload-artifact@v3 with: name: openapi-spec @@ -456,18 +458,24 @@ jobs: run: | echo "TGS_TEST_DATABASE_TYPE=Sqlite" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Data Source=TGS_${{ matrix.watchdog-type }}_${{ matrix.configuration }}.sqlite3;Mode=ReadWriteCreate" >> $GITHUB_ENV + echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_SQLITE }}" >> $GITHUB_ENV + echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_SQLITE }}" >> $GITHUB_ENV - name: Set PostgresSql Connection Info if: ${{ matrix.database-type == 'PostgresSql' }} run: | echo "TGS_TEST_DATABASE_TYPE=PostgresSql" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Application Name=tgstation-server;Host=127.0.0.1;Username=postgres;Password=postgres;Database=TGS__${{ matrix.watchdog-type }}_${{ matrix.configuration }}" >> $GITHUB_ENV + echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_POSTGRES }}" >> $GITHUB_ENV + echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_POSTGRES }}" >> $GITHUB_ENV - name: Set MariaDB Connection Info if: ${{ matrix.database-type == 'MariaDB' }} run: | echo "TGS_TEST_DATABASE_TYPE=MariaDB" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Server=127.0.0.1;uid=root;pwd=mariadb;database=tgs__${{ matrix.watchdog-type }}_${{ matrix.configuration }}" >> $GITHUB_ENV + echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_MARIADB }}" >> $GITHUB_ENV + echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_MARIADB }}" >> $GITHUB_ENV - name: Set MySQL Connection Info if: ${{ matrix.database-type == 'MySql' }} @@ -475,6 +483,8 @@ jobs: echo "TGS_TEST_DATABASE_TYPE=MySql" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Server=127.0.0.1;Port=3307;uid=root;pwd=mysql;database=tgs__${{ matrix.watchdog-type }}_${{ matrix.configuration }}" >> $GITHUB_ENV echo "Database__ServerVersion=5.7.31" >> $GITHUB_ENV + echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_MYSQL }}" >> $GITHUB_ENV + echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_MYSQL }}" >> $GITHUB_ENV - name: Set General__UseBasicWatchdog if: ${{ matrix.watchdog-type == 'Basic' }} From 234d848d83d93c28aacb79cc4300c137648d204b Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 17:10:36 -0400 Subject: [PATCH 15/41] Fix watchdog chat message send race condition on startup --- .../Components/Chat/ChatManager.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 05bc3f3c3f..d9bb4fa5f7 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -329,7 +329,7 @@ namespace Tgstation.Server.Host.Components.Chat if (channelIds == null) throw new ArgumentNullException(nameof(channelIds)); - QueueMessageInternal(message, channelIds, false); + QueueMessageInternal(message, () => channelIds, false); } /// @@ -338,23 +338,23 @@ namespace Tgstation.Server.Host.Components.Chat if (message == null) throw new ArgumentNullException(nameof(message)); - List wdChannels = null; message = String.Format(CultureInfo.InvariantCulture, "WD: {0}", message); if (!initialProviderConnectionsTask.IsCompleted) logger.LogTrace("Waiting for initial provider connections before sending watchdog message..."); - // so it doesn't change while we're using it - lock (mappedChannels) - wdChannels = mappedChannels.Where(x => x.Value.IsWatchdogChannel).Select(x => x.Key).ToList(); - // Reimplementing QueueMessage QueueMessageInternal( new MessageContent { Text = message, }, - wdChannels, + () => + { + // so it doesn't change while we're using it + lock (mappedChannels) + return mappedChannels.Where(x => x.Value.IsWatchdogChannel).Select(x => x.Key).ToList(); + }, true); } @@ -1041,9 +1041,9 @@ namespace Tgstation.Server.Host.Components.Chat /// Adds a given to the send queue. /// /// The being sent. - /// The s of the s to send to. + /// A to retrieve he s of the s to send to. /// If , the message send will wait for to complete before running. - void QueueMessageInternal(MessageContent message, IEnumerable channelIds, bool waitForConnections) + void QueueMessageInternal(MessageContent message, Func> channelIdsFactory, bool waitForConnections) { async Task SendMessageTask() { @@ -1052,7 +1052,7 @@ namespace Tgstation.Server.Host.Components.Chat await initialProviderConnectionsTask.WithToken(cancellationToken); await SendMessage( - channelIds, + channelIdsFactory(), null, message, cancellationToken); From b9a3a4121236cddd4b5f8a32ae77975a2ce0fa16 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 17:20:30 -0400 Subject: [PATCH 16/41] Do not attempt to send a message to no channels Fix in both chat manager and DMAPI --- src/DMAPI/tgs/v4/api.dm | 10 ++++++++++ src/DMAPI/tgs/v5/api.dm | 8 ++++++++ .../Components/Chat/ChatManager.cs | 4 ++++ 3 files changed, 22 insertions(+) diff --git a/src/DMAPI/tgs/v4/api.dm b/src/DMAPI/tgs/v4/api.dm index 2f05c38633..b9a75c4abb 100644 --- a/src/DMAPI/tgs/v4/api.dm +++ b/src/DMAPI/tgs/v4/api.dm @@ -263,7 +263,12 @@ for(var/I in channels) var/datum/tgs_chat_channel/channel = I ids += channel.id + message = UpgradeDeprecatedChatMessage(message) + + if (!length(channels)) + return + message = list("message" = message.text, "channelIds" = ids) if(intercepted_message_queue) intercepted_message_queue += list(message) @@ -276,7 +281,12 @@ var/datum/tgs_chat_channel/channel = I if (!channel.is_private_channel && ((channel.is_admin_channel && admin_only) || (!channel.is_admin_channel && !admin_only))) channels += channel.id + message = UpgradeDeprecatedChatMessage(message) + + if (!length(channels)) + return + message = list("message" = message.text, "channelIds" = channels) if(intercepted_message_queue) intercepted_message_queue += list(message) diff --git a/src/DMAPI/tgs/v5/api.dm b/src/DMAPI/tgs/v5/api.dm index 25bffb7850..926ea10a8f 100644 --- a/src/DMAPI/tgs/v5/api.dm +++ b/src/DMAPI/tgs/v5/api.dm @@ -166,6 +166,10 @@ ids += channel.id message = UpgradeDeprecatedChatMessage(message) + + if (!length(channels)) + return + message = message._interop_serialize() message[DMAPI5_CHAT_MESSAGE_CHANNEL_IDS] = ids if(intercepted_message_queue) @@ -181,6 +185,10 @@ channels += channel.id message = UpgradeDeprecatedChatMessage(message) + + if (!length(channels)) + return + message = message._interop_serialize() message[DMAPI5_CHAT_MESSAGE_CHANNEL_IDS] = channels if(intercepted_message_queue) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index d9bb4fa5f7..13196b3f57 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -983,6 +983,10 @@ namespace Tgstation.Server.Host.Components.Chat /// A representing the running operation. Task SendMessage(IEnumerable channelIds, Message replyTo, MessageContent message, CancellationToken cancellationToken) { + channelIds = channelIds.ToList(); + if (!channelIds.Any()) + return Task.CompletedTask; + logger.LogTrace( "Chat send \"{message}\"{embed} to channels: {channelIdsCommaSeperated}", message.Text, From 8784b955c177f5ccd344361a64cf89a10ea40677 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 17:21:20 -0400 Subject: [PATCH 17/41] Minor code cleanup --- .../Components/Watchdog/WatchdogBase.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index aceb6c0fcf..2c5d0ba569 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -364,7 +364,8 @@ namespace Tgstation.Server.Host.Components.Watchdog public async Task StartAsync(CancellationToken cancellationToken) { var reattachInfo = await SessionPersistor.Load(cancellationToken); - if (!autoStart && reattachInfo == null) + var reattaching = reattachInfo != null; + if (!autoStart && !reattaching) return; var job = new Models.Job @@ -373,7 +374,7 @@ namespace Tgstation.Server.Host.Components.Watchdog { Id = metadata.Id, }, - Description = $"Instance startup watchdog {(reattachInfo != null ? "reattach" : "launch")}", + Description = $"Instance startup watchdog {(reattaching ? "reattach" : "launch")}", CancelRight = (ulong)DreamDaemonRights.Shutdown, CancelRightsType = RightsType.DreamDaemon, }; From 97f9010b83d9cab9bde5549aacc3edde93adcc33 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 17:59:19 -0400 Subject: [PATCH 18/41] Remove useless comment --- src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs index b8f23b2339..f7bc152132 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs @@ -241,7 +241,6 @@ namespace Tgstation.Server.Host.Components.Watchdog return; } - // Server.AdjustPriority(true); if (!reattachInProgress) await SessionStartupPersist(cancellationToken); From f8bd4a011f97759cb92524b4b07ce46aca767593 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 18:15:45 -0400 Subject: [PATCH 19/41] Attempted workaround for a strange DD crash --- .../Components/Session/SessionController.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Session/SessionController.cs b/src/Tgstation.Server.Host/Components/Session/SessionController.cs index 82c0063193..f5e1491594 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionController.cs @@ -219,7 +219,11 @@ namespace Tgstation.Server.Host.Components.Session rebootTcs = new TaskCompletionSource(); primeTcs = new TaskCompletionSource(); - initialBridgeRequestTcs = new TaskCompletionSource(); + + // Run this asynchronously because we want to try to avoid any effects sending topics to the server while the initial bridge request is processing + // It MAY be the source of a DD crash. See this gist https://gist.github.com/Cyberboss/7776bbeff3a957d76affe0eae95c9f14 + // Worth further investigation as to if that sequence of events is a reliable crash vector and opening a BYOND bug if it is + initialBridgeRequestTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); reattachTopicCts = new CancellationTokenSource(); synchronizationLock = new object(); From 49c6f3769386dca6ae34e1fc6f86b2265994a344 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 19:11:06 -0400 Subject: [PATCH 20/41] Add test helpers to test .dmes - Make long_running_test_copy.dme just include the original --- tests/DMAPI/ApiFree/Test.dm | 7 +--- tests/DMAPI/ApiFree/api_free.dme | 1 + tests/DMAPI/BasicOperation/Test.dm | 8 +--- .../BasicOperation/basic_operation_test.dme | 2 +- tests/DMAPI/LongRunning/Test.dm | 39 +++---------------- tests/DMAPI/LongRunning/long_running_test.dme | 2 +- .../LongRunning/long_running_test_copy.dme | 4 +- .../DMAPI/{tgs_include.dm => test_prelude.dm} | 1 + tests/DMAPI/test_setup.dm | 32 +++++++++++++++ tgstation-server.sln | 5 ++- 10 files changed, 48 insertions(+), 53 deletions(-) rename tests/DMAPI/{tgs_include.dm => test_prelude.dm} (75%) create mode 100644 tests/DMAPI/test_setup.dm diff --git a/tests/DMAPI/ApiFree/Test.dm b/tests/DMAPI/ApiFree/Test.dm index b7b4ae5535..a14f28aef0 100644 --- a/tests/DMAPI/ApiFree/Test.dm +++ b/tests/DMAPI/ApiFree/Test.dm @@ -1,7 +1,2 @@ -/world/New() - text2file("SUCCESS", "test_success.txt") +/world/proc/RunTest() log << "Hello world!" - -/world/Error(exception) - fdel("test_success.txt") - text2file("Runtime Error: [exception]", "test_fail_reason.txt") diff --git a/tests/DMAPI/ApiFree/api_free.dme b/tests/DMAPI/ApiFree/api_free.dme index 444e2491d8..8179f9b698 100644 --- a/tests/DMAPI/ApiFree/api_free.dme +++ b/tests/DMAPI/ApiFree/api_free.dme @@ -11,5 +11,6 @@ // END_PREFERENCES // BEGIN_INCLUDE +#include "../test_setup.dm" #include "Test.dm" // END_INCLUDE diff --git a/tests/DMAPI/BasicOperation/Test.dm b/tests/DMAPI/BasicOperation/Test.dm index 42d311056c..022088fc67 100644 --- a/tests/DMAPI/BasicOperation/Test.dm +++ b/tests/DMAPI/BasicOperation/Test.dm @@ -1,4 +1,4 @@ -/world/New() +/world/proc/RunTest() text2file("SUCCESS", "test_success.txt") log << "About to call TgsNew()" sleep_offline = FALSE @@ -6,10 +6,6 @@ log << "About to call StartAsync()" StartAsync() -/world/Error(exception) - fdel("test_success.txt") - text2file("Runtime Error: [exception]", "test_fail_reason.txt") - /proc/StartAsync() set waitfor = FALSE Run() @@ -21,7 +17,7 @@ var/list/world_params = world.params if(!("test" in world_params) || world_params["test"] != "bababooey") - text2file("Expected parameter test=bababooey but did not receive", "test_fail_reason.txt") + FailTest("Expected parameter test=bababooey but did not receive", "test_fail_reason.txt") world.log << "sleep2" sleep(150) diff --git a/tests/DMAPI/BasicOperation/basic_operation_test.dme b/tests/DMAPI/BasicOperation/basic_operation_test.dme index 92781c7e16..b927014455 100644 --- a/tests/DMAPI/BasicOperation/basic_operation_test.dme +++ b/tests/DMAPI/BasicOperation/basic_operation_test.dme @@ -12,6 +12,6 @@ // BEGIN_INCLUDE #include "Config.dm" -#include "..\tgs_include.dm" +#include "../test_prelude.dm" #include "Test.dm" // END_INCLUDE diff --git a/tests/DMAPI/LongRunning/Test.dm b/tests/DMAPI/LongRunning/Test.dm index bdf85acc45..762add090c 100644 --- a/tests/DMAPI/LongRunning/Test.dm +++ b/tests/DMAPI/LongRunning/Test.dm @@ -2,32 +2,7 @@ sleep_offline = FALSE loop_checks = FALSE -/world/Error(exception/E, datum/e_src) - var/list/usrinfo = null - var/list/splitlines = splittext(E.desc, "\n") - var/list/desclines = list() - for(var/line in splitlines) - if(length(line) < 3 || findtext(line, "source file:") || findtext(line, "usr.loc:")) - continue - if(findtext(line, "usr:")) - if(usrinfo) - desclines.Add(usrinfo) - usrinfo = null - continue // Our usr info is better, replace it - - if(copytext(line, 1, 3) != " ")//3 == length(" ") + 1 - desclines += (" " + line) // Pad any unpadded lines, so they look pretty - else - desclines += line - - if(usrinfo) //If this info isn't null, it hasn't been added yet - desclines.Add(usrinfo) - - fdel("test_success.txt") - text2file("Runtime Error: [E]", "test_fail_reason.txt") - -/world/New() - text2file("SUCCESS", "test_success.txt") +/world/proc/RunTest() log << "Initial value of sleep_offline: [sleep_offline]" sleep_offline = FALSE @@ -38,8 +13,7 @@ var/list/channels = TgsChatChannelInfo() if(!length(channels)) - text2file("Expected some chat channels!", "test_fail_reason.txt") - del(world) + FailTest("Expected some chat channels!") StartAsync() @@ -124,8 +98,7 @@ var/run_bridge_test if(tactics4) var/size = isnum(tactics4) ? tactics4 : text2num(tactics4) if(!isnum(size)) - text2file("tgs_integration_test_tactics4 wasn't a number!", "test_fail_reason.txt") - del(world) + FailTest("tgs_integration_test_tactics4 wasn't a number!") var/payload = create_payload(size) return payload @@ -182,8 +155,7 @@ var/run_bridge_test // hack hack, calling world.TgsChatChannelInfo() will try to delay until the channels come back var/datum/tgs_api/v5/api = TGS_READ_GLOBAL(tgs) if(length(api.chat_channels)) - text2file("Expected no chat channels after detach!", "test_fail_reason.txt") - del(world) + FailTest("Expected no chat channels after detach!") world.TgsChatBroadcast(new /datum/tgs_message_content("Recieved event: `[json_encode(args)]`")) @@ -299,7 +271,6 @@ var/lastTgsError // this actually gets doubled because it's in two fields for backwards compatibility, but that's fine var/list/final_result = api.Bridge(0, list("chatMessage" = list("text" = "done:[create_payload(limit * 3)]"))) if(!final_result || lastTgsError || final_result["integrationHack"] != "ok") - text2file("Failed to end bridge limit test! [(istype(final_result) ? json_encode(final_result): (final_result || "null"))]", "test_fail_reason.txt") - del(world) + FailTest("Failed to end bridge limit test! [(istype(final_result) ? json_encode(final_result): (final_result || "null"))]") api.access_identifier = old_ai diff --git a/tests/DMAPI/LongRunning/long_running_test.dme b/tests/DMAPI/LongRunning/long_running_test.dme index 92781c7e16..b927014455 100644 --- a/tests/DMAPI/LongRunning/long_running_test.dme +++ b/tests/DMAPI/LongRunning/long_running_test.dme @@ -12,6 +12,6 @@ // BEGIN_INCLUDE #include "Config.dm" -#include "..\tgs_include.dm" +#include "../test_prelude.dm" #include "Test.dm" // END_INCLUDE diff --git a/tests/DMAPI/LongRunning/long_running_test_copy.dme b/tests/DMAPI/LongRunning/long_running_test_copy.dme index 92781c7e16..b4da8e0d96 100644 --- a/tests/DMAPI/LongRunning/long_running_test_copy.dme +++ b/tests/DMAPI/LongRunning/long_running_test_copy.dme @@ -11,7 +11,5 @@ // END_PREFERENCES // BEGIN_INCLUDE -#include "Config.dm" -#include "..\tgs_include.dm" -#include "Test.dm" +#include "long_running_test.dme" // END_INCLUDE diff --git a/tests/DMAPI/tgs_include.dm b/tests/DMAPI/test_prelude.dm similarity index 75% rename from tests/DMAPI/tgs_include.dm rename to tests/DMAPI/test_prelude.dm index d60b655b94..18b7192ec1 100644 --- a/tests/DMAPI/tgs_include.dm +++ b/tests/DMAPI/test_prelude.dm @@ -1,2 +1,3 @@ #include "..\..\src\DMAPI\tgs.dm" #include "..\..\src\DMAPI\tgs\includes.dm" +#include "test_setup.dm" diff --git a/tests/DMAPI/test_setup.dm b/tests/DMAPI/test_setup.dm new file mode 100644 index 0000000000..514921d668 --- /dev/null +++ b/tests/DMAPI/test_setup.dm @@ -0,0 +1,32 @@ +/world/New() + text2file("SUCCESS", "test_success.txt") + world.RunTest() + +/world/Error(exception/E, datum/e_src) + var/list/usrinfo = null + var/list/splitlines = splittext(E.desc, "\n") + var/list/desclines = list() + for(var/line in splitlines) + if(length(line) < 3 || findtext(line, "source file:") || findtext(line, "usr.loc:")) + continue + if(findtext(line, "usr:")) + if(usrinfo) + desclines.Add(usrinfo) + usrinfo = null + continue // Our usr info is better, replace it + + if(copytext(line, 1, 3) != " ")//3 == length(" ") + 1 + desclines += (" " + line) // Pad any unpadded lines, so they look pretty + else + desclines += line + + if(usrinfo) //If this info isn't null, it hasn't been added yet + desclines.Add(usrinfo) + + FailTest("Runtime Error: [E]") + +/proc/FailTest(reason) + world.log << "TEST ERROR DM-SIDE: [reason]" + fdel("test_success.txt") + text2file(reason, "test_fail_reason.txt") + del(world) diff --git a/tgstation-server.sln b/tgstation-server.sln index 2a7bda3f3d..fa1f1bf89f 100644 --- a/tgstation-server.sln +++ b/tgstation-server.sln @@ -93,7 +93,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{3BB10856-AA0 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{82066812-6C73-4360-943B-B23F2F491261}" ProjectSection(SolutionItems) = preProject - tests\DMAPI\tgs_include.dm = tests\DMAPI\tgs_include.dm + tests\DMAPI\test_prelude.dm = tests\DMAPI\test_prelude.dm + tests\DMAPI\test_setup.dm = tests\DMAPI\test_setup.dm EndProjectSection EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "v4", "v4", "{057FAC33-CC31-4948-91C6-B0977C335890}" @@ -193,7 +194,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Tgstation.Server.Migrator.C EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Tgstation.Server.Host.Common", "src\Tgstation.Server.Host.Common\Tgstation.Server.Host.Common.csproj", "{CF3968A0-EA81-4464-B2D4-C7D40F6B5BCB}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Tgstation.Server.Common", "src\Tgstation.Server.Common\Tgstation.Server.Common.csproj", "{70CD9A98-D31A-44A4-81D1-D02764CEEEFD}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Tgstation.Server.Common", "src\Tgstation.Server.Common\Tgstation.Server.Common.csproj", "{70CD9A98-D31A-44A4-81D1-D02764CEEEFD}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution From 1021c04d4b48ed885c1b723720e6901fee6e5628 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 19:11:25 -0400 Subject: [PATCH 21/41] Don't be cheeky with this .dme name It hinders full text searches --- tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 484be88225..37d63e884b 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -717,9 +717,8 @@ namespace Tgstation.Server.Tests.Live.Instance async Task RunLongRunningTestThenUpdateWithNewDme(CancellationToken cancellationToken) { System.Console.WriteLine("TEST: WATCHDOG LONG RUNNING WITH NEW DME TEST"); - const string DmeName = "LongRunning/long_running_test"; - var daemonStatus = await DeployTestDme(DmeName, DreamDaemonSecurity.Trusted, true, cancellationToken); + var daemonStatus = await DeployTestDme("LongRunning/long_running_test", DreamDaemonSecurity.Trusted, true, cancellationToken); var initialCompileJob = daemonStatus.ActiveCompileJob; Assert.AreEqual(WatchdogStatus.Offline, daemonStatus.Status.Value); @@ -732,7 +731,7 @@ namespace Tgstation.Server.Tests.Live.Instance await WaitForJob(startJob, 40, false, null, cancellationToken); - daemonStatus = await DeployTestDme(DmeName + "_copy", DreamDaemonSecurity.Safe, true, cancellationToken); + daemonStatus = await DeployTestDme("LongRunning/long_running_test_copy", DreamDaemonSecurity.Safe, true, cancellationToken); Assert.AreEqual(WatchdogStatus.Online, daemonStatus.Status.Value); From 1196df6a6b84a800d5c28e58ee5105d03eb8e2b6 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 19:15:27 -0400 Subject: [PATCH 22/41] Continue to log chat sends to no available channels --- src/Tgstation.Server.Host/Components/Chat/ChatManager.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 13196b3f57..5a2f37385f 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -984,15 +984,16 @@ namespace Tgstation.Server.Host.Components.Chat Task SendMessage(IEnumerable channelIds, Message replyTo, MessageContent message, CancellationToken cancellationToken) { channelIds = channelIds.ToList(); - if (!channelIds.Any()) - return Task.CompletedTask; logger.LogTrace( - "Chat send \"{message}\"{embed} to channels: {channelIdsCommaSeperated}", + "Chat send \"{message}\"{embed} to channels: [{channelIdsCommaSeperated}]", message.Text, message.Embed != null ? " (with embed)" : String.Empty, String.Join(", ", channelIds)); + if (!channelIds.Any()) + return Task.CompletedTask; + return Task.WhenAll( channelIds.Select(x => { From 9f1d169ff645d04a176622ad57d80db32a7f5584 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 19:38:00 -0400 Subject: [PATCH 23/41] Chat test connections are now required to succeed --- .../Live/Instance/ChatTest.cs | 29 +++++++++++++++++-- .../Live/Instance/InstanceTest.cs | 2 +- .../Live/TestLiveServer.cs | 2 +- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs index ab8553fc22..984c2e9f2c 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs @@ -13,13 +13,14 @@ using Tgstation.Server.Client.Components; namespace Tgstation.Server.Tests.Live.Instance { - sealed class ChatTest + sealed class ChatTest : JobsRequiredTest { readonly IChatBotsClient chatClient; readonly IInstanceManagerClient instanceClient; readonly Api.Models.Instance metadata; - public ChatTest(IChatBotsClient chatClient, IInstanceManagerClient instanceClient, Api.Models.Instance metadata) + public ChatTest(IChatBotsClient chatClient, IInstanceManagerClient instanceClient, IJobsClient jobsClient, Api.Models.Instance metadata) + : base(jobsClient) { this.chatClient = chatClient ?? throw new ArgumentNullException(nameof(chatClient)); this.instanceClient = instanceClient ?? throw new ArgumentNullException(nameof(instanceClient)); @@ -65,6 +66,8 @@ namespace Tgstation.Server.Tests.Live.Instance var retrievedBot = await chatClient.GetId(firstBot, cancellationToken); Assert.AreEqual(firstBot.Id, retrievedBot.Id); + var beforeChatBotEnabled = DateTimeOffset.UtcNow; + var updatedBot = await chatClient.Update(new ChatBotUpdateRequest { Id = firstBot.Id, @@ -73,7 +76,16 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(true, updatedBot.Enabled); - var channelId = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CHANNEL"); ; + var jobs = await JobsClient.List(null, cancellationToken); + var reconnectJob = jobs + .Where(x => x.StartedAt >= beforeChatBotEnabled && x.Description.Contains(updatedBot.Name)) + .OrderByDescending(x => x.StartedAt) + .FirstOrDefault(); + + Assert.IsNotNull(reconnectJob); + await WaitForJob(reconnectJob, 60, false, null, cancellationToken); + + var channelId = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CHANNEL"); updatedBot = await chatClient.Update(new ChatBotUpdateRequest { @@ -140,6 +152,8 @@ namespace Tgstation.Server.Tests.Live.Instance var retrievedBot = await chatClient.GetId(firstBot, cancellationToken); Assert.AreEqual(firstBot.Id, retrievedBot.Id); + var beforeChatBotEnabled = DateTimeOffset.UtcNow; + var updatedBot = await chatClient.Update(new ChatBotUpdateRequest { Id = firstBot.Id, @@ -148,6 +162,15 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(true, updatedBot.Enabled); + var jobs = await JobsClient.List(null, cancellationToken); + var reconnectJob = jobs + .Where(x => x.StartedAt >= beforeChatBotEnabled && x.Description.Contains(updatedBot.Name)) + .OrderByDescending(x => x.StartedAt) + .FirstOrDefault(); + + Assert.IsNotNull(reconnectJob); + await WaitForJob(reconnectJob, 60, false, null, cancellationToken); + var channelId = ulong.Parse(Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_CHANNEL")); updatedBot = await chatClient.Update(new ChatBotUpdateRequest diff --git a/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs index dae2bee1d9..fd502926a9 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs @@ -26,7 +26,7 @@ namespace Tgstation.Server.Tests.Live.Instance public async Task RunTests(CancellationToken cancellationToken) { var byondTest = new ByondTest(instanceClient.Byond, instanceClient.Jobs, instanceClient.Metadata); - var chatTest = new ChatTest(instanceClient.ChatBots, instanceManagerClient, instanceClient.Metadata); + var chatTest = new ChatTest(instanceClient.ChatBots, instanceManagerClient, instanceClient.Jobs, instanceClient.Metadata); var configTest = new ConfigurationTest(instanceClient.Configuration, instanceClient.Metadata); var repoTest = new RepositoryTest(instanceClient.Repository, instanceClient.Jobs); var dmTest = new DeploymentTest(instanceClient, instanceClient.Jobs); diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 1734afef0f..1f38d6cb21 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -969,7 +969,7 @@ namespace Tgstation.Server.Tests.Live Assert.IsNull(currentDD.StagedCompileJob); var repoTest = new RepositoryTest(instanceClient.Repository, instanceClient.Jobs).RunPostTest(cancellationToken); - await new ChatTest(instanceClient.ChatBots, adminClient.Instances, instance).RunPostTest(cancellationToken); + await new ChatTest(instanceClient.ChatBots, adminClient.Instances, instanceClient.Jobs, instance).RunPostTest(cancellationToken); await repoTest; await new InstanceManagerTest(adminClient, server.Directory).RunPostTest(cancellationToken); From 50873dc61697b43a5fdb10d2417f151074142f86 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 19:53:12 -0400 Subject: [PATCH 24/41] Connect/Disconnect test for IRC --- .../Chat/Providers/TestIrcProvider.cs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs index 2efc860a6a..89867bb98b 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs @@ -1,8 +1,14 @@ +using System; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using System.Xml.Linq; + using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using System; -using System.Threading.Tasks; +using Serilog.Parsing; + using Tgstation.Server.Api.Models; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; @@ -45,5 +51,41 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests await new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, mockBot).DisposeAsync(); } + + static Task InvokeConnect(IProvider provider, CancellationToken cancellationToken = default) => (Task)provider.GetType().GetMethod("Connect", BindingFlags.Instance | BindingFlags.NonPublic).Invoke(provider, new object[] { cancellationToken }); + + [TestMethod] + public async Task TestConnectAndDisconnect() + { + var actualToken = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CONNECTION_STRING"); + if (actualToken == null) + Assert.Inconclusive("Required environment variable TGS_TEST_IRC_CONNECTION_STRING isn't set!"); + + using var loggerFactory = LoggerFactory.Create(builder => + { + builder.AddConsole(); + builder.SetMinimumLevel(LogLevel.Trace); + }); + var mockSetup = new Mock(); + mockSetup + .Setup(x => x.RegisterOperation(It.IsNotNull(), It.IsNotNull(), It.IsAny())) + .Callback((job, entrypoint, cancellationToken) => job.StartedBy ??= new User { }) + .Returns(Task.CompletedTask); + mockSetup + .Setup(x => x.WaitForJobCompletion(It.IsNotNull(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + var mockJobManager = mockSetup.Object; + await using var provider = new IrcProvider(mockJobManager, Mock.Of(), new AsyncDelayer(), loggerFactory.CreateLogger(), new ChatBot + { + ConnectionString = actualToken, + Provider = ChatProvider.Irc, + }); + Assert.IsFalse(provider.Connected); + await InvokeConnect(provider); + Assert.IsTrue(provider.Connected); + + await provider.Disconnect(default); + Assert.IsFalse(provider.Connected); + } } } From c9403b79f3e229fb62166d3bcf091bccab01d579 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 19:53:38 -0400 Subject: [PATCH 25/41] DiscordProvider Constructor test doesn't need a valid token --- .../Components/Chat/Providers/TestDiscordProvider.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs index 696d1cc2fd..e346a6e78d 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs @@ -44,8 +44,11 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests [TestMethod] public async Task TestConstructionAndDisposal() { - if (testToken1 == null) - Assert.Inconclusive("Required environment variable TGS_TEST_DISCORD_TOKEN isn't set!"); + var bot = new ChatBot + { + ConnectionString = "fake_token", + ReconnectionInterval = 1, + }; Assert.ThrowsException(() => new DiscordProvider(null, null, null, null)); Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, null)); @@ -53,7 +56,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockAss.Object, null, null)); var mockLogger = new Mock>(); Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, mockLogger.Object, null)); - await new DiscordProvider(mockJobManager, mockAss.Object, mockLogger.Object, testToken1).DisposeAsync(); + await new DiscordProvider(mockJobManager, mockAss.Object, mockLogger.Object, bot).DisposeAsync(); } static Task InvokeConnect(IProvider provider, CancellationToken cancellationToken = default) => (Task)provider.GetType().GetMethod("Connect", BindingFlags.Instance | BindingFlags.NonPublic).Invoke(provider, new object[] { cancellationToken }); From 87c62621bffedb13b7a8ede57975a4b674194878 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 23:45:56 -0400 Subject: [PATCH 26/41] Chat fixes - Fix a slow Task leak when providers are disposed. - Fix non-thread-safe access for chat update message callbacks. --- .../Components/Chat/ChatManager.cs | 16 ++++++++++------ .../Components/Chat/Providers/Provider.cs | 5 ++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 5a2f37385f..400d55e9b7 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -397,10 +397,10 @@ namespace Tgstation.Server.Host.Components.Chat gitHubRepo, channelMapping.ProviderChannelId, localCommitPushed, - handlerCts.Token) - ; + handlerCts.Token); - callbacks.Add(callback); + lock (callbacks) + callbacks.Add(callback); } catch (Exception ex) { @@ -932,6 +932,12 @@ namespace Tgstation.Server.Host.Components.Chat // process completed ones foreach (var completedMessageTaskKvp in messageTasks.Where(x => x.Value.IsCompleted).ToList()) { + var provider = completedMessageTaskKvp.Key; + messageTasks.Remove(provider); + + if (provider.Disposed) // valid to receive one, but don't process it + continue; + var message = await completedMessageTaskKvp.Value; var messageNumber = Interlocked.Increment(ref messagesProcessed); @@ -941,7 +947,7 @@ namespace Tgstation.Server.Host.Components.Chat using (LogContext.PushProperty(SerilogContextHelper.ChatMessageIterationContextProperty, messageNumber)) try { - await ProcessMessage(completedMessageTaskKvp.Key, message, false, cancellationToken); + await ProcessMessage(provider, message, false, cancellationToken); } catch (Exception ex) { @@ -952,8 +958,6 @@ namespace Tgstation.Server.Host.Components.Chat } activeProcessingTask = WrapProcessMessage(); - - messageTasks.Remove(completedMessageTaskKvp.Key); } } } diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs index 1e2bf22ff0..010c21e106 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs @@ -98,6 +98,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers { Disposed = true; await StopReconnectionTimer(); + + // queue a final message to shutdown the NextMessage Task + EnqueueMessage(null); Logger.LogTrace("Disposed"); } @@ -206,7 +209,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// /// Queues a for . /// - /// The to queue. A value of indicates the channel mappings a out of date. + /// The to queue. A value of indicates the channel mappings are out of date. protected void EnqueueMessage(Message message) { if (message == null) From 9961051fdf7a2f86771baeeebdd7983833eb7dc4 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 23:50:11 -0400 Subject: [PATCH 27/41] Fix update message callbacks potentially not being registered before activation --- .../Components/Chat/ChatManager.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 400d55e9b7..8124434888 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -413,12 +413,17 @@ namespace Tgstation.Server.Host.Components.Chat AddMessageTask(task); - return (errorMessage, dreamMakerOutput) => AddMessageTask( - Task.WhenAll( + async Task CollateTasks(string errorMessage, string dreamMakerOutput) + { + await task; + await Task.WhenAll( callbacks.Select( x => x( errorMessage, - dreamMakerOutput)))); + dreamMakerOutput))); + } + + return (errorMessage, dreamMakerOutput) => AddMessageTask(CollateTasks(errorMessage, dreamMakerOutput)); } /// From 70c635ff8c15aa58ca7a3cebb87c534507af5661 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 22 May 2023 23:52:18 -0400 Subject: [PATCH 28/41] Add missing ArgumentNullExceptions --- .../Chat/Providers/DiscordProvider.cs | 15 ++- .../Components/Chat/Providers/IrcProvider.cs | 99 +++++++++++-------- 2 files changed, 70 insertions(+), 44 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index 1640d28516..b12b12a1ee 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -232,6 +232,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// public override async Task SendMessage(Message replyTo, MessageContent message, ulong channelId, CancellationToken cancellationToken) { + if (message == null) + throw new ArgumentNullException(nameof(message)); + Optional replyToReference = default; Optional allowedMentions = default; if (replyTo != null && replyTo is DiscordMessage discordMessage) @@ -329,6 +332,15 @@ namespace Tgstation.Server.Host.Components.Chat.Providers bool localCommitPushed, CancellationToken cancellationToken) { + if (revisionInformation == null) + throw new ArgumentNullException(nameof(revisionInformation)); + if (byondVersion == null) + throw new ArgumentNullException(nameof(byondVersion)); + if (gitHubOwner == null) + throw new ArgumentNullException(nameof(gitHubOwner)); + if (gitHubRepo == null) + throw new ArgumentNullException(nameof(gitHubRepo)); + localCommitPushed |= revisionInformation.CommitSha == revisionInformation.OriginCommitSha; var fields = BuildUpdateEmbedFields(revisionInformation, byondVersion, gitHubOwner, gitHubRepo, localCommitPushed); @@ -414,8 +426,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers new Snowflake(channelId), updatedMessage, embeds: new List { embed }, - ct: cancellationToken) - ; + ct: cancellationToken); if (!createUpdatedMessageResponse.IsSuccess) Logger.LogWarning( diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs index b0563406ea..6a871755b9 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs @@ -172,53 +172,59 @@ namespace Tgstation.Server.Host.Components.Chat.Providers } /// - public override Task SendMessage(Message replyTo, MessageContent message, ulong channelId, CancellationToken cancellationToken) => Task.Factory.StartNew( - () => - { - // IRC doesn't allow newlines - // Explicitly ignore embeds - var messageText = message.Text; - messageText ??= $"Embed Only: {JsonConvert.SerializeObject(message.Embed)}"; + public override Task SendMessage(Message replyTo, MessageContent message, ulong channelId, CancellationToken cancellationToken) + { + if (message == null) + throw new ArgumentNullException(nameof(message)); - messageText = String.Concat( - messageText - .Where(x => x != '\r') - .Select(x => x == '\n' ? '|' : x)); - - var channelName = channelIdMap[channelId]; - SendType sendType; - if (channelName == null) + return Task.Factory.StartNew( + () => { - channelName = queryChannelIdMap[channelId]; - sendType = SendType.Notice; - } - else - sendType = SendType.Message; + // IRC doesn't allow newlines + // Explicitly ignore embeds + var messageText = message.Text; + messageText ??= $"Embed Only: {JsonConvert.SerializeObject(message.Embed)}"; - var messageSize = Encoding.UTF8.GetByteCount(messageText) + Encoding.UTF8.GetByteCount(channelName) + PreambleMessageLength; - var messageTooLong = messageSize > MessageBytesLimit; - if (messageTooLong) - messageText = $"TGS: Could not send message to IRC. Line write exceeded protocol limit of {MessageBytesLimit}B."; + messageText = String.Concat( + messageText + .Where(x => x != '\r') + .Select(x => x == '\n' ? '|' : x)); - try - { - client.SendMessage(sendType, channelName, messageText); - } - catch (Exception e) - { - Logger.LogWarning(e, "Unable to send to channel {channelName}!", channelName); - return; - } + var channelName = channelIdMap[channelId]; + SendType sendType; + if (channelName == null) + { + channelName = queryChannelIdMap[channelId]; + sendType = SendType.Notice; + } + else + sendType = SendType.Message; - if (messageTooLong) - Logger.LogWarning( - "Failed to send to channel {channelId}: Message size ({messageSize}B) exceeds IRC limit of 512B", - channelId, - messageSize); - }, - cancellationToken, - DefaultIOManager.BlockingTaskCreationOptions, - TaskScheduler.Current); + var messageSize = Encoding.UTF8.GetByteCount(messageText) + Encoding.UTF8.GetByteCount(channelName) + PreambleMessageLength; + var messageTooLong = messageSize > MessageBytesLimit; + if (messageTooLong) + messageText = $"TGS: Could not send message to IRC. Line write exceeded protocol limit of {MessageBytesLimit}B."; + + try + { + client.SendMessage(sendType, channelName, messageText); + } + catch (Exception e) + { + Logger.LogWarning(e, "Unable to send to channel {channelName}!", channelName); + return; + } + + if (messageTooLong) + Logger.LogWarning( + "Failed to send to channel {channelId}: Message size ({messageSize}B) exceeds IRC limit of 512B", + channelId, + messageSize); + }, + cancellationToken, + DefaultIOManager.BlockingTaskCreationOptions, + TaskScheduler.Current); + } /// public override async Task> SendUpdateMessage( @@ -231,6 +237,15 @@ namespace Tgstation.Server.Host.Components.Chat.Providers bool localCommitPushed, CancellationToken cancellationToken) { + if (revisionInformation == null) + throw new ArgumentNullException(nameof(revisionInformation)); + if (byondVersion == null) + throw new ArgumentNullException(nameof(byondVersion)); + if (gitHubOwner == null) + throw new ArgumentNullException(nameof(gitHubOwner)); + if (gitHubRepo == null) + throw new ArgumentNullException(nameof(gitHubRepo)); + var commitInsert = revisionInformation.CommitSha[..7]; string remoteCommitInsert; if (revisionInformation.CommitSha == revisionInformation.OriginCommitSha) From f289c8298302f17ba0288788cce13bfb59d9b215 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 01:27:10 -0400 Subject: [PATCH 29/41] Replace a bunch of Task.Delays with IAsyncDelayer --- .../Chat/Providers/DiscordProvider.cs | 9 ++++--- .../Components/Chat/Providers/IrcProvider.cs | 24 +++++++------------ .../Components/Chat/Providers/Provider.cs | 12 ++++++++-- .../Chat/Providers/ProviderFactory.cs | 5 ++-- .../Components/Deployment/DreamMaker.cs | 13 ++++++++-- .../Components/Instance.cs | 10 +++++++- .../Components/InstanceFactory.cs | 12 ++++++++++ .../Components/Session/SessionController.cs | 7 +++++- .../Session/SessionControllerFactory.cs | 13 +++++++++- 9 files changed, 77 insertions(+), 28 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index b12b12a1ee..9e614fe941 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -27,6 +27,7 @@ using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.System; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Chat.Providers { @@ -179,15 +180,17 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// Initializes a new instance of the class. /// /// The for the . - /// The value of . + /// The for the . /// The for the . + /// The value of . /// The for the . public DiscordProvider( IJobManager jobManager, - IAssemblyInformationProvider assemblyInformationProvider, + IAsyncDelayer asyncDelayer, ILogger logger, + IAssemblyInformationProvider assemblyInformationProvider, ChatBot chatBot) - : base(jobManager, logger, chatBot) + : base(jobManager, asyncDelayer, logger, chatBot) { this.assemblyInformationProvider = assemblyInformationProvider ?? throw new ArgumentNullException(nameof(assemblyInformationProvider)); diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs index 6a871755b9..179105d248 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/IrcProvider.cs @@ -42,11 +42,6 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// public override string BotMention => client.Nickname; - /// - /// The for the . - /// - readonly IAsyncDelayer asyncDelayer; - /// /// The client. /// @@ -105,24 +100,22 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// /// Initializes a new instance of the class. /// - /// The for the provider. - /// The to get the from. - /// The value of . + /// The for the . + /// The for the . /// The for the . + /// The to get the from. /// The for the . public IrcProvider( IJobManager jobManager, - IAssemblyInformationProvider assemblyInformationProvider, IAsyncDelayer asyncDelayer, ILogger logger, + IAssemblyInformationProvider assemblyInformationProvider, Models.ChatBot chatBot) - : base(jobManager, logger, chatBot) + : base(jobManager, asyncDelayer, logger, chatBot) { if (assemblyInformationProvider == null) throw new ArgumentNullException(nameof(assemblyInformationProvider)); - this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); - var builder = chatBot.CreateConnectionStringBuilder(); if (builder == null || !builder.Valid || builder is not IrcConnectionStringBuilder ircBuilder) throw new InvalidOperationException("Invalid ChatConnectionStringBuilder!"); @@ -641,14 +634,14 @@ namespace Tgstation.Server.Host.Components.Chat.Providers var listenTimeSpan = TimeSpan.FromMilliseconds(10); for (; !recievedAck; - await asyncDelayer.Delay(listenTimeSpan, timeoutToken)) + await AsyncDelayer.Delay(listenTimeSpan, timeoutToken)) await NonBlockingListen(cancellationToken); client.WriteLine("AUTHENTICATE PLAIN", Priority.Critical); timeoutToken.ThrowIfCancellationRequested(); for (; !recievedPlus; - await asyncDelayer.Delay(listenTimeSpan, timeoutToken)) + await AsyncDelayer.Delay(listenTimeSpan, timeoutToken)) await NonBlockingListen(cancellationToken); } finally @@ -714,8 +707,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers Task.WhenAll( disconnectTask, listenTask ?? Task.CompletedTask), - asyncDelayer.Delay(TimeSpan.FromSeconds(5), cancellationToken)) - ; + AsyncDelayer.Delay(TimeSpan.FromSeconds(5), cancellationToken)); } } } diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs index 010c21e106..d613cc8872 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs @@ -10,6 +10,7 @@ using Tgstation.Server.Host.Components.Interop; using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Chat.Providers { @@ -24,6 +25,11 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// protected ChatBot ChatBot { get; } + /// + /// The for the . + /// + protected IAsyncDelayer AsyncDelayer { get; } + /// /// The for the . /// @@ -68,11 +74,13 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// Initializes a new instance of the class. /// /// The value of . + /// The value of . /// The value of . /// The value of . - protected Provider(IJobManager jobManager, ILogger logger, ChatBot chatBot) + protected Provider(IJobManager jobManager, IAsyncDelayer asyncDelayer, ILogger logger, ChatBot chatBot) { this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); + AsyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); Logger = logger ?? throw new ArgumentNullException(nameof(logger)); ChatBot = chatBot ?? throw new ArgumentNullException(nameof(chatBot)); @@ -259,7 +267,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers try { if (!connectNow) - await Task.Delay(TimeSpan.FromMinutes(reconnectInterval), cancellationToken); + await AsyncDelayer.Delay(TimeSpan.FromMinutes(reconnectInterval), cancellationToken); else connectNow = false; if (!Connected) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs index 6a7e0abd32..775db6c2e5 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/ProviderFactory.cs @@ -61,14 +61,15 @@ namespace Tgstation.Server.Host.Components.Chat.Providers { ChatProvider.Irc => new IrcProvider( jobManager, - assemblyInformationProvider, asyncDelayer, loggerFactory.CreateLogger(), + assemblyInformationProvider, settings), ChatProvider.Discord => new DiscordProvider( jobManager, - assemblyInformationProvider, + asyncDelayer, loggerFactory.CreateLogger(), + assemblyInformationProvider, settings), _ => throw new InvalidOperationException(String.Format(CultureInfo.InvariantCulture, "Invalid ChatProvider: {0}", settings.Provider)), }; diff --git a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs index ab04afebb9..4d1a37fe96 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DreamMaker.cs @@ -23,6 +23,7 @@ using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.System; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Deployment { @@ -89,6 +90,11 @@ namespace Tgstation.Server.Host.Components.Deployment /// readonly IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory; + /// + /// The for . + /// + readonly IAsyncDelayer asyncDelayer; + /// /// The for . /// @@ -147,6 +153,7 @@ namespace Tgstation.Server.Host.Components.Deployment /// The value of . /// The value of . /// The value of . + /// The value of . /// The value of . /// The value of . /// The value of . @@ -161,6 +168,7 @@ namespace Tgstation.Server.Host.Components.Deployment ICompileJobSink compileJobConsumer, IRepositoryManager repositoryManager, IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory, + IAsyncDelayer asyncDelayer, ILogger logger, SessionConfiguration sessionConfiguration, Api.Models.Instance metadata) @@ -174,6 +182,7 @@ namespace Tgstation.Server.Host.Components.Deployment this.processExecutor = processExecutor ?? throw new ArgumentNullException(nameof(processExecutor)); this.compileJobConsumer = compileJobConsumer ?? throw new ArgumentNullException(nameof(compileJobConsumer)); this.repositoryManager = repositoryManager ?? throw new ArgumentNullException(nameof(repositoryManager)); + this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); this.remoteDeploymentManagerFactory = remoteDeploymentManagerFactory ?? throw new ArgumentNullException(nameof(remoteDeploymentManagerFactory)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.sessionConfiguration = sessionConfiguration ?? throw new ArgumentNullException(nameof(sessionConfiguration)); @@ -741,13 +750,13 @@ namespace Tgstation.Server.Host.Components.Deployment var remainingSleepThisInterval = nextInterval - DateTimeOffset.UtcNow; var nextSleepSpan = remainingSleepThisInterval < minimumSleepInterval ? minimumSleepInterval : remainingSleepThisInterval; - await Task.Delay(nextSleepSpan, cancellationToken); + await asyncDelayer.Delay(nextSleepSpan, cancellationToken); progressReporter.ReportProgress(lastReport); } while (DateTimeOffset.UtcNow < nextInterval); } else - await Task.Delay(minimumSleepInterval, cancellationToken); + await asyncDelayer.Delay(minimumSleepInterval, cancellationToken); lastReport = estimatedDuration.HasValue ? sleepInterval * (iteration + 1) / estimatedDuration.Value : null; progressReporter.ReportProgress(lastReport); diff --git a/src/Tgstation.Server.Host/Components/Instance.cs b/src/Tgstation.Server.Host/Components/Instance.cs index 8d36d468b3..5c5ec4d692 100644 --- a/src/Tgstation.Server.Host/Components/Instance.cs +++ b/src/Tgstation.Server.Host/Components/Instance.cs @@ -70,6 +70,11 @@ namespace Tgstation.Server.Host.Components /// readonly IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory; + /// + /// The for the . + /// + readonly IAsyncDelayer asyncDelayer; + /// /// The for the . /// @@ -109,6 +114,7 @@ namespace Tgstation.Server.Host.Components /// The value of . /// The value of . /// The value of . + /// The value of . /// The value of . public Instance( Api.Models.Instance metadata, @@ -123,6 +129,7 @@ namespace Tgstation.Server.Host.Components IJobManager jobManager, IEventConsumer eventConsumer, IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory, + IAsyncDelayer asyncDelayer, ILogger logger) { this.metadata = metadata ?? throw new ArgumentNullException(nameof(metadata)); @@ -136,6 +143,7 @@ namespace Tgstation.Server.Host.Components this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer)); this.remoteDeploymentManagerFactory = remoteDeploymentManagerFactory ?? throw new ArgumentNullException(nameof(remoteDeploymentManagerFactory)); + this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); timerLock = new object(); @@ -488,7 +496,7 @@ namespace Tgstation.Server.Host.Components while (true) try { - await Task.Delay(TimeSpan.FromMinutes(minutes > Int32.MaxValue ? Int32.MaxValue : minutes), cancellationToken); + await asyncDelayer.Delay(TimeSpan.FromMinutes(minutes > Int32.MaxValue ? Int32.MaxValue : minutes), cancellationToken); logger.LogInformation("Beginning auto update..."); await eventConsumer.HandleEvent(EventType.InstanceAutoUpdateStart, Enumerable.Empty(), cancellationToken); try diff --git a/src/Tgstation.Server.Host/Components/InstanceFactory.cs b/src/Tgstation.Server.Host/Components/InstanceFactory.cs index 6f68acec65..6d78a4edd0 100644 --- a/src/Tgstation.Server.Host/Components/InstanceFactory.cs +++ b/src/Tgstation.Server.Host/Components/InstanceFactory.cs @@ -23,6 +23,7 @@ using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Security; using Tgstation.Server.Host.System; using Tgstation.Server.Host.Transfer; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components { @@ -139,6 +140,11 @@ namespace Tgstation.Server.Host.Components /// readonly IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory; + /// + /// The for the . + /// + readonly IAsyncDelayer asyncDelayer; + /// /// The for the . /// @@ -182,6 +188,7 @@ namespace Tgstation.Server.Host.Components /// The value of . /// The value of . /// The value of . + /// The value of . /// The containing the value of . /// The containing the value of . public InstanceFactory( @@ -207,6 +214,7 @@ namespace Tgstation.Server.Host.Components IFileTransferTicketProvider fileTransferService, IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory, + IAsyncDelayer asyncDelayer, IOptions generalConfigurationOptions, IOptions sessionConfigurationOptions) { @@ -232,6 +240,7 @@ namespace Tgstation.Server.Host.Components this.fileTransferService = fileTransferService ?? throw new ArgumentNullException(nameof(fileTransferService)); this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); this.remoteDeploymentManagerFactory = remoteDeploymentManagerFactory ?? throw new ArgumentNullException(nameof(remoteDeploymentManagerFactory)); + this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); sessionConfiguration = sessionConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(sessionConfigurationOptions)); } @@ -310,6 +319,7 @@ namespace Tgstation.Server.Host.Components bridgeRegistrar, serverPortProvider, eventConsumer, + asyncDelayer, loggerFactory, loggerFactory.CreateLogger(), sessionConfiguration, @@ -358,6 +368,7 @@ namespace Tgstation.Server.Host.Components dmbFactory, repoManager, remoteDeploymentManagerFactory, + asyncDelayer, loggerFactory.CreateLogger(), sessionConfiguration, metadata); @@ -374,6 +385,7 @@ namespace Tgstation.Server.Host.Components jobManager, eventConsumer, remoteDeploymentManagerFactory, + asyncDelayer, loggerFactory.CreateLogger()); return instance; diff --git a/src/Tgstation.Server.Host/Components/Session/SessionController.cs b/src/Tgstation.Server.Host/Components/Session/SessionController.cs index f5e1491594..a831a37679 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionController.cs @@ -180,6 +180,7 @@ namespace Tgstation.Server.Host.Components.Session /// The value of . /// The value of . /// The for the . + /// The for the . /// The value of . /// The returning a to be run after the ends. /// The optional time to wait before failing the . @@ -195,6 +196,7 @@ namespace Tgstation.Server.Host.Components.Session IBridgeRegistrar bridgeRegistrar, IChatManager chat, IAssemblyInformationProvider assemblyInformationProvider, + IAsyncDelayer asyncDelayer, ILogger logger, Func postLifetimeCallback, uint? startupTimeout, @@ -250,6 +252,7 @@ namespace Tgstation.Server.Host.Components.Session LaunchResult = GetLaunchResult( assemblyInformationProvider, + asyncDelayer, startupTimeout, reattached, apiValidate); @@ -512,12 +515,14 @@ namespace Tgstation.Server.Host.Components.Session /// The for . /// /// The . + /// The . /// The, optional, startup timeout in seconds. /// If DreamDaemon was reattached. /// If this is a DMAPI validation session. /// A resulting in the for the operation. async Task GetLaunchResult( IAssemblyInformationProvider assemblyInformationProvider, + IAsyncDelayer asyncDelayer, uint? startupTimeout, bool reattached, bool apiValidate) @@ -530,7 +535,7 @@ namespace Tgstation.Server.Host.Components.Session var toAwait = Task.WhenAny(startupTask, process.Lifetime); if (startupTimeout.HasValue) - toAwait = Task.WhenAny(toAwait, Task.Delay(TimeSpan.FromSeconds(startupTimeout.Value))); + toAwait = Task.WhenAny(toAwait, asyncDelayer.Delay(TimeSpan.FromSeconds(startupTimeout.Value), default)); // DCT: None available, task will clean up after delay Logger.LogTrace( "Waiting for LaunchResult based on {launchResultCompletionCause}{possibleTimeout}...", diff --git a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs index f785629871..0662b381d2 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs @@ -26,6 +26,7 @@ using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Security; using Tgstation.Server.Host.System; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Session { @@ -102,6 +103,11 @@ namespace Tgstation.Server.Host.Components.Session /// readonly IEventConsumer eventConsumer; + /// + /// The for the . + /// + readonly IAsyncDelayer asyncDelayer; + /// /// The for the . /// @@ -187,10 +193,11 @@ namespace Tgstation.Server.Host.Components.Session /// The value of . /// The value of . /// The value of . + /// The value of . + /// The value of . /// The value of . /// The value of . /// The value of . - /// The value of . public SessionControllerFactory( IProcessExecutor processExecutor, IByondManager byond, @@ -205,6 +212,7 @@ namespace Tgstation.Server.Host.Components.Session IBridgeRegistrar bridgeRegistrar, IServerPortProvider serverPortProvider, IEventConsumer eventConsumer, + IAsyncDelayer asyncDelayer, ILoggerFactory loggerFactory, ILogger logger, SessionConfiguration sessionConfiguration, @@ -223,6 +231,7 @@ namespace Tgstation.Server.Host.Components.Session this.bridgeRegistrar = bridgeRegistrar ?? throw new ArgumentNullException(nameof(bridgeRegistrar)); this.serverPortProvider = serverPortProvider ?? throw new ArgumentNullException(nameof(serverPortProvider)); this.eventConsumer = eventConsumer ?? throw new ArgumentNullException(nameof(eventConsumer)); + this.asyncDelayer = asyncDelayer ?? throw new ArgumentNullException(nameof(asyncDelayer)); this.loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.sessionConfiguration = sessionConfiguration ?? throw new ArgumentNullException(nameof(sessionConfiguration)); @@ -341,6 +350,7 @@ namespace Tgstation.Server.Host.Components.Session bridgeRegistrar, chat, assemblyInformationProvider, + asyncDelayer, loggerFactory.CreateLogger(), () => !launchParameters.LogOutput.Value ? LogDDOutput(process, outputFilePath, byondLock.SupportsCli, default) // DCT: None available @@ -425,6 +435,7 @@ namespace Tgstation.Server.Host.Components.Session bridgeRegistrar, chat, assemblyInformationProvider, + asyncDelayer, loggerFactory.CreateLogger(), () => Task.CompletedTask, null, From 487b8c81e09d3d03b1065d1e9245435966613a00 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:43:32 -0400 Subject: [PATCH 30/41] Prevent restart handler errors from propagating to the controller --- src/Tgstation.Server.Host/Server.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Tgstation.Server.Host/Server.cs b/src/Tgstation.Server.Host/Server.cs index f4bf53ad66..de7a97cf65 100644 --- a/src/Tgstation.Server.Host/Server.cs +++ b/src/Tgstation.Server.Host/Server.cs @@ -314,14 +314,14 @@ namespace Tgstation.Server.Host ? generalConfiguration.ShutdownTimeoutMinutes : generalConfiguration.RestartTimeoutMinutes)); var cancellationToken = cts.Token; - var eventsTask = Task.WhenAll( - restartHandlers.Select( - x => x.HandleRestart(newVersion, isGracefulShutdown, cancellationToken)) - .ToList()); - - logger.LogTrace("Joining restart handlers..."); try { + var eventsTask = Task.WhenAll( + restartHandlers.Select( + x => x.HandleRestart(newVersion, isGracefulShutdown, cancellationToken)) + .ToList()); + + logger.LogTrace("Joining restart handlers..."); await eventsTask; } catch (OperationCanceledException ex) From fb6196b847b8a9b36c180430d975dfa1991f73d4 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:46:26 -0400 Subject: [PATCH 31/41] Fix more annoying-ass semicolons --- .../Components/Chat/Providers/DiscordProvider.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index 9e614fe941..b15ccc02ef 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -371,8 +371,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers new Snowflake(channelId), "DM: Deployment in Progress...", embeds: new List { embed }, - ct: cancellationToken) - ; + ct: cancellationToken); if (!messageResponse.IsSuccess) Logger.LogWarning("Failed to post deploy embed to channel {channelId}: {result}", channelId, messageResponse.LogFormat()); @@ -446,8 +445,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers messageResponse.Entity.ID, updatedMessage, embeds: new List { embed }, - ct: cancellationToken) - ; + ct: cancellationToken); if (!editResponse.IsSuccess) { From 78c802e1c7784d3eede225aff4d3f79f31927f10 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:54:31 -0400 Subject: [PATCH 32/41] Doc comment improvement --- .../Components/Chat/ChannelRepresentation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChannelRepresentation.cs b/src/Tgstation.Server.Host/Components/Chat/ChannelRepresentation.cs index 239e68fa88..a24f416ec1 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChannelRepresentation.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChannelRepresentation.cs @@ -19,7 +19,7 @@ namespace Tgstation.Server.Host.Components.Chat /// /// The channel Id. /// - /// remaps this to an internal id using . Not sent over the DMAPI. + /// remaps this to an internal id using . Not sent over the DMAPI. [JsonIgnore] public ulong RealId { From 60fbb6f9b4528319e30bce88a81d1307fc57b38f Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:54:45 -0400 Subject: [PATCH 33/41] Add an informative comment --- src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs index d613cc8872..e13f46ade8 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs @@ -301,6 +301,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers } catch { + // we set this here because otherwise there could be stuff waiting on to connect us forever initialConnectionTcs.TrySetResult(); throw; } From e6003a7c3c90f7f24921ff6bc59c9837fee3cde8 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:55:10 -0400 Subject: [PATCH 34/41] Properly handle provider disconnect exceptions --- .../Components/Chat/ChatManager.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index 8124434888..c17c703ed6 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -523,13 +523,15 @@ namespace Tgstation.Server.Host.Components.Chat { await provider.Disconnect(cancellationToken); } - finally + catch (Exception ex) { - await provider.DisposeAsync(); - var duration = DateTimeOffset.UtcNow - startTime; - if (duration.TotalSeconds > 3) - logger.LogWarning("Disconnecting a {providerType} took {totalSeconds}s!", provider.GetType().Name, duration.TotalSeconds); + logger.LogError(ex, "Error disconnecting connection {connectionId}!", connectionId); } + + await provider.DisposeAsync(); + var duration = DateTimeOffset.UtcNow - startTime; + if (duration.TotalSeconds > 3) + logger.LogWarning("Disconnecting a {providerType} took {totalSeconds}s!", provider.GetType().Name, duration.TotalSeconds); } else logger.LogTrace("DeleteConnection: ID {connectionId} doesn't exist!", connectionId); From 76b214d40e1ac7d57231ff4403a6631eac83aa51 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:56:08 -0400 Subject: [PATCH 35/41] Mock `IChatProvider`s when testing without secrets --- .github/CONTRIBUTING.md | 11 +- .github/workflows/ci-suite.yml | 18 +- src/Tgstation.Server.Host/Core/Application.cs | 3 +- .../Extensions/ServiceCollectionExtensions.cs | 25 ++ .../Chat/Providers/TestDiscordProvider.cs | 21 +- .../Chat/Providers/TestIrcProvider.cs | 14 +- .../Live/DummyChatProvider.cs | 303 ++++++++++++++++++ .../Live/DummyChatProviderFactory.cs | 84 +++++ .../Live/Instance/WatchdogTest.cs | 3 + .../Live/TestLiveServer.cs | 19 ++ 10 files changed, 466 insertions(+), 35 deletions(-) create mode 100644 tests/Tgstation.Server.Tests/Live/DummyChatProvider.cs create mode 100644 tests/Tgstation.Server.Tests/Live/DummyChatProviderFactory.cs diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 79ee3f7ac3..934067d85f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -38,15 +38,16 @@ You need the Dotnet 6.0 SDK and npm>=v5.7 (in your PATH) to compile the server. The recommended IDE is Visual Studio 2019 which has installation options for both of these. -In order to run the integration tests you must have the following environment variables set: +In order to run the integration tests you must have the following environment variables set. To run them more accurately, include the optional ones. - `TGS_TEST_DATABASE_TYPE`: `MySql`, `MariaDB`, `PostgresSql`, or `SqlServer`. - `TGS_TEST_CONNECTION_STRING`: To a valid database connection string. You can use the setup wizard to create one. -- `TSG_TEST_DISCORD_TOKEN`: To a valid discord bot token. -- `TGS_TEST_DISCORD_CHANNEL`: To a valid discord channel ID that the above bot can access. -- `TGS_TEST_IRC_CONNECTION_STRING`: To a valid IRC connection string. See the code for [IrcConnectionStringBuilder](../src/Tgstation.Server.Api/Models/IrcConnectionStringBuilder.cs) for details. -- `TGS_TEST_IRC_CHANNEL`: To a valid IRC channel accessible with the above connection. - `TGS_TEST_BRANCH`: Should be either `dev` or `master` depending on what you are working off of. Used for repository tests. - (Optional) `TGS_TEST_GITHUB_TOKEN`: A GitHub personal access token with no scopes used to bypass rate limits. +- (Optional) The following variables are all interdependent, so if one is set they all must be. + - `TSG_TEST_DISCORD_TOKEN`: To a valid discord bot token. + - `TGS_TEST_DISCORD_CHANNEL`: To a valid discord channel ID that the above bot can access. + - `TGS_TEST_IRC_CONNECTION_STRING`: To a valid IRC connection string. See the code for [IrcConnectionStringBuilder](../src/Tgstation.Server.Api/Models/IrcConnectionStringBuilder.cs) for details. + - `TGS_TEST_IRC_CHANNEL`: To a valid IRC channel accessible with the above connection. ### Know your Code diff --git a/.github/workflows/ci-suite.yml b/.github/workflows/ci-suite.yml index cd9e7b5efa..5b934f08da 100644 --- a/.github/workflows/ci-suite.yml +++ b/.github/workflows/ci-suite.yml @@ -17,8 +17,6 @@ on: env: TGS_DOTNET_VERSION: 6.0.x - TGS_TEST_DISCORD_CHANNEL: ${{ secrets.DISCORD_CHANNEL_ID }} - TGS_TEST_IRC_CHANNEL: ${{ secrets.IRC_CHANNEL }} TGS_TEST_GITHUB_TOKEN: ${{ secrets.LIVE_TESTS_TOKEN }} TGS_RELEASE_NOTES_TOKEN: ${{ secrets.DEV_PUSH_TOKEN }} @@ -229,6 +227,9 @@ jobs: fail-fast: false matrix: configuration: [ 'Debug', 'Release' ] + env: + TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} + TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} runs-on: ubuntu-latest steps: - name: Setup dotnet @@ -269,6 +270,9 @@ jobs: fail-fast: false matrix: configuration: [ 'Debug', 'Release' ] + env: + TGS_TEST_DISCORD_TOKEN: ${{ secrets.DISCORD_TOKEN }} + TGS_TEST_IRC_CONNECTION_STRING: ${{ secrets.IRC_CONNECTION_STRING }} runs-on: windows-latest steps: - name: Setup dotnet @@ -334,8 +338,6 @@ jobs: TGS_CONNSTRING_VALUE="Server=(localdb)\MSSQLLocalDB;Integrated Security=true;Initial Catalog=TGS_${{ matrix.watchdog-type }}_${{ matrix.configuration }};Application Name=tgstation-server" echo "TGS_TEST_CONNECTION_STRING=$(echo $TGS_CONNSTRING_VALUE)" >> $GITHUB_ENV echo "TGS_TEST_DATABASE_TYPE=SqlServer" >> $GITHUB_ENV - echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_WINDOWS_SQLSERVER }}" >> $GITHUB_ENV - echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_WINDOWS_SQLSERVER }}" >> $GITHUB_ENV - name: Checkout (Branch Push) uses: actions/checkout@v3 @@ -458,24 +460,18 @@ jobs: run: | echo "TGS_TEST_DATABASE_TYPE=Sqlite" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Data Source=TGS_${{ matrix.watchdog-type }}_${{ matrix.configuration }}.sqlite3;Mode=ReadWriteCreate" >> $GITHUB_ENV - echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_SQLITE }}" >> $GITHUB_ENV - echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_SQLITE }}" >> $GITHUB_ENV - name: Set PostgresSql Connection Info if: ${{ matrix.database-type == 'PostgresSql' }} run: | echo "TGS_TEST_DATABASE_TYPE=PostgresSql" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Application Name=tgstation-server;Host=127.0.0.1;Username=postgres;Password=postgres;Database=TGS__${{ matrix.watchdog-type }}_${{ matrix.configuration }}" >> $GITHUB_ENV - echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_POSTGRES }}" >> $GITHUB_ENV - echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_POSTGRES }}" >> $GITHUB_ENV - name: Set MariaDB Connection Info if: ${{ matrix.database-type == 'MariaDB' }} run: | echo "TGS_TEST_DATABASE_TYPE=MariaDB" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Server=127.0.0.1;uid=root;pwd=mariadb;database=tgs__${{ matrix.watchdog-type }}_${{ matrix.configuration }}" >> $GITHUB_ENV - echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_MARIADB }}" >> $GITHUB_ENV - echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_MARIADB }}" >> $GITHUB_ENV - name: Set MySQL Connection Info if: ${{ matrix.database-type == 'MySql' }} @@ -483,8 +479,6 @@ jobs: echo "TGS_TEST_DATABASE_TYPE=MySql" >> $GITHUB_ENV echo "TGS_TEST_CONNECTION_STRING=Server=127.0.0.1;Port=3307;uid=root;pwd=mysql;database=tgs__${{ matrix.watchdog-type }}_${{ matrix.configuration }}" >> $GITHUB_ENV echo "Database__ServerVersion=5.7.31" >> $GITHUB_ENV - echo "TGS_TEST_DISCORD_TOKEN=${{ secrets.DISCORD_TOKEN_LINUX_MYSQL }}" >> $GITHUB_ENV - echo "TGS_TEST_IRC_CONNECTION_STRING=${{ secrets.IRC_CONNECTION_STRING_LINUX_MYSQL }}" >> $GITHUB_ENV - name: Set General__UseBasicWatchdog if: ${{ matrix.watchdog-type == 'Basic' }} diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs index c2b4547e7d..fa5897a9e5 100644 --- a/src/Tgstation.Server.Host/Core/Application.cs +++ b/src/Tgstation.Server.Host/Core/Application.cs @@ -30,7 +30,6 @@ using Tgstation.Server.Common; using Tgstation.Server.Host.Components; using Tgstation.Server.Host.Components.Byond; using Tgstation.Server.Host.Components.Chat; -using Tgstation.Server.Host.Components.Chat.Providers; using Tgstation.Server.Host.Components.Deployment.Remote; using Tgstation.Server.Host.Components.Interop; using Tgstation.Server.Host.Components.Interop.Bridge; @@ -360,7 +359,7 @@ namespace Tgstation.Server.Host.Core services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(); + services.AddChatProviderFactory(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/Tgstation.Server.Host/Extensions/ServiceCollectionExtensions.cs b/src/Tgstation.Server.Host/Extensions/ServiceCollectionExtensions.cs index 46bb712d39..4fcbc5d32e 100644 --- a/src/Tgstation.Server.Host/Extensions/ServiceCollectionExtensions.cs +++ b/src/Tgstation.Server.Host/Extensions/ServiceCollectionExtensions.cs @@ -11,6 +11,7 @@ using Serilog; using Serilog.Configuration; using Serilog.Sinks.Elasticsearch; +using Tgstation.Server.Host.Components.Chat.Providers; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Utils; @@ -21,6 +22,30 @@ namespace Tgstation.Server.Host.Extensions /// static class ServiceCollectionExtensions { + /// + /// The implementation used in calls to . + /// + static Type chatProviderFactoryType = typeof(ProviderFactory); + + /// + /// Change the used as an implementation for calls to . + /// + /// The implementation to use. + public static void UseChatProviderFactory() where TProviderFactory : IProviderFactory + { + chatProviderFactoryType = typeof(TProviderFactory); + } + + /// + /// Adds a implementation to the given . + /// + /// The to configure. + /// . + public static IServiceCollection AddChatProviderFactory(this IServiceCollection serviceCollection) + { + return serviceCollection.AddSingleton(typeof(IProviderFactory), chatProviderFactoryType); + } + /// /// Add a standard binding. /// diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs index e346a6e78d..f4457c285e 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs @@ -10,6 +10,7 @@ using Moq; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.System; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Chat.Providers.Tests { @@ -50,13 +51,15 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests ReconnectionInterval = 1, }; - Assert.ThrowsException(() => new DiscordProvider(null, null, null, null)); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, null)); - var mockAss = new Mock(); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockAss.Object, null, null)); - var mockLogger = new Mock>(); - Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, mockLogger.Object, null)); - await new DiscordProvider(mockJobManager, mockAss.Object, mockLogger.Object, bot).DisposeAsync(); + Assert.ThrowsException(() => new DiscordProvider(null, null, null, null, null)); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, null, null, null, null)); + var mockDel = Mock.Of(); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockDel, null, null, null)); + var mockLogger = Mock.Of>(); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockDel, mockLogger, null, null)); + var mockAss = Mock.Of(); + Assert.ThrowsException(() => new DiscordProvider(mockJobManager, mockDel, mockLogger, mockAss, null)); + await new DiscordProvider(mockJobManager, mockDel, mockLogger, mockAss, bot).DisposeAsync(); } static Task InvokeConnect(IProvider provider, CancellationToken cancellationToken = default) => (Task)provider.GetType().GetMethod("Connect", BindingFlags.Instance | BindingFlags.NonPublic).Invoke(provider, new object[] { cancellationToken }); @@ -65,7 +68,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests public async Task TestConnectWithFakeTokenFails() { var mockLogger = new Mock>(); - await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, new ChatBot + await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, Mock.Of(), new ChatBot { ReconnectionInterval = 1, ConnectionString = "asdf" @@ -81,7 +84,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests Assert.Inconclusive("Required environment variable TGS_TEST_DISCORD_TOKEN isn't set!"); var mockLogger = new Mock>(); - await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, testToken1); + await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, Mock.Of(), testToken1); Assert.IsFalse(provider.Connected); await InvokeConnect(provider); Assert.IsTrue(provider.Connected); diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs index 89867bb98b..2e8921a6a2 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs @@ -26,12 +26,12 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests Assert.ThrowsException(() => new IrcProvider(null, null, null, null, null)); var mockJobManager = new Mock(); Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, null, null, null, null)); - var mockAss = new Mock(); - Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, null, null, null)); var mockAsyncDelayer = new Mock(); - Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, null, null)); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAsyncDelayer.Object, null, null, null)); var mockLogger = new Mock>(); - Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, null)); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAsyncDelayer.Object, mockLogger.Object, null, null)); + var mockAss = new Mock(); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAsyncDelayer.Object, mockLogger.Object, mockAss.Object, null)); var mockBot = new ChatBot { @@ -39,7 +39,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests Provider = ChatProvider.Irc }; - Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, mockBot)); + Assert.ThrowsException(() => new IrcProvider(mockJobManager.Object, mockAsyncDelayer.Object, mockLogger.Object, mockAss.Object, mockBot)); mockBot.ConnectionString = new IrcConnectionStringBuilder { @@ -49,7 +49,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests Port = 6667 }.ToString(); - await new IrcProvider(mockJobManager.Object, mockAss.Object, mockAsyncDelayer.Object, mockLogger.Object, mockBot).DisposeAsync(); + await new IrcProvider(mockJobManager.Object, mockAsyncDelayer.Object, mockLogger.Object, mockAss.Object, mockBot).DisposeAsync(); } static Task InvokeConnect(IProvider provider, CancellationToken cancellationToken = default) => (Task)provider.GetType().GetMethod("Connect", BindingFlags.Instance | BindingFlags.NonPublic).Invoke(provider, new object[] { cancellationToken }); @@ -75,7 +75,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests .Setup(x => x.WaitForJobCompletion(It.IsNotNull(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); var mockJobManager = mockSetup.Object; - await using var provider = new IrcProvider(mockJobManager, Mock.Of(), new AsyncDelayer(), loggerFactory.CreateLogger(), new ChatBot + await using var provider = new IrcProvider(mockJobManager, new AsyncDelayer(), loggerFactory.CreateLogger(), Mock.Of(), new ChatBot { ConnectionString = actualToken, Provider = ChatProvider.Irc, diff --git a/tests/Tgstation.Server.Tests/Live/DummyChatProvider.cs b/tests/Tgstation.Server.Tests/Live/DummyChatProvider.cs new file mode 100644 index 0000000000..331931cef6 --- /dev/null +++ b/tests/Tgstation.Server.Tests/Live/DummyChatProvider.cs @@ -0,0 +1,303 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +using Microsoft.Extensions.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Moq; + +using Tgstation.Server.Host.Components.Chat; +using Tgstation.Server.Host.Components.Chat.Commands; +using Tgstation.Server.Host.Components.Chat.Providers; +using Tgstation.Server.Host.Components.Interop; +using Tgstation.Server.Host.Jobs; +using Tgstation.Server.Host.Models; +using Tgstation.Server.Host.Security; +using Tgstation.Server.Host.Utils; + +namespace Tgstation.Server.Tests.Live +{ + sealed class DummyChatProvider : Provider + { + public override bool Connected => connected; + + public override string BotMention => $"Dummy{ChatBot.Provider}-I-{ChatBot.InstanceId}-N-{ChatBot.Name}"; + + static int enableRandomDisconnections = 1; + + readonly Random random; // this RNG isn't perfect as calls into this class can theoretically happen in a random order due to async + + readonly IReadOnlyCollection commands; + readonly ICryptographySuite cryptographySuite; + readonly CancellationTokenSource randomMessageCts; + readonly Task randomMessageTask; + + bool connectedOnce; + bool connected; + + ulong channelIdAllocator; + + static ILoggerFactory CreateLoggerFactoryForLogger(ILogger logger, out Mock mockLoggerFactory) + { + mockLoggerFactory = new Mock(); + mockLoggerFactory.Setup(x => x.CreateLogger(It.IsAny())).Returns(() => + { + var temp = logger; + logger = null; + + Assert.IsNotNull(temp); + return temp; + }) + .Verifiable(); + return mockLoggerFactory.Object; + } + + static IAsyncDelayer CreateMockDelayer() + { + // at time of writing, this is used exclusively for the reconnection interval which works in minutes + // shorten it to 3s + var mock = new Mock(); + mock.Setup(x => x.Delay(It.IsAny(), It.IsAny())).Returns((delay, cancellationToken) => Task.Delay(TimeSpan.FromSeconds(3), cancellationToken)); + return mock.Object; + } + public static async Task RandomDisconnections(bool enabled, CancellationToken cancellationToken) + { + if (Interlocked.Exchange(ref enableRandomDisconnections, enabled ? 1 : 0) != 0 && !enabled) + await Task.Delay(TimeSpan.FromSeconds(5), cancellationToken); + } + + public DummyChatProvider( + IJobManager jobManager, + ILogger logger, + ChatBot chatBot, + ICryptographySuite cryptographySuite, + IReadOnlyCollection commands, + Random random) + : base(jobManager, CreateMockDelayer(), new Logger(CreateLoggerFactoryForLogger(logger, out var mockLoggerFactory)), chatBot) + { + mockLoggerFactory.VerifyAll(); + this.cryptographySuite = cryptographySuite ?? throw new ArgumentNullException(nameof(cryptographySuite)); + this.commands = commands ?? throw new ArgumentNullException(nameof(commands)); + this.random = random ?? throw new ArgumentNullException(nameof(random)); + + // this could be random but there's no point + channelIdAllocator = 100000; + logger.LogTrace("Base channel ID {baseChannelId}", channelIdAllocator); + + this.randomMessageCts = new CancellationTokenSource(); + this.randomMessageTask = RandomMessageLoop(this.randomMessageCts.Token); + } + + public override async ValueTask DisposeAsync() + { + Logger.LogTrace("DisposeAsync Child"); + this.randomMessageCts.Cancel(); + this.randomMessageCts.Dispose(); + await this.randomMessageTask; + await base.DisposeAsync(); + } + + public override Task SendMessage(Message replyTo, MessageContent message, ulong channelId, CancellationToken cancellationToken) + { + if (message == null) + throw new ArgumentNullException(nameof(message)); + + Logger.LogTrace("SendMessage"); + + Assert.AreNotEqual(0UL, channelId); + Assert.IsTrue(channelId <= channelIdAllocator); + + cancellationToken.ThrowIfCancellationRequested(); + + /* SendMessage is no-throw + if (random.Next(0, 100) > 70) + throw new Exception("Random SendMessage failure!"); */ + + return Task.CompletedTask; + } + + public override Task> SendUpdateMessage(RevisionInformation revisionInformation, Version byondVersion, DateTimeOffset? estimatedCompletionTime, string gitHubOwner, string gitHubRepo, ulong channelId, bool localCommitPushed, CancellationToken cancellationToken) + { + if (revisionInformation == null) + throw new ArgumentNullException(nameof(revisionInformation)); + if (byondVersion == null) + throw new ArgumentNullException(nameof(byondVersion)); + if (gitHubOwner == null) + throw new ArgumentNullException(nameof(gitHubOwner)); + if (gitHubRepo == null) + throw new ArgumentNullException(nameof(gitHubRepo)); + + Logger.LogTrace("SendUpdateMessage"); + + Assert.AreNotEqual(0UL, channelId); + Assert.IsTrue(channelId <= channelIdAllocator); + + cancellationToken.ThrowIfCancellationRequested(); + + /* SendUpdateMessage is no-throw + if (random.Next(0, 100) > 70) + throw new Exception("Random SendUpdateMessage failure!"); */ + + return Task.FromResult>((_, _) => + { + cancellationToken.ThrowIfCancellationRequested(); + + /* SendUpdateMessage callbacks are no-throw + if (random.Next(0, 100) > 70) + throw new Exception("Random SendUpdateMessage failure!"); */ + + return Task.CompletedTask; + }); + } + + protected override Task Connect(CancellationToken cancellationToken) + { + Logger.LogTrace("Connect"); + cancellationToken.ThrowIfCancellationRequested(); + + // 30% chance to fail AFTER initial connection + if (connectedOnce && enableRandomDisconnections != 0 && random.Next(0, 100) > 70) + throw new Exception("Random connection failure!"); + + connected = true; + connectedOnce = true; + return Task.CompletedTask; + } + + protected override Task DisconnectImpl(CancellationToken cancellationToken) + { + Logger.LogTrace("DisconnectImpl"); + cancellationToken.ThrowIfCancellationRequested(); + connected = false; + + if (random.Next(0, 100) > 70) + throw new Exception("Random disconnection failure!"); + return Task.CompletedTask; + } + + protected override Task>> MapChannelsImpl(IEnumerable channels, CancellationToken cancellationToken) + { + channels = channels.ToList(); + Logger.LogTrace("MapChannels: [{channels}]", String.Join(", ", channels.Select(channel => channel.IrcChannel ?? channel.DiscordChannelId?.ToString() ?? throw new InvalidOperationException("BAD CHANNEL")))); + + cancellationToken.ThrowIfCancellationRequested(); + + /* MapChannelsImpl is no-throw + if (random.Next(0, 100) > 70) + throw new Exception("Random MapChannelsImpl failure!"); */ + + return Task.FromResult( + new Dictionary>( + channels.Select( + channel => new KeyValuePair>( + channel, + new List + { + new ChannelRepresentation + { + IsAdminChannel = channel.IsAdminChannel.Value, + ConnectionName = $"Connection_{channelIdAllocator + 1}", + EmbedsSupported = ChatBot.Provider.Value != Api.Models.ChatProvider.Irc, + FriendlyName = $"(Friendly) Channel_ID_{channelIdAllocator + 1}", + IsPrivateChannel = false, + RealId = ++channelIdAllocator, + Tag = channel.Tag, + } + })))); + } + + async Task RandomMessageLoop(CancellationToken cancellationToken) + { + Logger.LogTrace("RandomMessageLoop"); + try + { + for (var i = 0UL; !cancellationToken.IsCancellationRequested; ++i) + { + // random intervals under 10s + var delay = random.Next(0, 10000); + await Task.Delay(delay, cancellationToken); + + if (!connected) + continue; + + // %5 chance to disconnect randomly + if (enableRandomDisconnections != 0 && random.Next(0, 100) > 95) + connected = false; + + if (channelIdAllocator >= Int32.MaxValue / 2) + Assert.Fail("Too many channels have been allocated!"); + + var isPm = channelIdAllocator == 0 || random.Next(0, 100) > 20; + var realId = (ulong)random.Next(1, (int)channelIdAllocator); + + if (isPm) + realId += Int32.MaxValue / 2; + + var username = $"RandomUser{i}"; + var sender = new ChatUser + { + Channel = new ChannelRepresentation + { + RealId = realId, + IsPrivateChannel = isPm, + ConnectionName = isPm ? $"{username}_Connection" : $"Connection_{realId}", + FriendlyName = isPm ? $"{username}_Channel" : $"(Friendly) Channel_ID_{realId}", + EmbedsSupported = ChatBot.Provider.Value != Api.Models.ChatProvider.Irc, + + // isAdmin and Tag populated by manager + }, + FriendlyName = username, + RealId = i + 50000, + Mention = $"@{username}", + }; + + var dice = random.Next(0, 100); + string content; + // 70% chance to be random chat + if (dice < 70) + content = cryptographySuite.GetSecureString(); + // 15% chance to be a !tgs + else if (dice < 85) + content = "!tgs"; + // 15% chance to be a strict mention + else + content = BotMention; + + // 30% chance to request help + if (random.Next(0, 100) > 70) + content = $"{content} help"; + + dice = random.Next(0, 100); + + // 20% chance to whiff + if (dice > 20) + // 40% chance to attempt a built-in TGS command + if (dice < 68) + // equal chance for each + content = $"{content} {commands.ElementAt(random.Next(0, commands.Count)).Name}"; + // 40% chance to attempt a custom chat command in long_running_test + else + // equal chance for each + if (random.Next(0, 100) > 50) + content = $"{content} embeds_test"; + else + content = $"{content} response_overload_test"; + + EnqueueMessage(new Message + { + Content = content, + User = sender, + }); + } + + } + catch (OperationCanceledException) + { + Logger.LogTrace("RandomMessageLoop cancelled"); + } + } + } +} diff --git a/tests/Tgstation.Server.Tests/Live/DummyChatProviderFactory.cs b/tests/Tgstation.Server.Tests/Live/DummyChatProviderFactory.cs new file mode 100644 index 0000000000..55162d85c2 --- /dev/null +++ b/tests/Tgstation.Server.Tests/Live/DummyChatProviderFactory.cs @@ -0,0 +1,84 @@ +using System; +using System.Collections.Generic; + +using Microsoft.Extensions.Logging; + +using Moq; + +using Tgstation.Server.Api.Models; +using Tgstation.Server.Host.Components.Byond; +using Tgstation.Server.Host.Components.Chat.Commands; +using Tgstation.Server.Host.Components.Chat.Providers; +using Tgstation.Server.Host.Components.Repository; +using Tgstation.Server.Host.Components.Watchdog; +using Tgstation.Server.Host.Database; +using Tgstation.Server.Host.Jobs; +using Tgstation.Server.Host.Models; +using Tgstation.Server.Host.Security; +using Tgstation.Server.Host.System; + +namespace Tgstation.Server.Tests.Live +{ + sealed class DummyChatProviderFactory : IProviderFactory + { + readonly IJobManager jobManager; + readonly ICryptographySuite cryptographySuite; + readonly ILoggerFactory loggerFactory; + readonly ILogger logger; + + readonly IReadOnlyList commands; + + readonly Dictionary seededRng; + + public DummyChatProviderFactory(IJobManager jobManager, ICryptographySuite cryptographySuite, ILoggerFactory loggerFactory, ILogger logger) + { + this.jobManager = jobManager ?? throw new ArgumentNullException(nameof(jobManager)); + this.cryptographySuite = cryptographySuite ?? throw new ArgumentNullException(nameof(cryptographySuite)); + this.loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); + this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + + var commandFactory = new CommandFactory( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + new Host.Models.Instance()); + + commandFactory.SetWatchdog(Mock.Of()); + commands = commandFactory.GenerateCommands(); + + var baseRng = new Random(22475); + seededRng = new Dictionary{ + { ChatProvider.Irc, new Random(baseRng.Next()) }, + { ChatProvider.Discord, new Random(baseRng.Next()) }, + }; // hope you get the reference + } + + public IProvider CreateProvider(ChatBot settings) + { + logger.LogTrace("CreateProvider"); + if (settings == null) + throw new ArgumentNullException(nameof(settings)); + + var provider = settings.Provider; + switch (provider) + { + case ChatProvider.Irc: + case ChatProvider.Discord: + logger.LogTrace("Creating DummyChatProvider in place of requested {providerType}Provider", settings.Provider); + + // for RNG to work, chat bots need to get created in a certain order + // the ChatTest creates one of each provider type + return new DummyChatProvider( + jobManager, + loggerFactory.CreateLogger($"Dummy{settings.Provider}Provider"), + settings, + cryptographySuite, + commands, + new Random(seededRng[provider.Value].Next())); + default: + throw new InvalidOperationException($"Invalid ChatProvider: {provider}"); + } + } + } +} diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 37d63e884b..78b287a641 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -79,6 +79,9 @@ namespace Tgstation.Server.Tests.Live.Instance await TestDMApiFreeDeploy(cancellationToken); + // long running test likes consistency with the channels + await DummyChatProvider.RandomDisconnections(false, cancellationToken); + await RunLongRunningTestThenUpdate(cancellationToken); await RunLongRunningTestThenUpdateWithNewDme(cancellationToken); diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 1f38d6cb21..01c50b5903 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -82,6 +82,7 @@ namespace Tgstation.Server.Tests.Live [TestInitialize] public async Task Initialize() { + await DummyChatProvider.RandomDisconnections(true, default); ServerClientFactory.ApiClientFactory = new RateLimitRetryingApiClientFactory(); var connectionString = Environment.GetEnvironmentVariable("TGS_TEST_CONNECTION_STRING"); @@ -697,6 +698,24 @@ namespace Tgstation.Server.Tests.Live [TestMethod] public async Task TestStandardTgsOperation() { + var missingChatVarsCount = Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_TOKEN"))) + + Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_CHANNEL"))) + + Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_IRC_CONNECTION_STRING"))) + + Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_IRC_CHANNEL"))); + + const int TotalChatVars = 4; + + // uncomment to force this test to run with DummyChatProviders + missingChatVarsCount = TotalChatVars; + + if (missingChatVarsCount != 0) + { + if (missingChatVarsCount != TotalChatVars) + Assert.Fail("All TGS_TEST_* chat environment variables must be present or none at all!"); + + ServiceCollectionExtensions.UseChatProviderFactory(); + } + var procs = System.Diagnostics.Process.GetProcessesByName("byond"); if (procs.Any()) { From e0cd13917ff918bf81080a8db8bb55f0de579f68 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 02:59:15 -0400 Subject: [PATCH 36/41] Promotes a warning to an error --- src/Tgstation.Server.Host/Components/Chat/ChatManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index c17c703ed6..aa65578aa0 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -1045,7 +1045,7 @@ namespace Tgstation.Server.Host.Components.Chat } catch (Exception ex) { - logger.LogWarning(ex, "Error in asynchronous chat message!"); + logger.LogError(ex, "Error in asynchronous chat message!"); } } From 6ac18508dab9af5d2ef99afe9f55a68f832ec9ad Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 07:59:02 -0400 Subject: [PATCH 37/41] Assert valid connection strings in chat tests --- .../Chat/Providers/TestDiscordProvider.cs | 4 ++ .../Chat/Providers/TestIrcProvider.cs | 3 ++ .../Live/Instance/ChatTest.cs | 41 +++++++++++++++---- .../Live/TestLiveServer.cs | 14 +++++-- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs index f4457c285e..cdc30c888e 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestDiscordProvider.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using Tgstation.Server.Api.Models; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.System; @@ -83,6 +84,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests if (testToken1 == null) Assert.Inconclusive("Required environment variable TGS_TEST_DISCORD_TOKEN isn't set!"); + if (!new DiscordConnectionStringBuilder(testToken1.ConnectionString).Valid) + Assert.Fail("TGS_TEST_DISCORD_TOKEN is not a valid Discord connection string!"); + var mockLogger = new Mock>(); await using var provider = new DiscordProvider(mockJobManager, Mock.Of(), mockLogger.Object, Mock.Of(), testToken1); Assert.IsFalse(provider.Connected); diff --git a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs index 2e8921a6a2..b11b0ea074 100644 --- a/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs +++ b/tests/Tgstation.Server.Host.Tests/Components/Chat/Providers/TestIrcProvider.cs @@ -61,6 +61,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers.Tests if (actualToken == null) Assert.Inconclusive("Required environment variable TGS_TEST_IRC_CONNECTION_STRING isn't set!"); + if (!new IrcConnectionStringBuilder(actualToken).Valid) + Assert.Fail("TGS_TEST_IRC_CONNECTION_STRING is not a valid IRC connection string!"); + using var loggerFactory = LoggerFactory.Create(builder => { builder.AddConsole(); diff --git a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs index 984c2e9f2c..c7e9944f7b 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs @@ -41,9 +41,22 @@ namespace Tgstation.Server.Tests.Live.Instance async Task RunIrc(CancellationToken cancellationToken) { + var connectionString = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CONNECTION_STRING"); + if (String.IsNullOrWhiteSpace(connectionString)) + // needs to just be valid + connectionString = new IrcConnectionStringBuilder + { + Address = "irc.fake.com", + Nickname = "irc_nick", + Password = "some_pw", + PasswordType = IrcPasswordType.Server, + Port = 6668, + UseSsl = true, + }.ToString(); + var firstBotReq = new ChatBotCreateRequest { - ConnectionString = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CONNECTION_STRING"), + ConnectionString = connectionString, Enabled = false, Name = "tgs_integration_test", Provider = ChatProvider.Irc, @@ -86,6 +99,8 @@ namespace Tgstation.Server.Tests.Live.Instance await WaitForJob(reconnectJob, 60, false, null, cancellationToken); var channelId = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CHANNEL"); + if (String.IsNullOrWhiteSpace(channelId)) + channelId = "#botbus"; updatedBot = await chatClient.Update(new ChatBotUpdateRequest { @@ -122,14 +137,20 @@ namespace Tgstation.Server.Tests.Live.Instance async Task RunDiscord(CancellationToken cancellationToken) { + var connectionString = Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_TOKEN"); + if (String.IsNullOrWhiteSpace(connectionString)) + // needs to just be valid + connectionString = new DiscordConnectionStringBuilder + { + BasedMeme = true, + BotToken = "some_token", + DeploymentBranding = true, + DMOutputDisplay = DiscordDMOutputDisplayType.Never, + }.ToString(); + var firstBotReq = new ChatBotCreateRequest { - ConnectionString = - new DiscordConnectionStringBuilder - { - BotToken = Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_TOKEN"), - DMOutputDisplay = DiscordDMOutputDisplayType.OnError - }.ToString(), + ConnectionString = connectionString, Enabled = false, Name = "r4407", Provider = ChatProvider.Discord, @@ -171,7 +192,11 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.IsNotNull(reconnectJob); await WaitForJob(reconnectJob, 60, false, null, cancellationToken); - var channelId = ulong.Parse(Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_CHANNEL")); + var channelIdStr = Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_CHANNEL"); + if (String.IsNullOrWhiteSpace(channelIdStr)) + channelIdStr = "487268744419344384"; + + var channelId = ulong.Parse(channelIdStr); updatedBot = await chatClient.Update(new ChatBotUpdateRequest { diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 01c50b5903..e39d36650e 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -698,15 +698,17 @@ namespace Tgstation.Server.Tests.Live [TestMethod] public async Task TestStandardTgsOperation() { - var missingChatVarsCount = Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_TOKEN"))) + var discordConnectionString = Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_TOKEN"); + var ircConnectionString = Environment.GetEnvironmentVariable("TGS_TEST_IRC_CONNECTION_STRING"); + var missingChatVarsCount = Convert.ToInt32(String.IsNullOrWhiteSpace(discordConnectionString)) + Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_DISCORD_CHANNEL"))) - + Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_IRC_CONNECTION_STRING"))) + + Convert.ToInt32(String.IsNullOrWhiteSpace(ircConnectionString)) + Convert.ToInt32(String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("TGS_TEST_IRC_CHANNEL"))); const int TotalChatVars = 4; // uncomment to force this test to run with DummyChatProviders - missingChatVarsCount = TotalChatVars; + // missingChatVarsCount = TotalChatVars; if (missingChatVarsCount != 0) { @@ -715,6 +717,12 @@ namespace Tgstation.Server.Tests.Live ServiceCollectionExtensions.UseChatProviderFactory(); } + else + { + // prevalidate + Assert.IsTrue(new DiscordConnectionStringBuilder(discordConnectionString).Valid); + Assert.IsTrue(new IrcConnectionStringBuilder(ircConnectionString).Valid); + } var procs = System.Diagnostics.Process.GetProcessesByName("byond"); if (procs.Any()) From 4cd7d57bd191ed145bf7c147ec4e7fe4b8cd9b8e Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 17:55:17 -0400 Subject: [PATCH 38/41] Fixes #1498 --- src/Tgstation.Server.Host/Components/Chat/ChatManager.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index aa65578aa0..5ae211107d 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -1043,6 +1043,10 @@ namespace Tgstation.Server.Host.Components.Chat { await task; } + catch (OperationCanceledException ex) + { + logger.LogDebug(ex, "Async chat message cancelled!"); + } catch (Exception ex) { logger.LogError(ex, "Error in asynchronous chat message!"); From 69b8d94ccdfaf4aa6d8095b5708478d74c760709 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 19:41:06 -0400 Subject: [PATCH 39/41] Fix a semi-colon --- src/Tgstation.Server.Host/Controllers/ChatController.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Controllers/ChatController.cs b/src/Tgstation.Server.Host/Controllers/ChatController.cs index d6a33f6fe8..22605192ad 100644 --- a/src/Tgstation.Server.Host/Controllers/ChatController.cs +++ b/src/Tgstation.Server.Host/Controllers/ChatController.cs @@ -115,8 +115,7 @@ namespace Tgstation.Server.Host.Controllers .ChatBots .AsQueryable() .Where(x => x.InstanceId == Instance.Id) - .CountAsync(cancellationToken) - ; + .CountAsync(cancellationToken); if (countOfExistingBotsInInstance >= Instance.ChatBotLimit.Value) return Conflict(new ErrorMessageResponse(ErrorCode.ChatBotMax)); From fc3ce4caf75445ea5ffefa4f7b9d625a61470b6a Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 19:44:09 -0400 Subject: [PATCH 40/41] Fix errors when using real chat connection strings --- tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs index c7e9944f7b..6bad61b0b2 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs @@ -53,6 +53,9 @@ namespace Tgstation.Server.Tests.Live.Instance Port = 6668, UseSsl = true, }.ToString(); + else + // standardize + connectionString = new IrcConnectionStringBuilder(connectionString).ToString(); var firstBotReq = new ChatBotCreateRequest { @@ -147,6 +150,9 @@ namespace Tgstation.Server.Tests.Live.Instance DeploymentBranding = true, DMOutputDisplay = DiscordDMOutputDisplayType.Never, }.ToString(); + else + // standardize + connectionString = new DiscordConnectionStringBuilder(connectionString).ToString(); var firstBotReq = new ChatBotCreateRequest { From 86f20a0a3cfe772b948328c61d1e51a46d611627 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 23 May 2023 19:51:24 -0400 Subject: [PATCH 41/41] Address race condition between enabling chat bots and their reconnect jobs appearing in Live test --- tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs index 6bad61b0b2..e53f5d54bc 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/ChatTest.cs @@ -92,6 +92,8 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(true, updatedBot.Enabled); + await Task.Delay(TimeSpan.FromSeconds(5), cancellationToken); + var jobs = await JobsClient.List(null, cancellationToken); var reconnectJob = jobs .Where(x => x.StartedAt >= beforeChatBotEnabled && x.Description.Contains(updatedBot.Name)) @@ -189,6 +191,8 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(true, updatedBot.Enabled); + await Task.Delay(TimeSpan.FromSeconds(5), cancellationToken); + var jobs = await JobsClient.List(null, cancellationToken); var reconnectJob = jobs .Where(x => x.StartedAt >= beforeChatBotEnabled && x.Description.Contains(updatedBot.Name))