Merge pull request #683 from Cyberboss/MoreFixups

Various TestMerge/GitHub API fixups

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

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);
 
@@ -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();

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}";
+
 		/// <summary>
 		/// The <see cref="IGitHubClientFactory"/> for the <see cref="AdministrationController"/>
 		/// </summary>
@@ -128,6 +130,11 @@ namespace Tgstation.Server.Host.Controllers
 			{
 				return RateLimit(e);
 			}
+			catch (ApiException e)
+			{
+				Logger.LogWarning(OctokitException, e);
+				return StatusCode((int)HttpStatusCode.FailedDependency);
+			}
 		}
 
 		/// <inheritdoc />
@@ -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)

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;
 			}
 		}
 	}

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);
@@ -531,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;
@@ -663,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
@@ -706,7 +716,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);