Improve ErrorHandler and quieten logging

Closes #1301

(cherry picked from commit 5fb6b449507af0b238afe6b699d8f55cc5deb790)

src/Readarr.Api.V1/Author/AuthorController.cs

@@ -95,7 +95,7 @@ public AuthorController(IBroadcastSignalRMessage signalRBroadcaster,
             PutValidator.RuleFor(s => s.Path).IsValidPath();
         }
 
-        public override AuthorResource GetResourceById(int id)
+        protected override AuthorResource GetResourceById(int id)
         {
             var author = _authorService.GetAuthor(id);
             return GetAuthorResource(author);

src/Readarr.Api.V1/BookFiles/BookFileController.cs

@@ -60,7 +60,7 @@ private BookFileResource MapToResource(BookFile bookFile)
             }
         }
 
-        public override BookFileResource GetResourceById(int id)
+        protected override BookFileResource GetResourceById(int id)
         {
             var resource = MapToResource(_mediaFileService.Get(id));
             resource.AudioTags = _metadataTagService.ReadTags((FileInfoBase)new FileInfo(resource.Path));

src/Readarr.Api.V1/Books/BookControllerWithSignalR.cs

@@ -33,7 +33,7 @@ protected BookControllerWithSignalR(IBookService bookService,
             _qualityUpgradableSpecification = qualityUpgradableSpecification;
         }
 
-        public override BookResource GetResourceById(int id)
+        protected override BookResource GetResourceById(int id)
         {
             var book = _bookService.GetBook(id);
             var resource = MapToResource(book, true);

src/Readarr.Api.V1/Commands/CommandController.cs

@@ -41,7 +41,7 @@ public CommandController(IManageCommandQueue commandQueueManager,
             _pendingUpdates = new Dictionary<int, CommandResource>();
         }
 
-        public override CommandResource GetResourceById(int id)
+        protected override CommandResource GetResourceById(int id)
         {
             return _commandQueueManager.Get(id).ToResource();
         }

src/Readarr.Api.V1/Config/ConfigController.cs

@@ -17,7 +17,7 @@ protected ConfigController(IConfigService configService)
             _configService = configService;
         }
 
-        public override TResource GetResourceById(int id)
+        protected override TResource GetResourceById(int id)
         {
             return GetConfig();
         }

src/Readarr.Api.V1/Config/DevelopmentConfigController.cs

@@ -26,7 +26,7 @@ public DevelopmentConfigController(IConfigFileProvider configFileProvider,
             SharedValidator.RuleFor(c => c.MetadataSource).IsValidUrl().When(c => !c.MetadataSource.IsNullOrWhiteSpace());
         }
 
-        public override DevelopmentConfigResource GetResourceById(int id)
+        protected override DevelopmentConfigResource GetResourceById(int id)
         {
             return GetDevelopmentConfig();
         }

src/Readarr.Api.V1/Config/HostConfigController.cs

@@ -78,7 +78,7 @@ private bool IsValidSslCertificate(HostConfigResource resource)
             return cert != null;
         }
 
-        public override HostConfigResource GetResourceById(int id)
+        protected override HostConfigResource GetResourceById(int id)
         {
             return GetHostConfig();
         }

src/Readarr.Api.V1/Config/NamingConfigController.cs

@@ -33,7 +33,7 @@ public NamingConfigController(INamingConfigService namingConfigService,
             SharedValidator.RuleFor(c => c.AuthorFolderFormat).ValidAuthorFolderFormat();
         }
 
-        public override NamingConfigResource GetResourceById(int id)
+        protected override NamingConfigResource GetResourceById(int id)
         {
             return GetNamingConfig();
         }

src/Readarr.Api.V1/CustomFilters/CustomFilterController.cs

@@ -17,7 +17,7 @@ public CustomFilterController(ICustomFilterService customFilterService)
             _customFilterService = customFilterService;
         }
 
-        public override CustomFilterResource GetResourceById(int id)
+        protected override CustomFilterResource GetResourceById(int id)
         {
             return _customFilterService.Get(id).ToResource();
         }

src/Readarr.Api.V1/Health/HealthController.cs

@@ -22,7 +22,7 @@ public HealthController(IBroadcastSignalRMessage signalRBroadcaster, IHealthChec
             _healthCheckService = healthCheckService;
         }
 
-        public override HealthResource GetResourceById(int id)
+        protected override HealthResource GetResourceById(int id)
         {
             throw new NotImplementedException();
         }

src/Readarr.Api.V1/ImportLists/ImportListExclusionController.cs

@@ -24,7 +24,7 @@ public ImportListExclusionController(IImportListExclusionService importListExclu
             SharedValidator.RuleFor(c => c.AuthorName).NotEmpty();
         }
 
-        public override ImportListExclusionResource GetResourceById(int id)
+        protected override ImportListExclusionResource GetResourceById(int id)
         {
             return _importListExclusionService.Get(id).ToResource();
         }

src/Readarr.Api.V1/Indexers/ReleaseModuleBase.cs

@@ -7,7 +7,7 @@ namespace Readarr.Api.V1.Indexers
 {
     public abstract class ReleaseControllerBase : RestController<ReleaseResource>
     {
-        public override ReleaseResource GetResourceById(int id)
+        protected override ReleaseResource GetResourceById(int id)
         {
             throw new NotImplementedException();
         }

src/Readarr.Api.V1/Languages/LanguageController.cs

@@ -10,7 +10,7 @@ namespace Readarr.Api.V1.Languages
     [V1ApiController]
     public class LanguageController : RestController<LanguageResource>
     {
-        public override LanguageResource GetResourceById(int id)
+        protected override LanguageResource GetResourceById(int id)
         {
             var language = (Language)id;
 

src/Readarr.Api.V1/Profiles/Delay/DelayProfileController.cs

@@ -61,7 +61,7 @@ public ActionResult<DelayProfileResource> Update(DelayProfileResource resource)
             return Accepted(model.Id);
         }
 
-        public override DelayProfileResource GetResourceById(int id)
+        protected override DelayProfileResource GetResourceById(int id)
         {
             return _delayProfileService.Get(id).ToResource();
         }

src/Readarr.Api.V1/Profiles/Metadata/MetadataProfileController.cs

@@ -53,7 +53,7 @@ public ActionResult<MetadataProfileResource> Update(MetadataProfileResource reso
             return Accepted(model.Id);
         }
 
-        public override MetadataProfileResource GetResourceById(int id)
+        protected override MetadataProfileResource GetResourceById(int id)
         {
             return _profileService.Get(id).ToResource();
         }

src/Readarr.Api.V1/Profiles/Quality/QualityProfileController.cs

@@ -45,7 +45,7 @@ public ActionResult<QualityProfileResource> Update(QualityProfileResource resour
             return Accepted(model.Id);
         }
 
-        public override QualityProfileResource GetResourceById(int id)
+        protected override QualityProfileResource GetResourceById(int id)
         {
             return _profileService.Get(id).ToResource();
         }

src/Readarr.Api.V1/Profiles/Release/ReleaseProfileController.cs

@@ -41,7 +41,7 @@ public ReleaseProfileController(IReleaseProfileService releaseProfileService, II
             });
         }
 
-        public override ReleaseProfileResource GetResourceById(int id)
+        protected override ReleaseProfileResource GetResourceById(int id)
         {
             return _releaseProfileService.Get(id).ToResource();
         }

src/Readarr.Api.V1/ProviderControllerBase.cs

@@ -32,7 +32,7 @@ protected ProviderControllerBase(IProviderFactory<TProvider, TProviderDefinition
             PostValidator.RuleFor(c => c.Fields).NotNull();
         }
 
-        public override TProviderResource GetResourceById(int id)
+        protected override TProviderResource GetResourceById(int id)
         {
             var definition = _providerFactory.Get(id);
             _providerFactory.SetProviderCharacteristics(definition);

src/Readarr.Api.V1/Qualities/QualityDefinitionController.cs

@@ -26,7 +26,7 @@ public ActionResult<QualityDefinitionResource> Update(QualityDefinitionResource
             return Accepted(model.Id);
         }
 
-        public override QualityDefinitionResource GetResourceById(int id)
+        protected override QualityDefinitionResource GetResourceById(int id)
         {
             return _qualityDefinitionService.GetById(id).ToResource();
         }

src/Readarr.Api.V1/Queue/QueueController.cs

@@ -53,7 +53,7 @@ public QueueController(IBroadcastSignalRMessage broadcastSignalRMessage,
             _qualityComparer = new QualityModelComparer(qualityProfileService.GetDefaultProfile(string.Empty));
         }
 
-        public override QueueResource GetResourceById(int id)
+        protected override QueueResource GetResourceById(int id)
         {
             throw new NotImplementedException();
         }

src/Readarr.Api.V1/Queue/QueueDetailsController.cs

@@ -26,7 +26,7 @@ public QueueDetailsController(IBroadcastSignalRMessage broadcastSignalRMessage,
             _pendingReleaseService = pendingReleaseService;
         }
 
-        public override QueueResource GetResourceById(int id)
+        protected override QueueResource GetResourceById(int id)
         {
             throw new NotImplementedException();
         }

src/Readarr.Api.V1/Queue/QueueStatusController.cs

@@ -30,7 +30,7 @@ public QueueStatusController(IBroadcastSignalRMessage broadcastSignalRMessage, I
             _broadcastDebounce = new Debouncer(BroadcastChange, TimeSpan.FromSeconds(5));
         }
 
-        public override QueueStatusResource GetResourceById(int id)
+        protected override QueueStatusResource GetResourceById(int id)
         {
             throw new NotImplementedException();
         }

src/Readarr.Api.V1/RemotePathMappings/RemotePathMappingController.cs

@@ -34,7 +34,7 @@ public RemotePathMappingController(IRemotePathMappingService remotePathMappingSe
                            .SetValidator(pathExistsValidator);
         }
 
-        public override RemotePathMappingResource GetResourceById(int id)
+        protected override RemotePathMappingResource GetResourceById(int id)
         {
             return _remotePathMappingService.Get(id).ToResource();
         }

src/Readarr.Api.V1/RootFolders/RootFolderController.cs

@@ -92,7 +92,7 @@ private string GetLibraryUri(CalibreSettings settings)
             return HttpUri.CombinePath(HttpRequestBuilder.BuildBaseUrl(settings.UseSsl, settings.Host, settings.Port, settings.UrlBase), settings.Library);
         }
 
-        public override RootFolderResource GetResourceById(int id)
+        protected override RootFolderResource GetResourceById(int id)
         {
             return _rootFolderService.Get(id).ToResource();
         }

src/Readarr.Api.V1/System/Tasks/TaskController.cs

@@ -31,7 +31,7 @@ public List<TaskResource> GetAll()
                                .ToList();
         }
 
-        public override TaskResource GetResourceById(int id)
+        protected override TaskResource GetResourceById(int id)
         {
             var task = _taskManager.GetAll()
                                .SingleOrDefault(t => t.Id == id);

src/Readarr.Api.V1/Tags/TagController.cs

@@ -23,7 +23,7 @@ public TagController(IBroadcastSignalRMessage signalRBroadcaster,
             _tagService = tagService;
         }
 
-        public override TagResource GetResourceById(int id)
+        protected override TagResource GetResourceById(int id)
         {
             return _tagService.GetTag(id).ToResource();
         }

src/Readarr.Api.V1/Tags/TagDetailsController.cs

@@ -16,7 +16,7 @@ public TagDetailsController(ITagService tagService)
             _tagService = tagService;
         }
 
-        public override TagDetailsResource GetResourceById(int id)
+        protected override TagDetailsResource GetResourceById(int id)
         {
             return _tagService.Details(id).ToResource();
         }

src/Readarr.Http/ErrorManagement/ReadarrErrorPipeline.cs

@@ -29,22 +29,17 @@ public async Task HandleException(HttpContext context)
             var exceptionHandlerPathFeature = context.Features.Get<IExceptionHandlerPathFeature>();
             var exception = exceptionHandlerPathFeature?.Error;
 
-            _logger.Warn(exception);
-
             var statusCode = HttpStatusCode.InternalServerError;
             var errorModel = new ErrorModel
             {
-                Message = exception.Message,
-                Description = exception.ToString()
+                Message = exception?.Message,
+                Description = exception?.ToString()
             };
 
             if (exception is ApiException apiException)
             {
                 _logger.Warn(apiException, "API Error:\n{0}", apiException.Message);
 
-                /* var body = RequestStream.FromStream(context.Request.Body).AsString();
-                 _logger.Trace("Request body:\n{0}", body);*/
-
                 errorModel = new ErrorModel(apiException);
                 statusCode = apiException.StatusCode;
             }
@@ -59,30 +54,14 @@ public async Task HandleException(HttpContext context)
             }
             else if (exception is NzbDroneClientException clientException)
             {
-                errorModel = new ErrorModel
-                {
-                    Message = exception.Message,
-                    Description = exception.ToString()
-                };
                 statusCode = clientException.StatusCode;
             }
-            else if (exception is ModelNotFoundException notFoundException)
+            else if (exception is ModelNotFoundException)
             {
-                errorModel = new ErrorModel
-                {
-                    Message = exception.Message,
-                    Description = exception.ToString()
-                };
                 statusCode = HttpStatusCode.NotFound;
             }
-            else if (exception is ModelConflictException conflictException)
+            else if (exception is ModelConflictException)
             {
-                _logger.Error(exception, "DB error");
-                errorModel = new ErrorModel
-                {
-                    Message = exception.Message,
-                    Description = exception.ToString()
-                };
                 statusCode = HttpStatusCode.Conflict;
             }
             else if (exception is SQLiteException sqLiteException)
@@ -91,18 +70,16 @@ public async Task HandleException(HttpContext context)
                 {
                     if (sqLiteException.Message.Contains("constraint failed"))
                     {
-                        errorModel = new ErrorModel
-                        {
-                            Message = exception.Message,
-                        };
                         statusCode = HttpStatusCode.Conflict;
                     }
                 }
 
                 _logger.Error(sqLiteException, "[{0} {1}]", context.Request.Method, context.Request.Path);
             }
-
-            _logger.Fatal(exception, "Request Failed. {0} {1}", context.Request.Method, context.Request.Path);
+            else
+            {
+                _logger.Fatal(exception, "Request Failed. {0} {1}", context.Request.Method, context.Request.Path);
+            }
 
             await errorModel.WriteToResponse(response, statusCode);
         }

src/Readarr.Http/REST/RestController.cs

@@ -39,7 +39,19 @@ protected RestController()
         }
 
         [RestGetById]
-        public abstract TResource GetResourceById(int id);
+        public ActionResult<TResource> GetResourceByIdWithErrorHandler(int id)
+        {
+            try
+            {
+                return GetResourceById(id);
+            }
+            catch (ModelNotFoundException)
+            {
+                return NotFound();
+            }
+        }
+
+        protected abstract TResource GetResourceById(int id);
 
         public override void OnActionExecuting(ActionExecutingContext context)
         {
@@ -73,19 +85,6 @@ public override void OnActionExecuting(ActionExecutingContext context)
             base.OnActionExecuting(context);
         }
 
-        public override void OnActionExecuted(ActionExecutedContext context)
-        {
-            var descriptor = context.ActionDescriptor as ControllerActionDescriptor;
-
-            var attributes = descriptor.MethodInfo.CustomAttributes;
-
-            if (context.Exception?.GetType() == typeof(ModelNotFoundException) &&
-                attributes.Any(x => x.AttributeType == typeof(RestGetByIdAttribute)))
-            {
-                context.Result = new NotFoundResult();
-            }
-        }
-
         protected void ValidateResource(TResource resource, bool skipValidate = false, bool skipSharedValidate = false)
         {
             if (resource == null)
@@ -118,13 +117,13 @@ protected void ValidateResource(TResource resource, bool skipValidate = false, b
         protected ActionResult<TResource> Accepted(int id)
         {
             var result = GetResourceById(id);
-            return AcceptedAtAction(nameof(GetResourceById), new { id = id }, result);
+            return AcceptedAtAction(nameof(GetResourceByIdWithErrorHandler), new { id = id }, result);
         }
 
         protected ActionResult<TResource> Created(int id)
         {
             var result = GetResourceById(id);
-            return CreatedAtAction(nameof(GetResourceById), new { id = id }, result);
+            return CreatedAtAction(nameof(GetResourceByIdWithErrorHandler), new { id = id }, result);
         }
     }
 }