From 4d1c29c1cb8cc9003691114e9e9982c036fbf5fc Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Sat, 18 Aug 2018 14:01:44 -0400 Subject: [PATCH] Add JobException to avoid printing needless stack traces for normal operation errors --- .../Components/Byond/WindowsByondInstaller.cs | 2 +- .../Components/Compiler/DreamMaker.cs | 6 ++-- .../Components/Repository/Repository.cs | 2 +- .../Components/Watchdog/Watchdog.cs | 6 ++-- .../Controllers/RepositoryController.cs | 14 ++++---- .../Core/JobException.cs | 34 +++++++++++++++++++ src/Tgstation.Server.Host/Core/JobManager.cs | 2 +- v4_prototype_TODO.txt | 2 -- 8 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 src/Tgstation.Server.Host/Core/JobException.cs diff --git a/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs b/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs index 6530a0cc4e..a17aa4eece 100644 --- a/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs +++ b/src/Tgstation.Server.Host/Components/Byond/WindowsByondInstaller.cs @@ -134,7 +134,7 @@ namespace Tgstation.Server.Host.Components.Byond cancellationToken.ThrowIfCancellationRequested(); if (exitCode != 0) - throw new Exception(String.Format(CultureInfo.InvariantCulture, "Failed to install included DirectX! Exit code: {0}", exitCode)); + throw new JobException(String.Format(CultureInfo.InvariantCulture, "Failed to install included DirectX! Exit code: {0}", exitCode)); installedDirectX = true; } } diff --git a/src/Tgstation.Server.Host/Components/Compiler/DreamMaker.cs b/src/Tgstation.Server.Host/Components/Compiler/DreamMaker.cs index 121d2b2871..c88bc67e91 100644 --- a/src/Tgstation.Server.Host/Components/Compiler/DreamMaker.cs +++ b/src/Tgstation.Server.Host/Components/Compiler/DreamMaker.cs @@ -263,7 +263,7 @@ namespace Tgstation.Server.Host.Components.Compiler lock (this) { if (Status != CompilerStatus.Idle) - throw new Exception("There is already a compile in progress!"); + throw new JobException("There is already a compile in progress!"); Status = CompilerStatus.Copying; } @@ -336,7 +336,7 @@ namespace Tgstation.Server.Host.Components.Compiler logger.LogTrace("Searching for available .dmes..."); var path = (await ioManager.GetFilesWithExtension(dirA, DmeExtension, cancellationToken).ConfigureAwait(false)).FirstOrDefault(); if (path == default) - throw new Exception("Unable to find any .dme!"); + throw new JobException("Unable to find any .dme!"); var dmeWithExtension = ioManager.GetFileName(path); job.DmeName = dmeWithExtension.Substring(0, dmeWithExtension.Length - DmeExtension.Length - 1); } @@ -364,7 +364,7 @@ namespace Tgstation.Server.Host.Components.Compiler { //server never validated or compile failed await eventConsumer.HandleEvent(EventType.CompileFailure, new List { resolvedGameDirectory, exitCode == 0 ? "1" : "0" }, cancellationToken).ConfigureAwait(false); - throw new Exception(exitCode == 0 ? "Validation of the TGS api failed!" : String.Format(CultureInfo.InvariantCulture, "DM exited with a non-zero code: {0}{1}", exitCode, job.Output)); + throw new JobException(exitCode == 0 ? "Validation of the TGS api failed!" : String.Format(CultureInfo.InvariantCulture, "DM exited with a non-zero code: {0}{1}", exitCode, job.Output)); } logger.LogTrace("Running post compile event..."); diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 2879581610..a792cfa9bc 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -35,7 +35,7 @@ namespace Tgstation.Server.Host.Components.Repository public string GitHubRepoName { get; } /// - public bool Tracking => repository.Head.IsTracking; + public bool Tracking => Reference != null && repository.Head.IsTracking; /// public string Head => repository.Head.Tip.Sha; diff --git a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs index 0cfd9abcb0..70aabb3302 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/Watchdog.cs @@ -320,7 +320,7 @@ namespace Tgstation.Server.Host.Components.Watchdog if (dmbBackup == null) //NANI!? //just give up, if THAT compile job is failing then the ActiveServer is gonna crash soon too or already has - throw new Exception("Creating backup DMB provider failed!"); + throw new JobException("Creating backup DMB provider failed!"); monitorState.InactiveServer = await sessionControllerFactory.LaunchNew(ActiveLaunchParameters, dmbBackup, null, false, !monitorState.ActiveServer.IsPrimary, false, cancellationToken).ConfigureAwait(false); usedMostRecentDmb = false; @@ -687,9 +687,9 @@ namespace Tgstation.Server.Host.Components.Watchdog var launch = await controller.LaunchResult.ConfigureAwait(false); if (launch.ExitCode.HasValue) //you killed us ray... - throw new Exception(String.Format(CultureInfo.InvariantCulture, "{1} server failed to start: {0}", launch.ToString(), serverName)); + throw new JobException(String.Format(CultureInfo.InvariantCulture, "{1} server failed to start: {0}", launch.ToString(), serverName)); if (!launch.StartupTime.HasValue) - throw new Exception(String.Format(CultureInfo.InvariantCulture, "{1} server timed out on startup: {0}s", launch.ToString(), ActiveLaunchParameters.StartupTimeout.Value)); + throw new JobException(String.Format(CultureInfo.InvariantCulture, "{1} server timed out on startup: {0}s", launch.ToString(), ActiveLaunchParameters.StartupTimeout.Value)); return launch; } diff --git a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs index e499498481..efb0ec9328 100644 --- a/src/Tgstation.Server.Host/Controllers/RepositoryController.cs +++ b/src/Tgstation.Server.Host/Controllers/RepositoryController.cs @@ -162,7 +162,7 @@ namespace Tgstation.Server.Host.Controllers using (var repos = await repoManager.CloneRepository(new Uri(origin), cloneBranch, currentModel.AccessUser, currentModel.AccessToken, progressReporter, cancellationToken).ConfigureAwait(false)) { if (repos == null) - throw new Exception("Filesystem conflict while cloning repository!"); + throw new JobException("Filesystem conflict while cloning repository!"); var db = serviceProvider.GetRequiredService(); if (await PopulateApi(api, repos, db, Instance, null, null, cancellationToken).ConfigureAwait(false)) await db.Save(cancellationToken).ConfigureAwait(false); @@ -315,7 +315,7 @@ namespace Tgstation.Server.Host.Controllers using (var repo = await instanceManager.GetInstance(Instance).RepositoryManager.LoadRepository(ct).ConfigureAwait(false)) { if (repo == null) - throw new InvalidOperationException("Repository could not be loaded!"); + throw new JobException("Repository could not be loaded!"); var modelHasShaOrReference = model.CheckoutSha != null || model.Reference != null; @@ -323,7 +323,7 @@ namespace Tgstation.Server.Host.Controllers var startSha = repo.Head; if (newTestMerges && !repo.IsGitHubRepository) - throw new InvalidOperationException("Cannot test merge on a non GitHub based repository!"); + throw new JobException("Cannot test merge on a non GitHub based repository!"); var committerName = currentModel.ShowTestMergeCommitters.Value ? AuthenticationContext.User.Name : currentModel.CommitterName; @@ -357,15 +357,15 @@ namespace Tgstation.Server.Host.Controllers //fetch/pull if (model.UpdateFromOrigin == true) { - if (!repo.Tracking && model.Reference == null) - throw new InvalidOperationException("Not on an updatable reference!"); + if (!repo.Tracking) + throw new JobException("Not on an updatable reference!"); await repo.FetchOrigin(currentModel.AccessUser, currentModel.AccessToken, x => progressReporter(x / numFetches), ct).ConfigureAwait(false); doneFetches = 1; if (!modelHasShaOrReference) { var fastForward = await repo.MergeOrigin(committerName, currentModel.CommitterEmail, ct).ConfigureAwait(false); if (!fastForward.HasValue) - throw new InvalidOperationException("Merge conflict occurred during origin update!"); + throw new JobException("Merge conflict occurred during origin update!"); await UpdateRevInfo().ConfigureAwait(false); if (fastForward.Value) { @@ -388,7 +388,7 @@ namespace Tgstation.Server.Host.Controllers if (model.UpdateFromOrigin == true && model.Reference != null) { if (!repo.Tracking) - throw new InvalidOperationException("Checked out reference does not track a remote object!"); + throw new JobException("Checked out reference does not track a remote object!"); await repo.ResetToOrigin(ct).ConfigureAwait(false); await repo.Sychronize(currentModel.AccessUser, currentModel.AccessToken, true, ct).ConfigureAwait(false); await LoadRevisionInformation(repo, databaseContext, attachedInstance, null, x => lastRevisionInfo = x, ct).ConfigureAwait(false); diff --git a/src/Tgstation.Server.Host/Core/JobException.cs b/src/Tgstation.Server.Host/Core/JobException.cs new file mode 100644 index 0000000000..88877c9548 --- /dev/null +++ b/src/Tgstation.Server.Host/Core/JobException.cs @@ -0,0 +1,34 @@ +using System; + +namespace Tgstation.Server.Host +{ + /// + /// Operation exceptions thrown from the context of a + /// + public sealed class JobException : Exception + { + /// + /// Construct a + /// + public JobException() + { + } + + /// + /// Construct a with a + /// + /// The message for the + public JobException(string message) : base(message) + { + } + + /// + /// Construct a with a and + /// + /// The message for the + /// The inner for the nase + public JobException(string message, Exception innerException) : base(message, innerException) + { + } + } +} diff --git a/src/Tgstation.Server.Host/Core/JobManager.cs b/src/Tgstation.Server.Host/Core/JobManager.cs index e4e13aa4d9..4a9b60419f 100644 --- a/src/Tgstation.Server.Host/Core/JobManager.cs +++ b/src/Tgstation.Server.Host/Core/JobManager.cs @@ -95,7 +95,7 @@ namespace Tgstation.Server.Host.Core catch (Exception e) { logger.LogDebug("Job {0} exited with error! Exception: {1}", job.Id, e); - job.ExceptionDetails = e.ToString(); + job.ExceptionDetails = e is JobException ? e.Message : e.ToString(); } job.StoppedAt = DateTimeOffset.Now; await databaseContext.Save(default).ConfigureAwait(false); diff --git a/v4_prototype_TODO.txt b/v4_prototype_TODO.txt index b2ea1bbeca..572d75b123 100644 --- a/v4_prototype_TODO.txt +++ b/v4_prototype_TODO.txt @@ -1,5 +1,3 @@ Verify the byond cache folder location on linux Test watchdog - -Add a JobException type that the job manager will just print the message of. Replace throw new Exception()s with it