From b53562a66735bc66233bb5d6c2d07e34a4f6f3fb Mon Sep 17 00:00:00 2001
From: Jordan Brown <Cyberboss@users.noreply.github.com>
Date: Thu, 9 Jan 2020 20:29:14 -0500
Subject: [PATCH] Remove developer exception page in production

- Replace it with an ErrorMessage payload
- Adjust the client to account for this
---
 src/Tgstation.Server.Client/ApiClient.cs      |  6 +++-
 .../ServerErrorException.cs                   | 17 +++------
 src/Tgstation.Server.Host/Core/Application.cs |  5 ++-
 .../Core/ApplicationBuilderExtensions.cs      | 35 +++++++++++++++++++
 4 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/src/Tgstation.Server.Client/ApiClient.cs b/src/Tgstation.Server.Client/ApiClient.cs
index 3d01a67871..b6ae18e747 100644
--- a/src/Tgstation.Server.Client/ApiClient.cs
+++ b/src/Tgstation.Server.Client/ApiClient.cs
@@ -112,7 +112,11 @@ namespace Tgstation.Server.Client
 					throw new MethodNotSupportedException();
 				case HttpStatusCode.InternalServerError:
 					// response json is html
-					throw new ServerErrorException(json);
+					throw new ServerErrorException(errorMessage ?? new ErrorMessage
+					{
+						Message = "An internal server error occurred!",
+						SeverApiVersion = null
+					}, response.StatusCode);
 				case (HttpStatusCode)429:
 					// rate limited
 					response.Headers.TryGetValues("Retry-After", out var values);
diff --git a/src/Tgstation.Server.Client/ServerErrorException.cs b/src/Tgstation.Server.Client/ServerErrorException.cs
index 940f3513a7..24794c8027 100644
--- a/src/Tgstation.Server.Client/ServerErrorException.cs
+++ b/src/Tgstation.Server.Client/ServerErrorException.cs
@@ -9,27 +9,18 @@ namespace Tgstation.Server.Client
 	/// </summary>
 	public sealed class ServerErrorException : ClientException
 	{
-		/// <summary>
-		/// The raw HTML of the error
-		/// </summary>
-		public string Html { get; }
-
 		/// <summary>
 		/// Construct an <see cref="ServerErrorException"/>
 		/// </summary>
 		public ServerErrorException() { }
 
 		/// <summary>
-		/// Construct an <see cref="ServerErrorException"/> with <paramref name="html"/>
+		/// Construct an <see cref="ServerErrorException"/> with a given <paramref name="errorMessage"/>
 		/// </summary>
-		/// <param name="html">The raw HTML response of the <see cref="ServerErrorException"/></param>
-		public ServerErrorException(string html) : base(new ErrorMessage
+		/// <param name="errorMessage">The <see cref="ErrorMessage"/> for the <see cref="ClientException"/></param>
+		/// <param name="statusCode">The <see cref="HttpStatusCode"/> for the <see cref="ClientException"/></param>
+		public ServerErrorException(ErrorMessage errorMessage, HttpStatusCode statusCode) : base(errorMessage, statusCode)
 		{
-			Message = "An internal server error occurred!",
-			SeverApiVersion = null
-		}, HttpStatusCode.InternalServerError)
-		{
-			Html = html;
 		}
 
 		/// <summary>
diff --git a/src/Tgstation.Server.Host/Core/Application.cs b/src/Tgstation.Server.Host/Core/Application.cs
index 4f5438ca5f..61e70fe05d 100644
--- a/src/Tgstation.Server.Host/Core/Application.cs
+++ b/src/Tgstation.Server.Host/Core/Application.cs
@@ -351,9 +351,12 @@ namespace Tgstation.Server.Host.Core
 				ChangeToken.OnChange(configuration.GetReloadToken, () => serverControl.Restart());
 
 			// setup the HTTP request pipeline
+			// Final point where we wrap exceptions in a 500 (ErrorMessage) response
+			applicationBuilder.UseServerErrorHandling();
 
 			// should anything after this throw an exception, catch it and display a detailed html page
-			applicationBuilder.UseDeveloperExceptionPage(); // it is not worth it to limit this, you should only ever get it if you're an authorized user
+			if(hostingEnvironment.IsDevelopment())
+				applicationBuilder.UseDeveloperExceptionPage(); // it is not worth it to limit this, you should only ever get it if you're an authorized user
 
 			// suppress OperationCancelledExceptions, they are just aborted HTTP requests
 			applicationBuilder.UseCancelledRequestSuppression();
diff --git a/src/Tgstation.Server.Host/Core/ApplicationBuilderExtensions.cs b/src/Tgstation.Server.Host/Core/ApplicationBuilderExtensions.cs
index 971d018f73..a9290b8c01 100644
--- a/src/Tgstation.Server.Host/Core/ApplicationBuilderExtensions.cs
+++ b/src/Tgstation.Server.Host/Core/ApplicationBuilderExtensions.cs
@@ -6,6 +6,7 @@ using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using System;
 using System.Globalization;
+using System.Net;
 using Tgstation.Server.Api.Models;
 
 namespace Tgstation.Server.Host.Core
@@ -69,5 +70,39 @@ namespace Tgstation.Server.Host.Core
 				}
 			});
 		}
+
+		/// <summary>
+		/// Suppress all in flight exceptions with error 500.
+		/// </summary>
+		/// <param name="applicationBuilder">The <see cref="IApplicationBuilder"/> to configure</param>
+		public static void UseServerErrorHandling(this IApplicationBuilder applicationBuilder)
+		{
+			if (applicationBuilder == null)
+				throw new ArgumentNullException(nameof(applicationBuilder));
+			applicationBuilder.Use(async (context, next) =>
+			{
+				var logger = GetLogger(context);
+				try
+				{
+					await next().ConfigureAwait(false);
+				}
+				catch (Exception e)
+				{
+					logger.LogError("Failed request: {0}", e);
+					await new ObjectResult(
+						new ErrorMessage
+						{
+							Message = $"A unhandled exception has occurred: {e}"
+						})
+					{
+						StatusCode = (int)HttpStatusCode.InternalServerError
+					}
+					.ExecuteResultAsync(new ActionContext
+					{
+						HttpContext = context
+					}).ConfigureAwait(false);
+				}
+			});
+		}
 	}
 }