Browse Source

Return to previous exception handle implementation

crobibero 5 years ago
parent
commit
3ef8448a51

+ 0 - 3
Jellyfin.Api/BaseJellyfinApiController.cs

@@ -1,5 +1,3 @@
-using Jellyfin.Api.Models.ExceptionDtos;
-using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 
 namespace Jellyfin.Api
@@ -9,7 +7,6 @@ namespace Jellyfin.Api
     /// </summary>
     [ApiController]
     [Route("[controller]")]
-    [ProducesResponseType(typeof(ExceptionDto), StatusCodes.Status500InternalServerError)]
     public class BaseJellyfinApiController : ControllerBase
     {
     }

+ 0 - 14
Jellyfin.Api/Models/ExceptionDtos/ExceptionDto.cs

@@ -1,14 +0,0 @@
-namespace Jellyfin.Api.Models.ExceptionDtos
-{
-    /// <summary>
-    /// Exception Dto.
-    /// Used for graceful handling of API exceptions.
-    /// </summary>
-    public class ExceptionDto
-    {
-        /// <summary>
-        /// Gets or sets exception message.
-        /// </summary>
-        public string Message { get; set; }
-    }
-}

+ 73 - 13
Jellyfin.Server/Middleware/ExceptionMiddleware.cs

@@ -1,7 +1,9 @@
 using System;
-using System.Text.Json;
+using System.IO;
 using System.Threading.Tasks;
-using Jellyfin.Api.Models.ExceptionDtos;
+using MediaBrowser.Common.Extensions;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Net;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 
@@ -14,17 +16,22 @@ namespace Jellyfin.Server.Middleware
     {
         private readonly RequestDelegate _next;
         private readonly ILogger<ExceptionMiddleware> _logger;
+        private readonly IServerConfigurationManager _configuration;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ExceptionMiddleware"/> class.
         /// </summary>
         /// <param name="next">Next request delegate.</param>
         /// <param name="loggerFactory">Instance of the <see cref="ILoggerFactory"/> interface.</param>
-        public ExceptionMiddleware(RequestDelegate next, ILoggerFactory loggerFactory)
+        /// <param name="serverConfigurationManager">Instance of the <see cref="IServerConfigurationManager"/> interface.</param>
+        public ExceptionMiddleware(
+            RequestDelegate next,
+            ILoggerFactory loggerFactory,
+            IServerConfigurationManager serverConfigurationManager)
         {
-            _next = next ?? throw new ArgumentNullException(nameof(next));
-            _logger = loggerFactory.CreateLogger<ExceptionMiddleware>() ??
-                      throw new ArgumentNullException(nameof(loggerFactory));
+            _next = next;
+            _logger = loggerFactory.CreateLogger<ExceptionMiddleware>();
+            _configuration = serverConfigurationManager;
         }
 
         /// <summary>
@@ -46,15 +53,68 @@ namespace Jellyfin.Server.Middleware
                     throw;
                 }
 
-                var exceptionBody = new ExceptionDto { Message = ex.Message };
-                var exceptionJson = JsonSerializer.Serialize(exceptionBody);
+                ex = GetActualException(ex);
+                _logger.LogError(ex, "Error processing request: {0}", ex.Message);
+                context.Response.StatusCode = GetStatusCode(ex);
+                context.Response.ContentType = "text/plain";
 
-                context.Response.Clear();
-                context.Response.StatusCode = StatusCodes.Status500InternalServerError;
-                // TODO switch between PascalCase and camelCase
-                context.Response.ContentType = "application/json";
-                await context.Response.WriteAsync(exceptionJson).ConfigureAwait(false);
+                var errorContent = NormalizeExceptionMessage(ex.Message);
+                await context.Response.WriteAsync(errorContent).ConfigureAwait(false);
             }
         }
+
+        private static Exception GetActualException(Exception ex)
+        {
+            if (ex is AggregateException agg)
+            {
+                var inner = agg.InnerException;
+                if (inner != null)
+                {
+                    return GetActualException(inner);
+                }
+
+                var inners = agg.InnerExceptions;
+                if (inners.Count > 0)
+                {
+                    return GetActualException(inners[0]);
+                }
+            }
+
+            return ex;
+        }
+
+        private static int GetStatusCode(Exception ex)
+        {
+            switch (ex)
+            {
+                case ArgumentException _: return StatusCodes.Status400BadRequest;
+                case SecurityException _: return StatusCodes.Status401Unauthorized;
+                case DirectoryNotFoundException _:
+                case FileNotFoundException _:
+                case ResourceNotFoundException _: return StatusCodes.Status404NotFound;
+                case MethodNotAllowedException _: return StatusCodes.Status405MethodNotAllowed;
+                default: return StatusCodes.Status500InternalServerError;
+            }
+        }
+
+        private string NormalizeExceptionMessage(string msg)
+        {
+            if (msg == null)
+            {
+                return string.Empty;
+            }
+
+            // Strip any information we don't want to reveal
+            msg = msg.Replace(
+                _configuration.ApplicationPaths.ProgramSystemPath,
+                string.Empty,
+                StringComparison.OrdinalIgnoreCase);
+            msg = msg.Replace(
+                _configuration.ApplicationPaths.ProgramDataPath,
+                string.Empty,
+                StringComparison.OrdinalIgnoreCase);
+
+            return msg;
+        }
     }
 }