From 7b19a41fa1614bad597b78bf9bb9fde0afddae6c Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Wed, 19 Sep 2018 14:04:54 -0400 Subject: [PATCH 1/6] Keep throwing cancelled requests --- src/Tgstation.Server.Host/Controllers/ApiController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Host/Controllers/ApiController.cs b/src/Tgstation.Server.Host/Controllers/ApiController.cs index 0fc660be6c..7b62fa020d 100644 --- a/src/Tgstation.Server.Host/Controllers/ApiController.cs +++ b/src/Tgstation.Server.Host/Controllers/ApiController.cs @@ -146,6 +146,7 @@ namespace Tgstation.Server.Host.Controllers catch (OperationCanceledException e) { Logger.LogDebug("Request cancelled! Exception: {0}", e); + throw; } } } From 9ad6d40a27323f5af74f1bfcf8da74cc860c3d00 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Wed, 19 Sep 2018 14:09:55 -0400 Subject: [PATCH 2/6] Clean up NonFastForwardException exceptions --- src/Tgstation.Server.Host/Components/Repository/Repository.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 7eb8890da0..10f7b12e84 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -550,6 +550,10 @@ namespace Tgstation.Server.Host.Components.Repository { repository.Network.Push(repository.Head, GeneratePushOptions(progressReporter, username, password, cancellationToken)); } + catch (NonFastForwardException) + { + logger.LogInformation("Synchronize aborted, non-fast forward!"); + } catch (UserCancelledException) { cancellationToken.ThrowIfCancellationRequested(); From bdd7f5f5dcb71edd38637b3f4fe640329ea97ce9 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Wed, 19 Sep 2018 14:12:16 -0400 Subject: [PATCH 3/6] Fix test merge commits sometimes not pushing --- .../Controllers/RepositoryController.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 7c18d5ec06..0c5ba6841b 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -428,6 +428,7 @@ namespace Tgstation.Server.Host.Controllers var startReference = repo.Reference; var startSha = repo.Head; + string postUpdateSha = null; if (newTestMerges && !repo.IsGitHubRepository) throw new JobException("Cannot test merge on a non GitHub based repository!"); @@ -486,6 +487,7 @@ namespace Tgstation.Server.Host.Controllers { lastRevisionInfo.OriginCommitSha = repo.Head; await repo.Sychronize(currentModel.AccessUser, currentModel.AccessToken, currentModel.CommitterName, currentModel.CommitterEmail, NextProgressReporter(), true, ct).ConfigureAwait(false); + postUpdateSha = repo.Head; } else NextProgressReporter()(100); @@ -706,7 +708,8 @@ namespace Tgstation.Server.Host.Controllers } } - if (startSha != repo.Head) + var currentHead = repo.Head; + if (startSha != currentHead || (postUpdateSha != null && postUpdateSha != currentHead)) { await repo.Sychronize(currentModel.AccessUser, currentModel.AccessToken, currentModel.CommitterName, currentModel.CommitterEmail, NextProgressReporter(), false, ct).ConfigureAwait(false); await UpdateRevInfo().ConfigureAwait(false); From efd347bfbd7172025ee5f5de43a32ec35639be86 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Wed, 19 Sep 2018 14:16:31 -0400 Subject: [PATCH 4/6] Prefer instance GitHub credentials to global ones --- .../Controllers/RepositoryController.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index 0c5ba6841b..3ca9664847 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -533,7 +533,11 @@ namespace Tgstation.Server.Host.Controllers foreach (var I in model.NewTestMerges.Where(x => String.IsNullOrWhiteSpace(x.PullRequestRevision))) I.PullRequestRevision = null; - var gitHubClient = String.IsNullOrEmpty(generalConfiguration.GitHubAccessToken) ? (currentModel.AccessToken != null ? gitHubClientFactory.CreateClient(currentModel.AccessToken) : gitHubClientFactory.CreateClient()) : gitHubClientFactory.CreateClient(generalConfiguration.GitHubAccessToken); + var gitHubClient = currentModel.AccessToken != null + ? gitHubClientFactory.CreateClient(currentModel.AccessToken) + : (String.IsNullOrEmpty(generalConfiguration.GitHubAccessToken) + ? gitHubClientFactory.CreateClient() + : gitHubClientFactory.CreateClient(generalConfiguration.GitHubAccessToken)); var repoOwner = repo.GitHubOwner; var repoName = repo.GitHubRepoName; @@ -665,6 +669,10 @@ namespace Tgstation.Server.Host.Controllers //you look at your anonymous access and sigh errorMessage = "P.R.E. RATE LIMITED"; } + catch (Octokit.AuthorizationException) + { + errorMessage = "P.R.E. BAD CREDENTIALS"; + } catch (Octokit.NotFoundException) { //you look at your shithub and sigh From 1db1fc35b58b7adc8c54585e4033b61e14361380 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Wed, 19 Sep 2018 14:19:02 -0400 Subject: [PATCH 5/6] At this level, AddTestMerge should throw an InvalidOperationException --- src/Tgstation.Server.Host/Components/Repository/Repository.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 10f7b12e84..d6cc835f61 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -203,7 +203,7 @@ namespace Tgstation.Server.Host.Components.Repository logger.LogDebug("Begin AddTestMerge: #{0} at {1} ({4}) by <{2} ({3})>", testMergeParameters.Number, testMergeParameters.PullRequestRevision?.Substring(0, 7), committerName, committerEmail, testMergeParameters.Comment); if (!IsGitHubRepository) - throw new JobException("Test merging is only available on GitHub hosted origin repositories!"); + throw new InvalidOperationException("Test merging is only available on GitHub hosted origin repositories!"); var commitMessage = String.Format(CultureInfo.InvariantCulture, "Test merge of pull request #{0}{1}{2}", testMergeParameters.Number.Value, testMergeParameters.Comment != null ? Environment.NewLine : String.Empty, testMergeParameters.Comment ?? String.Empty); From 3b9065061cddaefd0d265e1f4c7e00bc248666b2 Mon Sep 17 00:00:00 2001 From: Cyberboss Date: Wed, 19 Sep 2018 14:50:47 -0400 Subject: [PATCH 6/6] Return failed dependency when random GitHub API exceptions occur --- docs/API.dox | 1 + .../Controllers/AdministrationController.cs | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/docs/API.dox b/docs/API.dox index c05ca4243d..79ca621469 100644 --- a/docs/API.dox +++ b/docs/API.dox @@ -62,6 +62,7 @@ TGS will only every return the response codes listed here - 409: Conflict. Documented in the requests that use them - 410: Gone. Attempted to access/modify a resource that ideally should have been ready, but isn't or no longer is - 422: Unprocessable Entity: Used specifically when an operation that requires a server restart is unable to be performed due to the @ref Tgstation.Server.Host.Watchdog not being present in the deployment. Blame MSO. Response body contains an @ref Tgstation.Server.Api.Models.ErrorMessage +- 424: Failed Dependency: When a request that depends on the GitHub API fails for a reason other than rate limiting. Check server logs, usually this indicates a bad access token. - 426: Upgrade required: Used when the client's API version is not compatible with the server's. Response body contains an @ref Tgstation.Server.Api.Models.ErrorMessage - 429: Rate limited. Used with operations that rely on GitHub.com. If a rate limit is hit for an operation this will be returned. Response will contain a Retry-After header - 500: Server error. Please report the request and response body to the code repository diff --git a/src/Tgstation.Server.Host/Controllers/AdministrationController.cs b/src/Tgstation.Server.Host/Controllers/AdministrationController.cs index e3d8fb5e6a..553163da07 100644 --- a/src/Tgstation.Server.Host/Controllers/AdministrationController.cs +++ b/src/Tgstation.Server.Host/Controllers/AdministrationController.cs @@ -30,6 +30,8 @@ namespace Tgstation.Server.Host.Controllers { const string RestartNotSupportedException = "This deployment of tgstation-server is lacking the Tgstation.Server.Host.Watchdog component. Restarts and version changes cannot be completed!"; + const string OctokitException = "Bad GitHub API response, check configuration! Exception: {0}"; + /// /// The for the /// @@ -128,6 +130,11 @@ namespace Tgstation.Server.Host.Controllers { return RateLimit(e); } + catch (ApiException e) + { + Logger.LogWarning(OctokitException, e); + return StatusCode((int)HttpStatusCode.FailedDependency); + } } /// @@ -154,6 +161,11 @@ namespace Tgstation.Server.Host.Controllers { return RateLimit(e); } + catch (ApiException e) + { + Logger.LogWarning(OctokitException, e); + return StatusCode((int)HttpStatusCode.FailedDependency); + } Logger.LogTrace("Release query complete!"); foreach (var release in releases)