From 7f45281c4461834f55cffa7daa8e369ba6c32b19 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 Oct 2021 16:22:48 -0400 Subject: [PATCH 01/14] Improve logging of failed OAuth handshakes --- .../Security/OAuth/GenericOAuthValidator.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Security/OAuth/GenericOAuthValidator.cs b/src/Tgstation.Server.Host/Security/OAuth/GenericOAuthValidator.cs index 1fe959385c..aa834e38a7 100644 --- a/src/Tgstation.Server.Host/Security/OAuth/GenericOAuthValidator.cs +++ b/src/Tgstation.Server.Host/Security/OAuth/GenericOAuthValidator.cs @@ -55,6 +55,8 @@ namespace Tgstation.Server.Host.Security.OAuth public override async Task ValidateResponseCode(string code, CancellationToken cancellationToken) { using var httpClient = CreateHttpClient(); + string tokenResponsePayload = null; + string userInformationPayload = null; try { Logger.LogTrace("Validating response code..."); @@ -71,8 +73,8 @@ namespace Tgstation.Server.Host.Security.OAuth tokenRequest.Content = new FormUrlEncodedContent(tokenRequestDictionary); var tokenResponse = await httpClient.SendAsync(tokenRequest, cancellationToken).ConfigureAwait(false); + tokenResponsePayload = await tokenResponse.Content.ReadAsStringAsync().ConfigureAwait(false); tokenResponse.EnsureSuccessStatusCode(); - var tokenResponsePayload = await tokenResponse.Content.ReadAsStringAsync().ConfigureAwait(false); var tokenResponseJson = JObject.Parse(tokenResponsePayload); var accessToken = DecodeTokenPayload(tokenResponseJson); @@ -89,16 +91,20 @@ namespace Tgstation.Server.Host.Security.OAuth accessToken); var userInformationResponse = await httpClient.SendAsync(userInformationRequest, cancellationToken).ConfigureAwait(false); + userInformationPayload = await userInformationResponse.Content.ReadAsStringAsync().ConfigureAwait(false); userInformationResponse.EnsureSuccessStatusCode(); - var userInformationPayload = await userInformationResponse.Content.ReadAsStringAsync().ConfigureAwait(false); var userInformationJson = JObject.Parse(userInformationPayload); return DecodeUserInformationPayload(userInformationJson); } catch (Exception ex) { - Logger.LogWarning(ex, "Error while completing OAuth handshake!"); + Logger.LogWarning( + ex, + "Error while completing OAuth handshake! Payload:{newLine}{responsePayload}", + Environment.NewLine, + userInformationPayload ?? tokenResponsePayload); return null; } } From d0e3ae5c55674193aa3f1f71f321fb937a57354c Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 Oct 2021 17:17:27 -0400 Subject: [PATCH 02/14] Version bump to 4.15.4 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index 1609c6d1e7..8d2a18c79a 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 4.15.3 + 4.15.4 4.0.0 9.3.0 9.3.1 From 89960c61f0872ce51392dc226ba820d3b42e32bd Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Fri, 8 Oct 2021 17:18:35 -0400 Subject: [PATCH 03/14] Fix docker build labelling --- .github/workflows/ci-suite.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-suite.yml b/.github/workflows/ci-suite.yml index ffdca312c0..c59b672f23 100644 --- a/.github/workflows/ci-suite.yml +++ b/.github/workflows/ci-suite.yml @@ -773,7 +773,7 @@ jobs: run: | sudo apt-get update sudo apt-get install -y xmlstarlet - xmlstarlet sel -N X="http://schemas.microsoft.com/developer/msbuild/2003" --template --value-of /X:Project/X:PropertyGroup/X:TgsCoreVersion build/Version.props >> $Env:GITHUB_ENV + echo "TGS_VERSION=$(xmlstarlet sel -N X="http://schemas.microsoft.com/developer/msbuild/2003" --template --value-of /X:Project/X:PropertyGroup/X:TgsCoreVersion build/Version.props)" >> $Env:GITHUB_ENV - name: Docker Build and Push uses: elgohr/Publish-Docker-Github-Action@master From c3dfec103486151f9acee948dba2373d76fb5104 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sat, 9 Oct 2021 10:43:47 -0400 Subject: [PATCH 04/14] Fix chat manager issues hopefully --- .../Components/Chat/ChatManager.cs | 60 +++++++++---------- .../Chat/Providers/DiscordProvider.cs | 35 +++++------ .../Components/Chat/Providers/Provider.cs | 3 + 3 files changed, 46 insertions(+), 52 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs index b5f105ea3e..3aaf187d80 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatManager.cs @@ -190,9 +190,16 @@ namespace Tgstation.Server.Host.Components.Chat throw new ArgumentNullException(nameof(newChannels)); logger.LogTrace("ChangeChannels {0}...", connectionId); - var provider = await RemoveProvider(connectionId, false, cancellationToken).ConfigureAwait(false); + var provider = await RemoveProviderChannels(connectionId, false, cancellationToken).ConfigureAwait(false); if (provider == null) return; + + if (!provider.Connected) + { + logger.LogDebug("Cannot map channels, provider {providerId} disconnected!", connectionId); + return; + } + var results = await provider.MapChannels(newChannels, cancellationToken).ConfigureAwait(false); lock (activeChatBots) { @@ -260,29 +267,14 @@ namespace Tgstation.Server.Host.Components.Chat throw new ArgumentNullException(nameof(newSettings)); logger.LogTrace("ChangeSettings..."); - IProvider provider; - - async Task DisconnectProvider(IProvider p) - { - try - { - await p.Disconnect(cancellationToken).ConfigureAwait(false); - } - finally - { - await p.DisposeAsync().ConfigureAwait(false); - } - } Task disconnectTask; + IProvider provider = null; lock (providers) { // raw settings changes forces a rebuild of the provider - if (providers.TryGetValue(newSettings.Id.Value, out provider)) - { - providers.Remove(newSettings.Id.Value); - disconnectTask = DisconnectProvider(provider); - } + if (providers.ContainsKey(newSettings.Id.Value)) + disconnectTask = DeleteConnection(newSettings.Id.Value, cancellationToken); else disconnectTask = Task.CompletedTask; if (newSettings.Enabled.Value) @@ -430,7 +422,6 @@ namespace Tgstation.Server.Host.Components.Chat builtinCommands.Add(tgsCommand.Name.ToUpperInvariant(), tgsCommand); var initialChatBots = activeChatBots.ToList(); await Task.WhenAll(initialChatBots.Select(x => ChangeSettings(x, cancellationToken))).ConfigureAwait(false); - await Task.WhenAll(initialChatBots.Select(x => ChangeChannels(x.Id.Value, x.Channels, cancellationToken))).ConfigureAwait(false); initialProviderConnectionsTask = InitialConnection(); chatHandler = MonitorMessages(handlerCts.Token); } @@ -441,7 +432,7 @@ namespace Tgstation.Server.Host.Components.Chat handlerCts.Cancel(); if (chatHandler != null) await chatHandler.ConfigureAwait(false); - await Task.WhenAll(providers.Select(x => x.Value).Select(x => x.Disconnect(cancellationToken))).ConfigureAwait(false); + await Task.WhenAll(providers.Select(x => x.Key).Select(x => DeleteConnection(x, cancellationToken))).ConfigureAwait(false); await messageSendTask.ConfigureAwait(false); } @@ -480,7 +471,7 @@ namespace Tgstation.Server.Host.Components.Chat /// public async Task DeleteConnection(long connectionId, CancellationToken cancellationToken) { - var provider = await RemoveProvider(connectionId, true, cancellationToken).ConfigureAwait(false); + var provider = await RemoveProviderChannels(connectionId, true, cancellationToken).ConfigureAwait(false); if (provider != null) try { @@ -510,23 +501,28 @@ namespace Tgstation.Server.Host.Components.Chat } /// - /// Remove a from and optionally updating the as well. + /// Remove a from optionally removing the provider itself from and updating the as well. /// /// The of the to delete. - /// If should be update. + /// If the provider should be removed from and should be update. /// The for the operation. /// A resulting in the being removed if it exists, otherwise. - async Task RemoveProvider(long connectionId, bool updateTrackings, CancellationToken cancellationToken) + async Task RemoveProviderChannels(long connectionId, bool removeProvider, CancellationToken cancellationToken) { - logger.LogTrace("RemoveProvider {0}...", connectionId); + logger.LogTrace("RemoveProviderChannels {0}...", connectionId); IProvider provider; lock (providers) + { if (!providers.TryGetValue(connectionId, out provider)) { logger.LogTrace("Aborted, no such provider!"); return null; } + if (removeProvider) + providers.Remove(connectionId); + } + Task trackingContextsUpdateTask; lock (mappedChannels) { @@ -535,7 +531,7 @@ namespace Tgstation.Server.Host.Components.Chat var newMappedChannels = mappedChannels.Select(y => y.Value.Channel).ToList(); - if (updateTrackings) + if (removeProvider) lock (trackingContexts) trackingContextsUpdateTask = Task.WhenAll(trackingContexts.Select(x => x.UpdateChannels(newMappedChannels, cancellationToken))); else @@ -607,6 +603,7 @@ namespace Tgstation.Server.Host.Components.Chat .First(); mappedChannel = mappedChannels .Where(x => x.Value.ProviderId == providerId && x.Value.ProviderChannelId == providerChannelId) + .Select(x => (KeyValuePair?)x) .FirstOrDefault(); } @@ -652,12 +649,11 @@ namespace Tgstation.Server.Host.Components.Chat return; } - var mappingNonNullableKvp = mappedChannel.Value; - var mapping = mappingNonNullableKvp.Value; + var mappingChannelRepresentation = mappedChannel.Value.Value.Channel; - message.User.Channel.Id = mapping.Channel.Id; - message.User.Channel.Tag = mapping.Channel.Tag; - message.User.Channel.IsAdminChannel = mapping.Channel.IsAdminChannel; + message.User.Channel.Id = mappingChannelRepresentation.Id; + message.User.Channel.Tag = mappingChannelRepresentation.Tag; + message.User.Channel.IsAdminChannel = mappingChannelRepresentation.IsAdminChannel; } var splits = new List(message.Content.Trim().Split(' ')); diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index cc6942b1d2..dc018973f8 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -93,6 +93,11 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// Snowflake currentUserId; + /// + /// The bot's username at the time of connection. + /// + string initialUserName; + /// /// Normalize a discord mention string. /// @@ -194,20 +199,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers if (channels == null) throw new ArgumentNullException(nameof(channels)); - if (!Connected) - { - Logger.LogWarning("Cannot map channels, provider disconnected!"); - return Array.Empty(); - } - - var usersClient = serviceProvider.GetRequiredService(); - var currentUserResponse = await usersClient.GetCurrentUserAsync(cancellationToken).ConfigureAwait(false); - - if (!currentUserResponse.IsSuccess) - { - Logger.LogWarning("Error retrieving current Discord user: {0}", currentUserResponse.Error.Message); - return Array.Empty(); - } + bool remapRequired = false; async Task GetModelChannelFromDBChannel(Api.Models.ChatChannel channelFromDB) { @@ -215,14 +207,12 @@ namespace Tgstation.Server.Host.Components.Chat.Providers throw new InvalidOperationException("ChatChannel missing DiscordChannelId!"); var channelId = channelFromDB.DiscordChannelId.Value; - ulong discordChannelId; string connectionName; string friendlyName; if (channelId == 0) { - connectionName = currentUserResponse.Entity.Username; + connectionName = initialUserName; friendlyName = "(Unmapped accessible channels)"; - discordChannelId = 0; } else { @@ -231,6 +221,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers if (!discordChannelResponse.IsSuccess) { Logger.LogWarning("Error retrieving discord channel {0}: {1}", channelId, discordChannelResponse.Error.Message); + remapRequired = true; return null; } @@ -240,7 +231,6 @@ namespace Tgstation.Server.Host.Components.Chat.Providers return null; } - discordChannelId = discordChannelResponse.Entity.ID.Value; friendlyName = discordChannelResponse.Entity.Name.Value; var guildsClient = serviceProvider.GetRequiredService(); @@ -254,6 +244,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers "Error retrieving discord guild {0}: {1}", discordChannelResponse.Entity.GuildID.Value, discordChannelResponse.Error.Message); + remapRequired = true; return null; } @@ -262,7 +253,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers var channelModel = new ChannelRepresentation { - RealId = discordChannelId, + RealId = channelId, IsAdminChannel = channelFromDB.IsAdminChannel == true, ConnectionName = connectionName, FriendlyName = friendlyName, @@ -276,13 +267,13 @@ namespace Tgstation.Server.Host.Components.Chat.Providers var tasks = channels .Select(x => GetModelChannelFromDBChannel(x)) - .Where(x => x != null) .ToList(); await Task.WhenAll(tasks); var enumerator = tasks .Select(x => x.Result) + .Where(x => x != null) .ToList(); lock (mappedChannels) @@ -291,6 +282,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers mappedChannels.AddRange(enumerator.Select(x => x.RealId)); } + if (remapRequired) + EnqueueMessage(null); + return enumerator; } @@ -634,6 +628,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers } currentUserId = currentUserResult.Entity.ID; + initialUserName = currentUserResult.Entity.Username; } finally { diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs index bfdebd4dc8..1b589fa4a3 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs @@ -183,6 +183,9 @@ namespace Tgstation.Server.Host.Components.Chat.Providers /// The to queue. protected void EnqueueMessage(Message message) { + if (message == null) + Logger.LogTrace("Requesting channel remap..."); + lock (messageQueue) { messageQueue.Enqueue(message); From 89b1c4044e9586dd4e5c174b6445120d8d1bf80d Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sat, 9 Oct 2021 17:36:14 -0400 Subject: [PATCH 05/14] Update Remora.Discord to 3.0.71 --- src/Tgstation.Server.Host/Tgstation.Server.Host.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj b/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj index fe8a7385ed..3d7385b40f 100644 --- a/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj +++ b/src/Tgstation.Server.Host/Tgstation.Server.Host.csproj @@ -87,7 +87,7 @@ - + From 1c18931534ad1cf86343dd753a7926b82db7fbde Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 12 Oct 2021 13:09:29 -0400 Subject: [PATCH 06/14] Additional logging --- .../Components/Chat/Providers/DiscordProvider.cs | 1 + src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index dc018973f8..88755dc418 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -607,6 +607,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers gatewayReadyTcs = new TaskCompletionSource(); using var gatewayConnectionAbortRegistration = cancellationToken.Register(() => gatewayReadyTcs.TrySetCanceled()); + gatewayCancellationToken.Register(() => Logger.LogTrace("Stopping gateway client...")); // reconnects keep happening until we stop or it faults, our auto-reconnector will handle the latter localGatewayTask = gatewayClient.RunAsync(gatewayCancellationToken); diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs index 1b589fa4a3..fa13f46f12 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs @@ -107,6 +107,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers if (Connected) { + Logger.LogTrace("Disconnecting..."); await DisconnectImpl(cancellationToken).ConfigureAwait(false); Logger.LogTrace("Disconnected"); } From eeb69eed77f56aca70d9f024db4a8a2c22e7820e Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 12 Oct 2021 13:12:04 -0400 Subject: [PATCH 07/14] Better translate bad credentials on submodule operations --- .../Repository/ICredentialsProvider.cs | 11 +++++++++- .../Repository/LibGit2RepositoryFactory.cs | 22 +++++++++++++++++++ .../Components/Repository/Repository.cs | 21 ++++-------------- .../Repository/RepositoryManager.cs | 6 ++--- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Repository/ICredentialsProvider.cs b/src/Tgstation.Server.Host/Components/Repository/ICredentialsProvider.cs index 3568f0747e..cef08354c6 100644 --- a/src/Tgstation.Server.Host/Components/Repository/ICredentialsProvider.cs +++ b/src/Tgstation.Server.Host/Components/Repository/ICredentialsProvider.cs @@ -1,4 +1,7 @@ -using LibGit2Sharp.Handlers; +using LibGit2Sharp; +using LibGit2Sharp.Handlers; + +using Tgstation.Server.Host.Jobs; namespace Tgstation.Server.Host.Components.Repository { @@ -14,5 +17,11 @@ namespace Tgstation.Server.Host.Components.Repository /// The optional password to use in the . /// A new . CredentialsHandler GenerateCredentialsHandler(string username, string password); + + /// + /// Rethrow the authentication failure message as a if it is one. + /// + /// The current . + public void CheckBadCredentialsException(LibGit2SharpException exception); } } diff --git a/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs b/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs index 1dda89d84d..2c26a74fcf 100644 --- a/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs +++ b/src/Tgstation.Server.Host/Components/Repository/LibGit2RepositoryFactory.cs @@ -71,6 +71,11 @@ namespace Tgstation.Server.Host.Components.Repository logger.LogTrace(ex, "Suppressing clone cancellation exception"); cancellationToken.ThrowIfCancellationRequested(); } + catch (LibGit2SharpException ex) + { + CheckBadCredentialsException(ex); + throw; + } }, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, @@ -99,5 +104,22 @@ namespace Tgstation.Server.Host.Components.Repository throw new JobException(ErrorCode.RepoCannotAuthenticate); }; + + /// + public void CheckBadCredentialsException(LibGit2SharpException exception) + { + if (exception == null) + throw new ArgumentNullException(nameof(exception)); + + if (exception.Message == "too many redirects or authentication replays") + throw new JobException("Bad git credentials exchange!", exception); + + if (exception.Message == ErrorCode.RepoCredentialsRequired.Describe()) + throw new JobException(ErrorCode.RepoCredentialsRequired); + + // submodule recursion + if (exception.InnerException is LibGit2SharpException innerException) + CheckBadCredentialsException(innerException); + } } } diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 0ebddb2640..452418ce5f 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -111,19 +111,6 @@ namespace Tgstation.Server.Host.Components.Repository /// bool disposed; - /// - /// Rethrow the authentication failure message as a if it is one. - /// - /// The current . - static void CheckBadCredentialsException(LibGit2SharpException exception) - { - if (exception.Message == "too many redirects or authentication replays") - throw new JobException("Bad git credentials exchange!", exception); - - if (exception.Message == ErrorCode.RepoCredentialsRequired.Describe()) - throw new JobException(ErrorCode.RepoCredentialsRequired); - } - /// /// Initializes a new instance of the class. /// @@ -262,7 +249,7 @@ namespace Tgstation.Server.Host.Components.Repository } catch (LibGit2SharpException ex) { - CheckBadCredentialsException(ex); + credentialsProvider.CheckBadCredentialsException(ex); } cancellationToken.ThrowIfCancellationRequested(); @@ -439,7 +426,7 @@ namespace Tgstation.Server.Host.Components.Repository } catch (LibGit2SharpException ex) { - CheckBadCredentialsException(ex); + credentialsProvider.CheckBadCredentialsException(ex); } }, cancellationToken, @@ -982,7 +969,7 @@ namespace Tgstation.Server.Host.Components.Repository { // workaround for https://github.com/libgit2/libgit2/issues/3820 // kill off the modules/ folder in .git and try again - CheckBadCredentialsException(ex); + credentialsProvider.CheckBadCredentialsException(ex); logger.LogWarning(ex, "Initial update of submodule {0} failed. Deleting submodule directories and re-attempting...", submodule.Name); await Task.WhenAll( @@ -1001,7 +988,7 @@ namespace Tgstation.Server.Host.Components.Repository } catch (LibGit2SharpException ex2) { - CheckBadCredentialsException(ex2); + credentialsProvider.CheckBadCredentialsException(ex2); logger.LogTrace(ex2, "Retried update of submodule {0} failed!", submodule.Name); throw new AggregateException(ex, ex2); } diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs index 41cd9eb7b7..bb424f25f4 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs @@ -153,7 +153,7 @@ namespace Tgstation.Server.Host.Components.Repository cancellationToken) .ConfigureAwait(false); } - catch + catch (Exception ex) { try { @@ -162,9 +162,9 @@ namespace Tgstation.Server.Host.Components.Repository // DCT: Cancellation token is for job, operation must run regardless await ioManager.DeleteDirectory(repositoryPath, default).ConfigureAwait(false); } - catch (Exception e) + catch (Exception innerException) { - logger.LogDebug(e, "Error deleting partially cloned repository!"); + logger.LogError(innerException, "Error deleting partially cloned repository!"); } throw; From 8f22548c40802e929b253436ab39057e4ee0d8bd Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 12 Oct 2021 13:18:40 -0400 Subject: [PATCH 08/14] Remove unused declaration --- .../Components/Repository/RepositoryManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs index bb424f25f4..bb3fbe09b1 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs @@ -153,7 +153,7 @@ namespace Tgstation.Server.Host.Components.Repository cancellationToken) .ConfigureAwait(false); } - catch (Exception ex) + catch { try { From 2f9f871580e63a12b145919ddb6985cc1f362a24 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 12 Oct 2021 15:22:19 -0400 Subject: [PATCH 09/14] Fix checkout initialization progress --- .../Components/Repository/Repository.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 452418ce5f..67401c31c5 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -276,7 +276,7 @@ namespace Tgstation.Server.Host.Components.Repository FastForwardStrategy = FastForwardStrategy.NoFastForward, SkipReuc = true, OnCheckoutProgress = CheckoutProgressHandler( - (lambdaStage, progress) => progressReporter(lambdaStage, 50 + (progress / 2)), + (lambdaStage, progress) => progressReporter(lambdaStage, progress.HasValue ? 50 + (progress.Value / 2) : null), $"Merge {testMergeParameters.TargetCommitSha}"), }); } @@ -646,7 +646,7 @@ namespace Tgstation.Server.Host.Components.Repository { libGitRepo.Reset(ResetMode.Hard, libGitRepo.Head.Tip, new CheckoutOptions { - OnCheckoutProgress = CheckoutProgressHandler((stage, progress) => progressReporter(stage, progress / 10), "Hard reset and remove untracked files"), + OnCheckoutProgress = CheckoutProgressHandler((stage, progress) => progressReporter(stage, progress.HasValue ? progress.Value / 10 : null), "Hard reset and remove untracked files"), }); cancellationToken.ThrowIfCancellationRequested(); libGitRepo.RemoveUntrackedFiles(); @@ -939,7 +939,7 @@ namespace Tgstation.Server.Host.Components.Repository var factor = 100 / submoduleCount; foreach (var submodule in libGitRepo.Submodules) { - void LocalProgressReporter(string stage, int percentage) => progressReporter(stage, (iteration * factor) + (percentage / submoduleCount)); + void LocalProgressReporter(string stage, int? percentage) => progressReporter(stage, percentage.HasValue ? (iteration * factor) + (percentage.Value / submoduleCount) : null); var submoduleUpdateOptions = new SubmoduleUpdateOptions { Init = true, @@ -951,7 +951,7 @@ namespace Tgstation.Server.Host.Components.Repository OnUpdateTips = (a, b, c) => !cancellationToken.IsCancellationRequested, CredentialsProvider = credentialsProvider.GenerateCredentialsHandler(username, password), OnCheckoutProgress = CheckoutProgressHandler( - (stage, progress) => LocalProgressReporter(stage, 50 + (progress.Value / 2)), + (stage, progress) => LocalProgressReporter(stage, progress.HasValue ? 50 + (progress.Value / 2) : null), $"Checkout submodule {submodule.Name}"), }; @@ -1007,7 +1007,11 @@ namespace Tgstation.Server.Host.Components.Repository CheckoutProgressHandler CheckoutProgressHandler(JobProgressReporter progressReporter, string stage) => (a, completedSteps, totalSteps) => { int? percentage; - if (totalSteps < completedSteps || totalSteps == 0) + + // short circuit initialization where totalSteps is 0 + if (completedSteps == 0) + percentage = 0; + else if (totalSteps < completedSteps || totalSteps == 0) percentage = null; else { From e310886c773f69b8d2812010b03f4ad140bdef28 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 12 Oct 2021 16:26:59 -0400 Subject: [PATCH 10/14] Fix bad conversions --- .../Components/Repository/Repository.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 67401c31c5..68febe458b 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -276,7 +276,7 @@ namespace Tgstation.Server.Host.Components.Repository FastForwardStrategy = FastForwardStrategy.NoFastForward, SkipReuc = true, OnCheckoutProgress = CheckoutProgressHandler( - (lambdaStage, progress) => progressReporter(lambdaStage, progress.HasValue ? 50 + (progress.Value / 2) : null), + (lambdaStage, progress) => progressReporter(lambdaStage, progress.HasValue ? (int?)(50 + (progress.Value / 2)) : null), $"Merge {testMergeParameters.TargetCommitSha}"), }); } @@ -646,7 +646,7 @@ namespace Tgstation.Server.Host.Components.Repository { libGitRepo.Reset(ResetMode.Hard, libGitRepo.Head.Tip, new CheckoutOptions { - OnCheckoutProgress = CheckoutProgressHandler((stage, progress) => progressReporter(stage, progress.HasValue ? progress.Value / 10 : null), "Hard reset and remove untracked files"), + OnCheckoutProgress = CheckoutProgressHandler((stage, progress) => progressReporter(stage, progress.HasValue ? (int?)(progress.Value / 10) : null), "Hard reset and remove untracked files"), }); cancellationToken.ThrowIfCancellationRequested(); libGitRepo.RemoveUntrackedFiles(); @@ -939,7 +939,7 @@ namespace Tgstation.Server.Host.Components.Repository var factor = 100 / submoduleCount; foreach (var submodule in libGitRepo.Submodules) { - void LocalProgressReporter(string stage, int? percentage) => progressReporter(stage, percentage.HasValue ? (iteration * factor) + (percentage.Value / submoduleCount) : null); + void LocalProgressReporter(string stage, int? percentage) => progressReporter(stage, percentage.HasValue ? (int?)((iteration * factor) + (percentage.Value / submoduleCount)) : null); var submoduleUpdateOptions = new SubmoduleUpdateOptions { Init = true, @@ -951,7 +951,7 @@ namespace Tgstation.Server.Host.Components.Repository OnUpdateTips = (a, b, c) => !cancellationToken.IsCancellationRequested, CredentialsProvider = credentialsProvider.GenerateCredentialsHandler(username, password), OnCheckoutProgress = CheckoutProgressHandler( - (stage, progress) => LocalProgressReporter(stage, progress.HasValue ? 50 + (progress.Value / 2) : null), + (stage, progress) => LocalProgressReporter(stage, progress.HasValue ? (int?)(50 + (progress.Value / 2)) : null), $"Checkout submodule {submodule.Name}"), }; From 110d560220b98982afda8229473b8b973fbb6483 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Tue, 12 Oct 2021 20:29:09 -0400 Subject: [PATCH 11/14] We don't need the chat reconnection jobs to succeed here --- tests/Tgstation.Server.Tests/Instance/JobsRequiredTest.cs | 6 +++--- tests/Tgstation.Server.Tests/IntegrationTest.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Instance/JobsRequiredTest.cs b/tests/Tgstation.Server.Tests/Instance/JobsRequiredTest.cs index f10a6efc2c..9728ec9eef 100644 --- a/tests/Tgstation.Server.Tests/Instance/JobsRequiredTest.cs +++ b/tests/Tgstation.Server.Tests/Instance/JobsRequiredTest.cs @@ -1,4 +1,4 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.VisualStudio.TestTools.UnitTesting; using System; using System.Collections.Generic; using System.Text; @@ -20,7 +20,7 @@ namespace Tgstation.Server.Tests.Instance this.JobsClient = jobsClient; } - public async Task WaitForJob(JobResponse originalJob, int timeout, bool expectFailure, ErrorCode? expectedCode, CancellationToken cancellationToken) + public async Task WaitForJob(JobResponse originalJob, int timeout, bool? expectFailure, ErrorCode? expectedCode, CancellationToken cancellationToken) { var job = originalJob; do @@ -37,7 +37,7 @@ namespace Tgstation.Server.Tests.Instance Assert.Fail($"Job ID {job.Id} \"{job.Description}\" timed out!"); } - if (expectFailure ^ job.ExceptionDetails != null) + if(expectFailure.HasValue && (expectFailure.Value ^ job.ExceptionDetails != null)) Assert.Fail(job.ExceptionDetails ?? $"Expected job \"{job.Id}\" \"{job.Description}\" to fail {(expectedCode.HasValue ? $"with ErrorCode \"{expectedCode.Value}\" " : String.Empty)}but it didn't"); diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index a1b7b1a5c6..ef33a03ccd 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -958,7 +958,7 @@ namespace Tgstation.Server.Tests foreach (var job in jobs) { Assert.IsTrue(job.StartedAt.Value >= preStartupTime); - await jrt.WaitForJob(job, 140, false, null, cancellationToken); + await jrt.WaitForJob(job, 140, job.Description.Contains("Reconnect chat bot") ? false : null, null, cancellationToken); } var dd = await instanceClient.DreamDaemon.Read(cancellationToken); From 23a91056cc1fcb0cb5a3f9ee8f4343f4c736090f Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 13 Oct 2021 09:25:18 -0400 Subject: [PATCH 12/14] Hoisted by old C# again --- tests/Tgstation.Server.Tests/IntegrationTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/IntegrationTest.cs b/tests/Tgstation.Server.Tests/IntegrationTest.cs index ef33a03ccd..f47f166cfa 100644 --- a/tests/Tgstation.Server.Tests/IntegrationTest.cs +++ b/tests/Tgstation.Server.Tests/IntegrationTest.cs @@ -958,7 +958,7 @@ namespace Tgstation.Server.Tests foreach (var job in jobs) { Assert.IsTrue(job.StartedAt.Value >= preStartupTime); - await jrt.WaitForJob(job, 140, job.Description.Contains("Reconnect chat bot") ? false : null, null, cancellationToken); + await jrt.WaitForJob(job, 140, job.Description.Contains("Reconnect chat bot") ? (bool?)null : (bool?)false, null, cancellationToken); } var dd = await instanceClient.DreamDaemon.Read(cancellationToken); From afa9d192de825636d171656a7795549f825f10f4 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 13 Oct 2021 11:38:46 -0400 Subject: [PATCH 13/14] Did I actually just fix CI? --- .github/workflows/ci-suite.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/ci-suite.yml b/.github/workflows/ci-suite.yml index c59b672f23..64ca278308 100644 --- a/.github/workflows/ci-suite.yml +++ b/.github/workflows/ci-suite.yml @@ -187,9 +187,7 @@ jobs: env: TGS_TEST_DATABASE_TYPE: SqlServer TGS_TEST_DUMP_API_SPEC: yes - concurrency: integration-windows-${{ github.head_ref }} strategy: - max-parallel: 2 matrix: watchdog-type: [ 'Basic', 'System' ] configuration: [ 'Debug', 'Release' ] @@ -303,9 +301,7 @@ jobs: --health-interval=10s --health-timeout=5s --health-retries=3 - concurrency: integration-linux-${{ github.head_ref }} strategy: - max-parallel: 2 matrix: database-type: [ 'Sqlite', 'PostgresSql', 'MariaDB', 'MySql' ] watchdog-type: [ 'System' ] From 5c9c47a36f6eb2a146352259e3ced9bcce76fb15 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Wed, 13 Oct 2021 11:38:55 -0400 Subject: [PATCH 14/14] Extremely minor code cleanup --- src/Tgstation.Server.Host/Controllers/InstanceController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Controllers/InstanceController.cs b/src/Tgstation.Server.Host/Controllers/InstanceController.cs index 74fa613001..77793afe08 100644 --- a/src/Tgstation.Server.Host/Controllers/InstanceController.cs +++ b/src/Tgstation.Server.Host/Controllers/InstanceController.cs @@ -519,7 +519,7 @@ namespace Tgstation.Server.Host.Controllers { IQueryable GetBaseQuery() { - IQueryable query = DatabaseContext + var query = DatabaseContext .Instances .AsQueryable() .Where(x => x.SwarmIdentifer == swarmConfiguration.Identifier);