From c6ef6ad97966eaac4c6ac42cd1ba0b760bce6bef Mon Sep 17 00:00:00 2001 From: Flaminel Date: Tue, 16 Jun 2026 15:26:56 +0300 Subject: [PATCH] Use ProblemDetails handling for the API (#632) --- .../Auth/AccountControllerOidcTests.cs | 2 +- .../Features/Auth/OidcAuthControllerTests.cs | 4 +- .../DeadTorrentConfigControllerTests.cs | 7 +- .../SeedingRulesControllerTests.cs | 30 +- .../Middleware/GlobalExceptionHandlerTests.cs | 108 +++++ .../TestHelpers/ControllerTestContext.cs | 35 ++ .../Controllers/HealthCheckController.cs | 61 +-- .../Controllers/JobsController.cs | 103 ++--- .../Controllers/StatsController.cs | 24 +- .../Controllers/StatusController.cs | 403 ++++++++---------- .../DependencyInjection/ApiDI.cs | 31 +- .../Extensions/ControllerBaseExtensions.cs | 38 ++ .../Arr/Controllers/ArrConfigController.cs | 29 +- .../Auth/Controllers/AccountController.cs | 66 ++- .../Auth/Controllers/AuthController.cs | 67 +-- .../BlacklistSyncConfigController.cs | 5 - .../DeadTorrentConfigController.cs | 12 +- .../DownloadCleanerConfigController.cs | 9 - .../OrphanedFilesConfigController.cs | 10 +- .../Controllers/SeedingRulesController.cs | 71 +-- .../Controllers/UnlinkedConfigController.cs | 10 +- .../Controllers/DownloadClientController.cs | 26 +- .../Controllers/GeneralConfigController.cs | 5 - .../MalwareBlockerConfigController.cs | 9 - .../NotificationProvidersController.cs | 280 +++--------- .../QueueCleanerConfigController.cs | 11 - .../Controllers/QueueRulesController.cs | 110 +---- .../Controllers/SeekerConfigController.cs | 8 +- .../Middleware/ExceptionMiddleware.cs | 77 ---- .../Middleware/GlobalExceptionHandler.cs | 76 ++++ .../Cleanuparr.Api/Models/ErrorResponse.cs | 17 - .../Exceptions/NotificationTestException.cs | 16 + .../Exceptions/RateLimitException.cs | 20 + .../core/interceptors/error.interceptor.ts | 54 ++- 34 files changed, 790 insertions(+), 1044 deletions(-) create mode 100644 code/backend/Cleanuparr.Api.Tests/Middleware/GlobalExceptionHandlerTests.cs create mode 100644 code/backend/Cleanuparr.Api.Tests/TestHelpers/ControllerTestContext.cs create mode 100644 code/backend/Cleanuparr.Api/Extensions/ControllerBaseExtensions.cs delete mode 100644 code/backend/Cleanuparr.Api/Middleware/ExceptionMiddleware.cs create mode 100644 code/backend/Cleanuparr.Api/Middleware/GlobalExceptionHandler.cs delete mode 100644 code/backend/Cleanuparr.Api/Models/ErrorResponse.cs create mode 100644 code/backend/Cleanuparr.Domain/Exceptions/NotificationTestException.cs create mode 100644 code/backend/Cleanuparr.Domain/Exceptions/RateLimitException.cs diff --git a/code/backend/Cleanuparr.Api.Tests/Features/Auth/AccountControllerOidcTests.cs b/code/backend/Cleanuparr.Api.Tests/Features/Auth/AccountControllerOidcTests.cs index a943c98f..b0085b50 100644 --- a/code/backend/Cleanuparr.Api.Tests/Features/Auth/AccountControllerOidcTests.cs +++ b/code/backend/Cleanuparr.Api.Tests/Features/Auth/AccountControllerOidcTests.cs @@ -87,7 +87,7 @@ public class AccountControllerOidcTests : IClassFixture(); - body.GetProperty("error").GetString().ShouldContain("OIDC is not enabled"); + body.GetProperty("detail").GetString().ShouldContain("OIDC is not enabled"); } [Fact, TestPriority(3)] diff --git a/code/backend/Cleanuparr.Api.Tests/Features/Auth/OidcAuthControllerTests.cs b/code/backend/Cleanuparr.Api.Tests/Features/Auth/OidcAuthControllerTests.cs index 7522b0db..5692d626 100644 --- a/code/backend/Cleanuparr.Api.Tests/Features/Auth/OidcAuthControllerTests.cs +++ b/code/backend/Cleanuparr.Api.Tests/Features/Auth/OidcAuthControllerTests.cs @@ -67,9 +67,11 @@ public class OidcAuthControllerTests : IClassFixture(); - body.GetProperty("error").GetString()!.ShouldContain("OIDC is not enabled"); + body.GetProperty("detail").GetString()!.ShouldContain("OIDC is not enabled"); + body.GetProperty("traceId").GetString().ShouldNotBeNullOrEmpty(); } [Fact, TestPriority(3)] diff --git a/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/DeadTorrentConfigControllerTests.cs b/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/DeadTorrentConfigControllerTests.cs index 42f921fa..6efe73ca 100644 --- a/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/DeadTorrentConfigControllerTests.cs +++ b/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/DeadTorrentConfigControllerTests.cs @@ -2,9 +2,11 @@ using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Requests; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Responses; using Cleanuparr.Api.Features.DownloadCleaner.Controllers; using Cleanuparr.Api.Tests.Features.DownloadCleaner.TestHelpers; +using Cleanuparr.Api.Tests.TestHelpers; using Cleanuparr.Domain.Enums; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Models.Configuration.DownloadCleaner; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; @@ -24,6 +26,7 @@ public class DeadTorrentConfigControllerTests : IDisposable _dataContext = SeedingRulesTestDataFactory.CreateDataContext(); var logger = Substitute.For>(); _controller = new DeadTorrentConfigController(logger, _dataContext); + ControllerTestContext.Attach(_controller); } public void Dispose() @@ -90,7 +93,7 @@ public class DeadTorrentConfigControllerTests : IDisposable var result = await _controller.UpdateDeadTorrentConfig(client.Id, ValidRequest()); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status400BadRequest); } [Fact] @@ -98,6 +101,6 @@ public class DeadTorrentConfigControllerTests : IDisposable { var result = await _controller.UpdateDeadTorrentConfig(Guid.NewGuid(), ValidRequest()); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status404NotFound); } } diff --git a/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/SeedingRulesControllerTests.cs b/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/SeedingRulesControllerTests.cs index 1c8e5d3d..fd7b5b7f 100644 --- a/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/SeedingRulesControllerTests.cs +++ b/code/backend/Cleanuparr.Api.Tests/Features/DownloadCleaner/SeedingRulesControllerTests.cs @@ -2,9 +2,12 @@ using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Requests; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Responses; using Cleanuparr.Api.Features.DownloadCleaner.Controllers; using Cleanuparr.Api.Tests.Features.DownloadCleaner.TestHelpers; +using Cleanuparr.Api.Tests.TestHelpers; using Cleanuparr.Domain.Enums; +using Cleanuparr.Domain.Exceptions; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Models.Configuration.DownloadCleaner; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using NSubstitute; @@ -22,6 +25,7 @@ public class SeedingRulesControllerTests : IDisposable _dataContext = SeedingRulesTestDataFactory.CreateDataContext(); var logger = Substitute.For>(); _controller = new SeedingRulesController(logger, _dataContext); + ControllerTestContext.Attach(_controller); } public void Dispose() @@ -108,7 +112,7 @@ public class SeedingRulesControllerTests : IDisposable public async Task GetSeedingRules_NonExistentClient_ReturnsNotFound() { var result = await _controller.GetSeedingRules(Guid.NewGuid()); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status404NotFound); } [Fact] @@ -214,7 +218,7 @@ public class SeedingRulesControllerTests : IDisposable var request = CreateValidRequest(priority: 1); var result = await _controller.CreateSeedingRule(client.Id, request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status400BadRequest); } [Fact] @@ -223,7 +227,7 @@ public class SeedingRulesControllerTests : IDisposable var request = CreateValidRequest(); var result = await _controller.CreateSeedingRule(Guid.NewGuid(), request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status404NotFound); } [Fact] @@ -232,10 +236,7 @@ public class SeedingRulesControllerTests : IDisposable var client = SeedingRulesTestDataFactory.AddDownloadClient(_dataContext); var request = CreateValidRequest(categories: []); - var result = await _controller.CreateSeedingRule(client.Id, request); - - // Validate() throws ValidationException → caught → BadRequest - result.ShouldBeOfType(); + await Should.ThrowAsync(() => _controller.CreateSeedingRule(client.Id, request)); } [Fact] @@ -335,7 +336,7 @@ public class SeedingRulesControllerTests : IDisposable var request = CreateValidRequest(); var result = await _controller.UpdateSeedingRule(Guid.NewGuid(), request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status404NotFound); } [Fact] @@ -347,8 +348,7 @@ public class SeedingRulesControllerTests : IDisposable // Both maxRatio and maxSeedTime negative → validation failure var request = CreateValidRequest(maxRatio: -1, maxSeedTime: -1); - var result = await _controller.UpdateSeedingRule(rule.Id, request); - result.ShouldBeOfType(); + await Should.ThrowAsync(() => _controller.UpdateSeedingRule(rule.Id, request)); } // ────────────────────────────────────────────────────────────────────── @@ -396,7 +396,7 @@ public class SeedingRulesControllerTests : IDisposable var request = new ReorderSeedingRulesRequest { OrderedIds = [Guid.NewGuid()] }; var result = await _controller.ReorderSeedingRules(Guid.NewGuid(), request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status404NotFound); } [Fact] @@ -409,7 +409,7 @@ public class SeedingRulesControllerTests : IDisposable var request = new ReorderSeedingRulesRequest { OrderedIds = [rule1.Id, rule1.Id] }; var result = await _controller.ReorderSeedingRules(client.Id, request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status400BadRequest); } [Fact] @@ -423,7 +423,7 @@ public class SeedingRulesControllerTests : IDisposable var request = new ReorderSeedingRulesRequest { OrderedIds = [rule1.Id] }; var result = await _controller.ReorderSeedingRules(client.Id, request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status400BadRequest); } [Fact] @@ -436,7 +436,7 @@ public class SeedingRulesControllerTests : IDisposable var request = new ReorderSeedingRulesRequest { OrderedIds = [rule1.Id, Guid.NewGuid()] }; var result = await _controller.ReorderSeedingRules(client.Id, request); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status400BadRequest); } // ────────────────────────────────────────────────────────────────────── @@ -468,6 +468,6 @@ public class SeedingRulesControllerTests : IDisposable public async Task DeleteSeedingRule_NonExistentRule_ReturnsNotFound() { var result = await _controller.DeleteSeedingRule(Guid.NewGuid()); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(StatusCodes.Status404NotFound); } } diff --git a/code/backend/Cleanuparr.Api.Tests/Middleware/GlobalExceptionHandlerTests.cs b/code/backend/Cleanuparr.Api.Tests/Middleware/GlobalExceptionHandlerTests.cs new file mode 100644 index 00000000..e9da1168 --- /dev/null +++ b/code/backend/Cleanuparr.Api.Tests/Middleware/GlobalExceptionHandlerTests.cs @@ -0,0 +1,108 @@ +using Cleanuparr.Api.DependencyInjection; +using Cleanuparr.Api.Middleware; +using Cleanuparr.Domain.Exceptions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Shouldly; + +namespace Cleanuparr.Api.Tests.Middleware; + +public class GlobalExceptionHandlerTests +{ + private static readonly ProblemDetailsFactory ProblemDetailsFactory = BuildProblemDetailsFactory(); + + private static ProblemDetailsFactory BuildProblemDetailsFactory() + { + ServiceCollection services = new(); + services.AddLogging(); + services.AddControllers(); + services.AddCleanuparrProblemDetails(); + return services.BuildServiceProvider().GetRequiredService(); + } + + private static async Task<(bool handled, HttpContext context, ProblemDetails problemDetails)> Handle(Exception exception) + { + IProblemDetailsService problemDetailsService = Substitute.For(); + problemDetailsService + .TryWriteAsync(Arg.Any()) + .Returns(callInfo => ValueTask.FromResult(true)); + + DefaultHttpContext context = new(); + GlobalExceptionHandler handler = new(problemDetailsService, ProblemDetailsFactory, NullLogger.Instance); + + bool handled = await handler.TryHandleAsync(context, exception, CancellationToken.None); + + ProblemDetailsContext captured = (ProblemDetailsContext)problemDetailsService + .ReceivedCalls() + .Single() + .GetArguments()[0]!; + + return (handled, context, captured.ProblemDetails); + } + + [Fact] + public async Task ValidationException_MapsTo400_WithMessageAsDetail() + { + (bool handled, HttpContext context, ProblemDetails problemDetails) = await Handle(new ValidationException("Name is required")); + + handled.ShouldBeTrue(); + context.Response.StatusCode.ShouldBe(StatusCodes.Status400BadRequest); + problemDetails.Status.ShouldBe(StatusCodes.Status400BadRequest); + problemDetails.Title.ShouldBe("Validation failed"); + problemDetails.Detail.ShouldBe("Name is required"); + problemDetails.Type.ShouldNotBeNullOrEmpty(); + problemDetails.Extensions.ShouldContainKey("traceId"); + } + + [Fact] + public async Task NotificationTestException_MapsTo400() + { + (bool handled, HttpContext context, ProblemDetails problemDetails) = await Handle(new NotificationTestException("Test failed: connection refused")); + + handled.ShouldBeTrue(); + context.Response.StatusCode.ShouldBe(StatusCodes.Status400BadRequest); + problemDetails.Status.ShouldBe(StatusCodes.Status400BadRequest); + problemDetails.Title.ShouldBe("Notification test failed"); + problemDetails.Detail.ShouldBe("Test failed: connection refused"); + } + + [Fact] + public async Task RateLimitException_MapsTo429_WithRetryAfterExtensionAndHeader() + { + (bool handled, HttpContext context, ProblemDetails problemDetails) = await Handle(new RateLimitException("Account is locked", 30)); + + handled.ShouldBeTrue(); + context.Response.StatusCode.ShouldBe(StatusCodes.Status429TooManyRequests); + problemDetails.Status.ShouldBe(StatusCodes.Status429TooManyRequests); + problemDetails.Title.ShouldBe("Too many requests"); + problemDetails.Extensions["retryAfterSeconds"].ShouldBe(30); + context.Response.Headers.RetryAfter.ToString().ShouldBe("30"); + } + + [Fact] + public async Task RateLimitException_WithZeroRetry_MapsTo429_WithoutRetryAfter() + { + (bool handled, HttpContext context, ProblemDetails problemDetails) = await Handle(new RateLimitException("Too many pending OIDC flows", 0)); + + handled.ShouldBeTrue(); + context.Response.StatusCode.ShouldBe(StatusCodes.Status429TooManyRequests); + problemDetails.Extensions.ShouldNotContainKey("retryAfterSeconds"); + context.Response.Headers.RetryAfter.ToString().ShouldBeEmpty(); + } + + [Fact] + public async Task UnknownException_MapsTo500_WithGenericDetail_AndDoesNotLeakMessage() + { + (bool handled, HttpContext context, ProblemDetails problemDetails) = await Handle(new InvalidOperationException("internal connection string leaked")); + + handled.ShouldBeTrue(); + context.Response.StatusCode.ShouldBe(StatusCodes.Status500InternalServerError); + problemDetails.Status.ShouldBe(StatusCodes.Status500InternalServerError); + problemDetails.Detail.ShouldBe("An unexpected error occurred"); + problemDetails.Detail.ShouldNotContain("connection string"); + } +} diff --git a/code/backend/Cleanuparr.Api.Tests/TestHelpers/ControllerTestContext.cs b/code/backend/Cleanuparr.Api.Tests/TestHelpers/ControllerTestContext.cs new file mode 100644 index 00000000..b952ba0a --- /dev/null +++ b/code/backend/Cleanuparr.Api.Tests/TestHelpers/ControllerTestContext.cs @@ -0,0 +1,35 @@ +using Cleanuparr.Api.DependencyInjection; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.DependencyInjection; + +namespace Cleanuparr.Api.Tests.TestHelpers; + +/// +/// Attaches a minimal MVC (with a real +/// and ) to a directly-instantiated controller so that +/// this.ProblemResult(...) can build problem-details responses in unit tests. +/// +public static class ControllerTestContext +{ + private static readonly IServiceProvider Services = BuildServices(); + + private static IServiceProvider BuildServices() + { + ServiceCollection services = new(); + services.AddLogging(); + services.AddControllers(); + services.AddCleanuparrProblemDetails(); + return services.BuildServiceProvider(); + } + + public static void Attach(ControllerBase controller) + { + controller.ControllerContext = new ControllerContext + { + HttpContext = new DefaultHttpContext { RequestServices = Services }, + }; + controller.ProblemDetailsFactory = Services.GetRequiredService(); + } +} diff --git a/code/backend/Cleanuparr.Api/Controllers/HealthCheckController.cs b/code/backend/Cleanuparr.Api/Controllers/HealthCheckController.cs index f1eac330..b33f9943 100644 --- a/code/backend/Cleanuparr.Api/Controllers/HealthCheckController.cs +++ b/code/backend/Cleanuparr.Api/Controllers/HealthCheckController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Infrastructure.Health; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -12,17 +13,13 @@ namespace Cleanuparr.Api.Controllers; [Authorize] public class HealthCheckController : ControllerBase { - private readonly ILogger _logger; private readonly IHealthCheckService _healthCheckService; /// /// Initializes a new instance of the class /// - public HealthCheckController( - ILogger logger, - IHealthCheckService healthCheckService) + public HealthCheckController(IHealthCheckService healthCheckService) { - _logger = logger; _healthCheckService = healthCheckService; } @@ -32,16 +29,8 @@ public class HealthCheckController : ControllerBase [HttpGet] public IActionResult GetAllHealth() { - try - { - var healthStatuses = _healthCheckService.GetAllClientHealth(); - return Ok(healthStatuses); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error retrieving client health statuses"); - return StatusCode(500, new { Error = "An error occurred while retrieving client health statuses" }); - } + var healthStatuses = _healthCheckService.GetAllClientHealth(); + return Ok(healthStatuses); } /// @@ -50,21 +39,13 @@ public class HealthCheckController : ControllerBase [HttpGet("{id:guid}")] public IActionResult GetClientHealth(Guid id) { - try + var healthStatus = _healthCheckService.GetClientHealth(id); + if (healthStatus == null) { - var healthStatus = _healthCheckService.GetClientHealth(id); - if (healthStatus == null) - { - return NotFound(new { Message = $"Health status for client with ID '{id}' not found" }); - } + return this.ProblemResult(StatusCodes.Status404NotFound, $"Health status for client with ID '{id}' not found"); + } - return Ok(healthStatus); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error retrieving health status for client {id}", id); - return StatusCode(500, new { Error = "An error occurred while retrieving the client health status" }); - } + return Ok(healthStatus); } /// @@ -73,16 +54,8 @@ public class HealthCheckController : ControllerBase [HttpPost("check")] public async Task CheckAllHealth() { - try - { - var results = await _healthCheckService.CheckAllClientsHealthAsync(); - return Ok(results); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error checking health for all clients"); - return StatusCode(500, new { Error = "An error occurred while checking client health" }); - } + var results = await _healthCheckService.CheckAllClientsHealthAsync(); + return Ok(results); } /// @@ -91,15 +64,7 @@ public class HealthCheckController : ControllerBase [HttpPost("check/{id:guid}")] public async Task CheckClientHealth(Guid id) { - try - { - var result = await _healthCheckService.CheckClientHealthAsync(id); - return Ok(result); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error checking health for client {id}", id); - return StatusCode(500, new { Error = "An error occurred while checking client health" }); - } + var result = await _healthCheckService.CheckClientHealthAsync(id); + return Ok(result); } } diff --git a/code/backend/Cleanuparr.Api/Controllers/JobsController.cs b/code/backend/Cleanuparr.Api/Controllers/JobsController.cs index 1bbf477e..404d045b 100644 --- a/code/backend/Cleanuparr.Api/Controllers/JobsController.cs +++ b/code/backend/Cleanuparr.Api/Controllers/JobsController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Models; using Cleanuparr.Domain.Enums; using Cleanuparr.Infrastructure.Models; @@ -13,47 +14,29 @@ namespace Cleanuparr.Api.Controllers; public class JobsController : ControllerBase { private readonly IJobManagementService _jobManagementService; - private readonly ILogger _logger; - public JobsController(IJobManagementService jobManagementService, ILogger logger) + public JobsController(IJobManagementService jobManagementService) { _jobManagementService = jobManagementService; - _logger = logger; } [HttpGet] public async Task GetAllJobs() { - try - { - var result = await _jobManagementService.GetAllJobs(); - return Ok(result); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error getting all jobs"); - return StatusCode(500, "An error occurred while retrieving jobs"); - } + var result = await _jobManagementService.GetAllJobs(); + return Ok(result); } [HttpGet("{jobType}")] public async Task GetJob(JobType jobType) { - try + var jobInfo = await _jobManagementService.GetJob(jobType); + + if (jobInfo.Status == "Not Found") { - var jobInfo = await _jobManagementService.GetJob(jobType); - - if (jobInfo.Status == "Not Found") - { - return NotFound($"Job '{jobType}' not found"); - } - return Ok(jobInfo); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error getting job {jobType}", jobType); - return StatusCode(500, $"An error occurred while retrieving job '{jobType}'"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Job '{jobType}' not found"); } + return Ok(jobInfo); } [HttpPost("{jobType}/start")] @@ -61,27 +44,19 @@ public class JobsController : ControllerBase { if (jobType == JobType.Seeker) { - return BadRequest("The Seeker job cannot be manually controlled"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "The Seeker job cannot be manually controlled"); } - try + // Get the schedule from the request body if provided + JobSchedule jobSchedule = scheduleRequest.Schedule; + + var result = await _jobManagementService.StartJob(jobType, jobSchedule); + + if (!result) { - // Get the schedule from the request body if provided - JobSchedule jobSchedule = scheduleRequest.Schedule; - - var result = await _jobManagementService.StartJob(jobType, jobSchedule); - - if (!result) - { - return BadRequest($"Failed to start job '{jobType}'"); - } - return Ok(new { Message = $"Job '{jobType}' started successfully" }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error starting job {jobType}", jobType); - return StatusCode(500, $"An error occurred while starting job '{jobType}'"); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Failed to start job '{jobType}'"); } + return Ok(new { Message = $"Job '{jobType}' started successfully" }); } [HttpPost("{jobType}/trigger")] @@ -89,24 +64,16 @@ public class JobsController : ControllerBase { if (jobType == JobType.Seeker) { - return BadRequest("The Seeker job cannot be manually triggered"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "The Seeker job cannot be manually triggered"); } - try + var result = await _jobManagementService.TriggerJobOnce(jobType); + + if (!result) { - var result = await _jobManagementService.TriggerJobOnce(jobType); - - if (!result) - { - return BadRequest($"Failed to trigger job '{jobType}' - job may not exist or be configured"); - } - return Ok(new { Message = $"Job '{jobType}' triggered successfully for one-time execution" }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error triggering job {jobType}", jobType); - return StatusCode(500, $"An error occurred while triggering job '{jobType}'"); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Failed to trigger job '{jobType}' - job may not exist or be configured"); } + return Ok(new { Message = $"Job '{jobType}' triggered successfully for one-time execution" }); } [HttpPut("{jobType}/schedule")] @@ -114,28 +81,20 @@ public class JobsController : ControllerBase { if (jobType == JobType.Seeker) { - return BadRequest("The Seeker job schedule cannot be manually modified"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "The Seeker job schedule cannot be manually modified"); } if (scheduleRequest?.Schedule == null) { - return BadRequest("Schedule is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Schedule is required"); } - try + var result = await _jobManagementService.UpdateJobSchedule(jobType, scheduleRequest.Schedule); + + if (!result) { - var result = await _jobManagementService.UpdateJobSchedule(jobType, scheduleRequest.Schedule); - - if (!result) - { - return BadRequest($"Failed to update schedule for job '{jobType}'"); - } - return Ok(new { Message = $"Job '{jobType}' schedule updated successfully" }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error updating job {jobType} schedule", jobType); - return StatusCode(500, $"An error occurred while updating schedule for job '{jobType}'"); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Failed to update schedule for job '{jobType}'"); } + return Ok(new { Message = $"Job '{jobType}' schedule updated successfully" }); } } diff --git a/code/backend/Cleanuparr.Api/Controllers/StatsController.cs b/code/backend/Cleanuparr.Api/Controllers/StatsController.cs index 133349c6..f9d06578 100644 --- a/code/backend/Cleanuparr.Api/Controllers/StatsController.cs +++ b/code/backend/Cleanuparr.Api/Controllers/StatsController.cs @@ -12,14 +12,10 @@ namespace Cleanuparr.Api.Controllers; [Authorize] public class StatsController : ControllerBase { - private readonly ILogger _logger; private readonly IStatsService _statsService; - public StatsController( - ILogger logger, - IStatsService statsService) + public StatsController(IStatsService statsService) { - _logger = logger; _statsService = statsService; } @@ -35,19 +31,11 @@ public class StatsController : ControllerBase [FromQuery] int includeEvents = 0, [FromQuery] int includeStrikes = 0) { - try - { - hours = Math.Clamp(hours, 1, 720); - includeEvents = Math.Clamp(includeEvents, 0, 100); - includeStrikes = Math.Clamp(includeStrikes, 0, 100); + hours = Math.Clamp(hours, 1, 720); + includeEvents = Math.Clamp(includeEvents, 0, 100); + includeStrikes = Math.Clamp(includeStrikes, 0, 100); - var stats = await _statsService.GetStatsAsync(hours, includeEvents, includeStrikes); - return Ok(stats); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error retrieving stats"); - return StatusCode(500, new { Error = "An error occurred while retrieving stats" }); - } + var stats = await _statsService.GetStatsAsync(hours, includeEvents, includeStrikes); + return Ok(stats); } } diff --git a/code/backend/Cleanuparr.Api/Controllers/StatusController.cs b/code/backend/Cleanuparr.Api/Controllers/StatusController.cs index 4a61121d..83203db4 100644 --- a/code/backend/Cleanuparr.Api/Controllers/StatusController.cs +++ b/code/backend/Cleanuparr.Api/Controllers/StatusController.cs @@ -13,16 +13,13 @@ namespace Cleanuparr.Api.Controllers; [Authorize] public class StatusController : ControllerBase { - private readonly ILogger _logger; private readonly DataContext _dataContext; private readonly IArrClientFactory _arrClientFactory; public StatusController( - ILogger logger, DataContext dataContext, IArrClientFactory arrClientFactory) { - _logger = logger; _dataContext = dataContext; _arrClientFactory = arrClientFactory; } @@ -30,247 +27,219 @@ public class StatusController : ControllerBase [HttpGet] public async Task GetSystemStatus() { - try - { - var process = Process.GetCurrentProcess(); - - // Get configuration - var downloadClients = await _dataContext.DownloadClients - .AsNoTracking() - .ToListAsync(); - var sonarrConfig = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .AsNoTracking() - .FirstAsync(x => x.Type == InstanceType.Sonarr); - var radarrConfig = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .AsNoTracking() - .FirstAsync(x => x.Type == InstanceType.Radarr); - var lidarrConfig = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .AsNoTracking() - .FirstAsync(x => x.Type == InstanceType.Lidarr); - var readarrConfig = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .AsNoTracking() - .FirstAsync(x => x.Type == InstanceType.Readarr); - - var status = new - { - Application = new - { - Version = GetType().Assembly.GetName().Version?.ToString() ?? "Unknown", - process.StartTime, - UpTime = DateTimeOffset.UtcNow - process.StartTime.ToUniversalTime(), - MemoryUsageMB = Math.Round(process.WorkingSet64 / 1024.0 / 1024.0, 2), - ProcessorTime = process.TotalProcessorTime - }, - DownloadClient = new - { - // TODO - }, - MediaManagers = new - { - Sonarr = new - { - InstanceCount = sonarrConfig.Instances.Count - }, - Radarr = new - { - InstanceCount = radarrConfig.Instances.Count - }, - Lidarr = new - { - InstanceCount = lidarrConfig.Instances.Count - }, - Readarr = new - { - InstanceCount = readarrConfig.Instances.Count - } - } - }; + using var process = Process.GetCurrentProcess(); - return Ok(status); - } - catch (Exception ex) + // Get configuration + var sonarrConfig = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .AsNoTracking() + .FirstAsync(x => x.Type == InstanceType.Sonarr); + var radarrConfig = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .AsNoTracking() + .FirstAsync(x => x.Type == InstanceType.Radarr); + var lidarrConfig = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .AsNoTracking() + .FirstAsync(x => x.Type == InstanceType.Lidarr); + var readarrConfig = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .AsNoTracking() + .FirstAsync(x => x.Type == InstanceType.Readarr); + + var status = new { - _logger.LogError(ex, "Error retrieving system status"); - return StatusCode(500, "An error occurred while retrieving system status"); - } + Application = new + { + Version = GetType().Assembly.GetName().Version?.ToString() ?? "Unknown", + process.StartTime, + UpTime = DateTimeOffset.UtcNow - process.StartTime.ToUniversalTime(), + MemoryUsageMB = Math.Round(process.WorkingSet64 / 1024.0 / 1024.0, 2), + ProcessorTime = process.TotalProcessorTime + }, + DownloadClient = new + { + // TODO + }, + MediaManagers = new + { + Sonarr = new + { + InstanceCount = sonarrConfig.Instances.Count + }, + Radarr = new + { + InstanceCount = radarrConfig.Instances.Count + }, + Lidarr = new + { + InstanceCount = lidarrConfig.Instances.Count + }, + Readarr = new + { + InstanceCount = readarrConfig.Instances.Count + } + } + }; + + return Ok(status); } [HttpGet("download-client")] public async Task GetDownloadClientStatus() { - try + var downloadClients = await _dataContext.DownloadClients + .AsNoTracking() + .ToListAsync(); + var result = new Dictionary(); + + // Check for configured clients + if (downloadClients.Count > 0) { - var downloadClients = await _dataContext.DownloadClients - .AsNoTracking() - .ToListAsync(); - var result = new Dictionary(); - - // Check for configured clients - if (downloadClients.Count > 0) + var clientsStatus = new List(); + foreach (var client in downloadClients) { - var clientsStatus = new List(); - foreach (var client in downloadClients) + clientsStatus.Add(new { - clientsStatus.Add(new - { - client.Id, - client.Name, - Type = client.TypeName, - client.Host, - client.Enabled, - IsConnected = client.Enabled, // We can't check connection status without implementing test methods - }); - } - - result["Clients"] = clientsStatus; + client.Id, + client.Name, + Type = client.TypeName, + client.Host, + client.Enabled, + IsConnected = client.Enabled, // We can't check connection status without implementing test methods + }); } - return Ok(result); - } - catch (Exception ex) - { - _logger.LogError(ex, "Error retrieving download client status"); - return StatusCode(500, "An error occurred while retrieving download client status"); + result["Clients"] = clientsStatus; } + + return Ok(result); } [HttpGet("arrs")] public async Task GetMediaManagersStatus() { - try + var status = new Dictionary(); + + // Get configurations + var enabledSonarrInstances = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .Where(x => x.Type == InstanceType.Sonarr) + .SelectMany(x => x.Instances) + .Where(x => x.Enabled) + .AsNoTracking() + .ToListAsync(); + var enabledRadarrInstances = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .Where(x => x.Type == InstanceType.Radarr) + .SelectMany(x => x.Instances) + .Where(x => x.Enabled) + .AsNoTracking() + .ToListAsync(); + var enabledLidarrInstances = await _dataContext.ArrConfigs + .Include(x => x.Instances) + .Where(x => x.Type == InstanceType.Lidarr) + .SelectMany(x => x.Instances) + .Where(x => x.Enabled) + .AsNoTracking() + .ToListAsync(); + + // Check Sonarr instances + var sonarrStatus = new List(); + + foreach (var instance in enabledSonarrInstances) { - var status = new Dictionary(); - - // Get configurations - var enabledSonarrInstances = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .Where(x => x.Type == InstanceType.Sonarr) - .SelectMany(x => x.Instances) - .Where(x => x.Enabled) - .AsNoTracking() - .ToListAsync(); - var enabledRadarrInstances = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .Where(x => x.Type == InstanceType.Radarr) - .SelectMany(x => x.Instances) - .Where(x => x.Enabled) - .AsNoTracking() - .ToListAsync(); - var enabledLidarrInstances = await _dataContext.ArrConfigs - .Include(x => x.Instances) - .Where(x => x.Type == InstanceType.Lidarr) - .SelectMany(x => x.Instances) - .Where(x => x.Enabled) - .AsNoTracking() - .ToListAsync();; - - - // Check Sonarr instances - var sonarrStatus = new List(); - - foreach (var instance in enabledSonarrInstances) + try { - try + var sonarrClient = _arrClientFactory.GetClient(InstanceType.Sonarr, instance.Version); + await sonarrClient.HealthCheckAsync(instance); + + sonarrStatus.Add(new { - var sonarrClient = _arrClientFactory.GetClient(InstanceType.Sonarr, instance.Version); - await sonarrClient.HealthCheckAsync(instance); - - sonarrStatus.Add(new - { - instance.Name, - instance.Url, - IsConnected = true, - Message = "Successfully connected" - }); - } - catch (Exception ex) - { - sonarrStatus.Add(new - { - instance.Name, - instance.Url, - IsConnected = false, - Message = $"Connection failed: {ex.Message}" - }); - } + instance.Name, + instance.Url, + IsConnected = true, + Message = "Successfully connected" + }); } - - status["Sonarr"] = sonarrStatus; - - // Check Radarr instances - var radarrStatus = new List(); - - foreach (var instance in enabledRadarrInstances) + catch (Exception ex) { - try + sonarrStatus.Add(new { - var radarrClient = _arrClientFactory.GetClient(InstanceType.Radarr, instance.Version); - await radarrClient.HealthCheckAsync(instance); - - radarrStatus.Add(new - { - instance.Name, - instance.Url, - IsConnected = true, - Message = "Successfully connected" - }); - } - catch (Exception ex) - { - radarrStatus.Add(new - { - instance.Name, - instance.Url, - IsConnected = false, - Message = $"Connection failed: {ex.Message}" - }); - } + instance.Name, + instance.Url, + IsConnected = false, + Message = $"Connection failed: {ex.Message}" + }); } - - status["Radarr"] = radarrStatus; - - // Check Lidarr instances - var lidarrStatus = new List(); - - foreach (var instance in enabledLidarrInstances) - { - try - { - var lidarrClient = _arrClientFactory.GetClient(InstanceType.Lidarr, instance.Version); - await lidarrClient.HealthCheckAsync(instance); - - lidarrStatus.Add(new - { - instance.Name, - instance.Url, - IsConnected = true, - Message = "Successfully connected" - }); - } - catch (Exception ex) - { - lidarrStatus.Add(new - { - instance.Name, - instance.Url, - IsConnected = false, - Message = $"Connection failed: {ex.Message}" - }); - } - } - - status["Lidarr"] = lidarrStatus; - - return Ok(status); } - catch (Exception ex) + + status["Sonarr"] = sonarrStatus; + + // Check Radarr instances + var radarrStatus = new List(); + + foreach (var instance in enabledRadarrInstances) { - _logger.LogError(ex, "Error retrieving media managers status"); - return StatusCode(500, "An error occurred while retrieving media managers status"); + try + { + var radarrClient = _arrClientFactory.GetClient(InstanceType.Radarr, instance.Version); + await radarrClient.HealthCheckAsync(instance); + + radarrStatus.Add(new + { + instance.Name, + instance.Url, + IsConnected = true, + Message = "Successfully connected" + }); + } + catch (Exception ex) + { + radarrStatus.Add(new + { + instance.Name, + instance.Url, + IsConnected = false, + Message = $"Connection failed: {ex.Message}" + }); + } } + + status["Radarr"] = radarrStatus; + + // Check Lidarr instances + var lidarrStatus = new List(); + + foreach (var instance in enabledLidarrInstances) + { + try + { + var lidarrClient = _arrClientFactory.GetClient(InstanceType.Lidarr, instance.Version); + await lidarrClient.HealthCheckAsync(instance); + + lidarrStatus.Add(new + { + instance.Name, + instance.Url, + IsConnected = true, + Message = "Successfully connected" + }); + } + catch (Exception ex) + { + lidarrStatus.Add(new + { + instance.Name, + instance.Url, + IsConnected = false, + Message = $"Connection failed: {ex.Message}" + }); + } + } + + status["Lidarr"] = lidarrStatus; + + return Ok(status); } } diff --git a/code/backend/Cleanuparr.Api/DependencyInjection/ApiDI.cs b/code/backend/Cleanuparr.Api/DependencyInjection/ApiDI.cs index c7b2e6c9..e6a7c3f8 100644 --- a/code/backend/Cleanuparr.Api/DependencyInjection/ApiDI.cs +++ b/code/backend/Cleanuparr.Api/DependencyInjection/ApiDI.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; using Cleanuparr.Api.Filters; @@ -55,24 +56,42 @@ public static class ApiDI // Add health status broadcaster services.AddHostedService(); + services.AddCleanuparrProblemDetails(); + services.AddExceptionHandler(); + + return services; + } + + /// + /// Registers RFC 9457 problem-details responses for both the exception handler and + /// [ApiController] model-state validation, attaching a uniform Activity-tied traceId. + /// + public static IServiceCollection AddCleanuparrProblemDetails(this IServiceCollection services) + { + services.AddProblemDetails(options => + { + options.CustomizeProblemDetails = ctx => + ctx.ProblemDetails.Extensions.TryAdd( + "traceId", Activity.Current?.Id ?? ctx.HttpContext.TraceIdentifier); + }); + return services; } public static WebApplication ConfigureApi(this WebApplication app) { - ILogger logger = app.Services.GetRequiredService>(); - + // Map unhandled exceptions to RFC 9457 problem-details responses (GlobalExceptionHandler). + // Registered first so it also covers exceptions thrown by downstream middleware. + app.UseExceptionHandler(); + // Enable compression app.UseResponseCompression(); - + // Serve static files without caching app.UseStaticFiles(new StaticFileOptions { OnPrepareResponse = ctx => NoCacheAttribute.Apply(ctx.Context.Response.Headers) }); - - // Add the global exception handling middleware first - app.UseMiddleware(); // Resolve the real client IP / scheme / host from X-Forwarded-* headers app.UseMiddleware(); diff --git a/code/backend/Cleanuparr.Api/Extensions/ControllerBaseExtensions.cs b/code/backend/Cleanuparr.Api/Extensions/ControllerBaseExtensions.cs new file mode 100644 index 00000000..991c82b0 --- /dev/null +++ b/code/backend/Cleanuparr.Api/Extensions/ControllerBaseExtensions.cs @@ -0,0 +1,38 @@ +using Microsoft.AspNetCore.Mvc; + +namespace Cleanuparr.Api.Extensions; + +public static class ControllerBaseExtensions +{ + /// + /// Builds an RFC 9457 problem-details error response for a direct (non-throwing) controller return. + /// Mirrors the shape produced by so every error + /// response carries the same application/problem+json body and traceId. The + /// traceId extension is added by the shared CustomizeProblemDetails hook inside + /// . + /// + public static ObjectResult ProblemResult( + this ControllerBase controller, + int statusCode, + string detail, + string? title = null, + IReadOnlyDictionary? extensions = null) + { + ProblemDetails problemDetails = controller.ProblemDetailsFactory + .CreateProblemDetails(controller.HttpContext, statusCode: statusCode, title: title, detail: detail); + + if (extensions is not null) + { + foreach (KeyValuePair extension in extensions) + { + problemDetails.Extensions[extension.Key] = extension.Value; + } + } + + return new ObjectResult(problemDetails) + { + StatusCode = statusCode, + ContentTypes = { "application/problem+json" }, + }; + } +} diff --git a/code/backend/Cleanuparr.Api/Features/Arr/Controllers/ArrConfigController.cs b/code/backend/Cleanuparr.Api/Features/Arr/Controllers/ArrConfigController.cs index 48bceb11..6ecb918b 100644 --- a/code/backend/Cleanuparr.Api/Features/Arr/Controllers/ArrConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/Arr/Controllers/ArrConfigController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.Arr.Contracts.Requests; using Cleanuparr.Domain.Enums; using Cleanuparr.Infrastructure.Features.Arr.Dtos; @@ -182,11 +183,6 @@ public sealed class ArrConfigController : ControllerBase return Ok(new { Message = $"{type} configuration updated successfully" }); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save {Type} configuration", type); - throw; - } finally { DataContext.Lock.Release(); @@ -207,11 +203,6 @@ public sealed class ArrConfigController : ControllerBase return CreatedAtAction(GetConfigActionName(type), new { id = instance.Id }, instance.Adapt()); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create {Type} instance", type); - throw; - } finally { DataContext.Lock.Release(); @@ -230,7 +221,7 @@ public sealed class ArrConfigController : ControllerBase var instance = config.Instances.FirstOrDefault(i => i.Id == id); if (instance is null) { - return NotFound($"{type} instance with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"{type} instance with ID {id} not found"); } request.ApplyTo(instance); @@ -239,11 +230,6 @@ public sealed class ArrConfigController : ControllerBase return Ok(instance.Adapt()); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update {Type} instance with ID {Id}", type, id); - throw; - } finally { DataContext.Lock.Release(); @@ -262,7 +248,7 @@ public sealed class ArrConfigController : ControllerBase var instance = config.Instances.FirstOrDefault(i => i.Id == id); if (instance is null) { - return NotFound($"{type} instance with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"{type} instance with ID {id} not found"); } config.Instances.Remove(instance); @@ -270,11 +256,6 @@ public sealed class ArrConfigController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to delete {Type} instance with ID {Id}", type, id); - throw; - } finally { DataContext.Lock.Release(); @@ -295,7 +276,7 @@ public sealed class ArrConfigController : ControllerBase if (existingInstance is null) { - return NotFound($"Instance with ID {request.InstanceId.Value} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Instance with ID {request.InstanceId.Value} not found"); } resolvedApiKey = existingInstance.ApiKey; @@ -310,7 +291,7 @@ public sealed class ArrConfigController : ControllerBase catch (Exception ex) { _logger.LogError(ex, "Failed to test {Type} instance connection", type); - return BadRequest(new { Message = $"Connection failed: {ex.Message}" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Connection failed: {ex.Message}"); } } diff --git a/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AccountController.cs b/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AccountController.cs index 8481debd..cbd6ebdc 100644 --- a/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AccountController.cs +++ b/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AccountController.cs @@ -4,13 +4,13 @@ using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.Auth.Contracts.Requests; using Cleanuparr.Api.Features.Auth.Contracts.Responses; using Cleanuparr.Api.Filters; +using Cleanuparr.Domain.Exceptions; using Cleanuparr.Infrastructure.Features.Auth; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Models.Auth; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; -using ValidationException = Cleanuparr.Domain.Exceptions.ValidationException; namespace Cleanuparr.Api.Features.Auth.Controllers; @@ -67,7 +67,7 @@ public sealed class AccountController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Password changes are disabled while OIDC exclusive mode is active." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Password changes are disabled while OIDC exclusive mode is active."); } var user = await GetCurrentUser(); @@ -78,7 +78,7 @@ public sealed class AccountController : ControllerBase if (!_passwordService.VerifyPassword(request.CurrentPassword, user.PasswordHash)) { - return BadRequest(new { error = "Current password is incorrect" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Current password is incorrect"); } DateTimeOffset now = DateTimeOffset.UtcNow; @@ -115,12 +115,12 @@ public sealed class AccountController : ControllerBase // Verify current credentials if (!_passwordService.VerifyPassword(request.Password, user.PasswordHash)) { - return BadRequest(new { error = "Incorrect password" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Incorrect password"); } if (!_totpService.ValidateCode(user.TotpSecret, request.TotpCode)) { - return BadRequest(new { error = "Invalid 2FA code" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Invalid 2FA code"); } // Generate new TOTP @@ -168,12 +168,12 @@ public sealed class AccountController : ControllerBase if (user.TotpEnabled) { - return Conflict(new { error = "2FA is already enabled" }); + return this.ProblemResult(StatusCodes.Status409Conflict, "2FA is already enabled"); } if (!_passwordService.VerifyPassword(request.Password, user.PasswordHash)) { - return BadRequest(new { error = "Incorrect password" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Incorrect password"); } // Generate new TOTP @@ -221,17 +221,17 @@ public sealed class AccountController : ControllerBase if (user.TotpEnabled) { - return Conflict(new { error = "2FA is already enabled" }); + return this.ProblemResult(StatusCodes.Status409Conflict, "2FA is already enabled"); } if (string.IsNullOrEmpty(user.TotpSecret)) { - return BadRequest(new { error = "Generate 2FA setup first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Generate 2FA setup first"); } if (!_totpService.ValidateCode(user.TotpSecret, request.Code)) { - return BadRequest(new { error = "Invalid verification code" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Invalid verification code"); } user.TotpEnabled = true; @@ -254,17 +254,17 @@ public sealed class AccountController : ControllerBase if (!user.TotpEnabled) { - return BadRequest(new { error = "2FA is not enabled" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "2FA is not enabled"); } if (!_passwordService.VerifyPassword(request.Password, user.PasswordHash)) { - return BadRequest(new { error = "Incorrect password" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Incorrect password"); } if (!_totpService.ValidateCode(user.TotpSecret, request.TotpCode)) { - return BadRequest(new { error = "Invalid 2FA code" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Invalid 2FA code"); } user.TotpEnabled = false; @@ -320,7 +320,7 @@ public sealed class AccountController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Plex account management is disabled while OIDC exclusive mode is active." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Plex account management is disabled while OIDC exclusive mode is active."); } var pin = await _plexAuthService.RequestPin(); @@ -333,7 +333,7 @@ public sealed class AccountController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Plex account management is disabled while OIDC exclusive mode is active." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Plex account management is disabled while OIDC exclusive mode is active."); } var pinResult = await _plexAuthService.CheckPin(request.PinId); @@ -369,7 +369,7 @@ public sealed class AccountController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Plex account management is disabled while OIDC exclusive mode is active." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Plex account management is disabled while OIDC exclusive mode is active."); } var user = await GetCurrentUser(); @@ -405,25 +405,18 @@ public sealed class AccountController : ControllerBase [HttpPut("oidc")] public async Task UpdateOidcConfig([FromBody] UpdateOidcConfigRequest request) { - try + var user = await GetCurrentUser(); + if (user is null) { - var user = await GetCurrentUser(); - if (user is null) - { - return Unauthorized(); - } - - request.ApplyTo(user.Oidc); - user.Oidc.Validate(); - user.UpdatedAt = DateTimeOffset.UtcNow; - await _usersContext.SaveChangesAsync(); - - return Ok(new { message = "OIDC configuration updated" }); - } - catch (ValidationException ex) - { - return BadRequest(new { error = ex.Message }); + return Unauthorized(); } + + request.ApplyTo(user.Oidc); + user.Oidc.Validate(); + user.UpdatedAt = DateTimeOffset.UtcNow; + await _usersContext.SaveChangesAsync(); + + return Ok(new { message = "OIDC configuration updated" }); } [HttpPost("oidc/link")] @@ -437,7 +430,7 @@ public sealed class AccountController : ControllerBase if (user.Oidc is not { Enabled: true }) { - return BadRequest(new { error = "OIDC is not enabled" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "OIDC is not enabled"); } var redirectUri = GetOidcLinkCallbackUrl(user.Oidc.RedirectUrl); @@ -450,8 +443,7 @@ public sealed class AccountController : ControllerBase } catch (InvalidOperationException ex) { - _logger.LogWarning(ex, "Failed to start OIDC link authorization"); - return StatusCode(429, new { error = ex.Message }); + throw new RateLimitException(ex.Message, ex); } } @@ -556,7 +548,7 @@ public sealed class AccountController : ControllerBase { if (request.FeatureIds.Count > MaxFeatureIdsPerRequest) { - return BadRequest(new { error = $"featureIds exceeds the maximum allowed ({MaxFeatureIdsPerRequest})." }); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"featureIds exceeds the maximum allowed ({MaxFeatureIdsPerRequest})."); } await UsersContext.Lock.WaitAsync(); diff --git a/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs b/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs index 46edbfe0..129792b2 100644 --- a/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs +++ b/code/backend/Cleanuparr.Api/Features/Auth/Controllers/AuthController.cs @@ -4,6 +4,7 @@ using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.Auth.Contracts.Requests; using Cleanuparr.Api.Features.Auth.Contracts.Responses; using Cleanuparr.Api.Filters; +using Cleanuparr.Domain.Exceptions; using Cleanuparr.Infrastructure.Features.Auth; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Models.Auth; @@ -92,7 +93,7 @@ public sealed class AuthController : ControllerBase var existingUser = await _usersContext.Users.FirstOrDefaultAsync(); if (existingUser is not null) { - return Conflict(new { error = "Account already exists" }); + return this.ProblemResult(StatusCodes.Status409Conflict, "Account already exists"); } var user = new User @@ -133,12 +134,12 @@ public sealed class AuthController : ControllerBase if (user is null) { - return BadRequest(new { error = "Create an account first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Create an account first"); } if (user.SetupCompleted) { - return Conflict(new { error = "Setup already completed. Use account settings to manage 2FA." }); + return this.ProblemResult(StatusCodes.Status409Conflict, "Setup already completed. Use account settings to manage 2FA."); } // Generate new TOTP secret @@ -190,22 +191,22 @@ public sealed class AuthController : ControllerBase var user = await _usersContext.Users.FirstOrDefaultAsync(); if (user is null) { - return BadRequest(new { error = "Create an account first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Create an account first"); } if (user.SetupCompleted) { - return Conflict(new { error = "Setup already completed. Use account settings to manage 2FA." }); + return this.ProblemResult(StatusCodes.Status409Conflict, "Setup already completed. Use account settings to manage 2FA."); } if (string.IsNullOrEmpty(user.TotpSecret)) { - return BadRequest(new { error = "Generate 2FA setup first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Generate 2FA setup first"); } if (!_totpService.ValidateCode(user.TotpSecret, request.Code)) { - return Unauthorized(new { error = "Invalid verification code" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid verification code"); } user.TotpEnabled = true; @@ -231,12 +232,12 @@ public sealed class AuthController : ControllerBase var user = await _usersContext.Users.FirstOrDefaultAsync(); if (user is null) { - return BadRequest(new { error = "Create an account first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Create an account first"); } if (user.SetupCompleted) { - return Conflict(new { error = "Setup already completed" }); + return this.ProblemResult(StatusCodes.Status409Conflict, "Setup already completed"); } user.SetupCompleted = true; @@ -258,7 +259,7 @@ public sealed class AuthController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Login with credentials is disabled. Use OIDC to sign in." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Login with credentials is disabled. Use OIDC to sign in."); } var user = await _usersContext.Users.AsNoTracking().FirstOrDefaultAsync(); @@ -270,20 +271,21 @@ public sealed class AuthController : ControllerBase if (user is null || !user.SetupCompleted) { - return Unauthorized(new { error = "Invalid credentials" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid credentials"); } // Check lockout if (user.LockoutEnd.HasValue && user.LockoutEnd.Value > DateTimeOffset.UtcNow) { - var remaining = (int)Math.Ceiling((user.LockoutEnd.Value - DateTimeOffset.UtcNow).TotalSeconds); - return StatusCode(429, new { error = "Account is locked", retryAfterSeconds = remaining }); + int remaining = (int)Math.Ceiling((user.LockoutEnd.Value - DateTimeOffset.UtcNow).TotalSeconds); + throw new RateLimitException("Account is locked", remaining); } if (!passwordValid || !string.Equals(user.Username, request.Username, StringComparison.OrdinalIgnoreCase)) { - var retryAfterSeconds = await IncrementFailedAttempts(user.Id); - return Unauthorized(new { error = "Invalid credentials", retryAfterSeconds }); + int retryAfterSeconds = await IncrementFailedAttempts(user.Id); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid credentials", + extensions: new Dictionary { ["retryAfterSeconds"] = retryAfterSeconds }); } // Reset failed attempts on successful password verification @@ -320,13 +322,13 @@ public sealed class AuthController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Login with credentials is disabled. Use OIDC to sign in." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Login with credentials is disabled. Use OIDC to sign in."); } var userId = _jwtService.ValidateLoginToken(request.LoginToken); if (userId is null) { - return Unauthorized(new { error = "Invalid or expired login token" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid or expired login token"); } var user = await _usersContext.Users @@ -335,7 +337,7 @@ public sealed class AuthController : ControllerBase if (user is null) { - return Unauthorized(new { error = "Invalid login token" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid login token"); } bool codeValid; @@ -351,7 +353,7 @@ public sealed class AuthController : ControllerBase if (!codeValid) { - return Unauthorized(new { error = "Invalid verification code" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid verification code"); } return Ok(await GenerateTokenResponse(user)); @@ -371,7 +373,7 @@ public sealed class AuthController : ControllerBase if (storedToken is null || storedToken.ExpiresAt < DateTimeOffset.UtcNow) { - return Unauthorized(new { error = "Invalid or expired refresh token" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Invalid or expired refresh token"); } // Revoke the old token (rotation) @@ -420,12 +422,12 @@ public sealed class AuthController : ControllerBase var user = await _usersContext.Users.AsNoTracking().FirstOrDefaultAsync(); if (user is null) { - return BadRequest(new { error = "Create an account first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Create an account first"); } if (user.SetupCompleted) { - return Conflict(new { error = "Setup already completed. Use account settings to manage Plex." }); + return this.ProblemResult(StatusCodes.Status409Conflict, "Setup already completed. Use account settings to manage Plex."); } var pin = await _plexAuthService.RequestPin(); @@ -455,12 +457,12 @@ public sealed class AuthController : ControllerBase var user = await _usersContext.Users.FirstOrDefaultAsync(); if (user is null) { - return BadRequest(new { error = "Create an account first" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Create an account first"); } if (user.SetupCompleted) { - return Conflict(new { error = "Setup already completed. Use account settings to manage Plex." }); + return this.ProblemResult(StatusCodes.Status409Conflict, "Setup already completed. Use account settings to manage Plex."); } user.PlexAccountId = plexAccount.AccountId; @@ -486,13 +488,13 @@ public sealed class AuthController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Plex login is disabled. Use OIDC to sign in." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Plex login is disabled. Use OIDC to sign in."); } var user = await _usersContext.Users.AsNoTracking().FirstOrDefaultAsync(); if (user is null || !user.SetupCompleted || user.PlexAccountId is null) { - return BadRequest(new { error = "Plex login is not available" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Plex login is not available"); } var pin = await _plexAuthService.RequestPin(); @@ -509,13 +511,13 @@ public sealed class AuthController : ControllerBase { if (await IsOidcExclusiveModeActive()) { - return StatusCode(403, new { error = "Plex login is disabled. Use OIDC to sign in." }); + return this.ProblemResult(StatusCodes.Status403Forbidden, "Plex login is disabled. Use OIDC to sign in."); } var user = await _usersContext.Users.FirstOrDefaultAsync(); if (user is null || !user.SetupCompleted || user.PlexAccountId is null) { - return BadRequest(new { error = "Plex login is not available" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Plex login is not available"); } var pinResult = await _plexAuthService.CheckPin(request.PinId); @@ -530,7 +532,7 @@ public sealed class AuthController : ControllerBase if (plexAccount.AccountId != user.PlexAccountId) { - return Unauthorized(new { error = "Plex account does not match the linked account" }); + return this.ProblemResult(StatusCodes.Status401Unauthorized, "Plex account does not match the linked account"); } // Plex OAuth acts as a trusted identity provider — the user explicitly linked their @@ -558,7 +560,7 @@ public sealed class AuthController : ControllerBase string.IsNullOrEmpty(oidcConfig.IssuerUrl) || string.IsNullOrEmpty(oidcConfig.ClientId)) { - return BadRequest(new { error = "OIDC is not enabled or not configured" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "OIDC is not enabled or not configured"); } var redirectUri = GetOidcCallbackUrl(oidcConfig.RedirectUrl); @@ -571,8 +573,7 @@ public sealed class AuthController : ControllerBase } catch (InvalidOperationException ex) { - _logger.LogWarning(ex, "Failed to start OIDC authorization"); - return StatusCode(429, new { error = ex.Message }); + throw new RateLimitException(ex.Message, ex); } } @@ -642,7 +643,7 @@ public sealed class AuthController : ControllerBase if (result is null) { - return NotFound(new { error = "Invalid or expired code" }); + return this.ProblemResult(StatusCodes.Status404NotFound, "Invalid or expired code"); } return Ok(new TokenResponse diff --git a/code/backend/Cleanuparr.Api/Features/BlacklistSync/Controllers/BlacklistSyncConfigController.cs b/code/backend/Cleanuparr.Api/Features/BlacklistSync/Controllers/BlacklistSyncConfigController.cs index 08094e7f..3fba784c 100644 --- a/code/backend/Cleanuparr.Api/Features/BlacklistSync/Controllers/BlacklistSyncConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/BlacklistSync/Controllers/BlacklistSyncConfigController.cs @@ -89,11 +89,6 @@ public sealed class BlacklistSyncConfigController : ControllerBase return Ok(new { Message = "BlacklistSynchronizer configuration updated successfully" }); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save BlacklistSync configuration"); - throw; - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DeadTorrentConfigController.cs b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DeadTorrentConfigController.cs index 6b81b564..0a00a33c 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DeadTorrentConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DeadTorrentConfigController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Requests; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Responses; using Cleanuparr.Domain.Enums; @@ -38,7 +39,7 @@ public class DeadTorrentConfigController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var config = await _dataContext.DeadTorrentConfigs @@ -56,11 +57,6 @@ public class DeadTorrentConfigController : ControllerBase [HttpPut("{downloadClientId}")] public async Task UpdateDeadTorrentConfig(Guid downloadClientId, [FromBody] DeadTorrentConfigRequest dto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -70,12 +66,12 @@ public class DeadTorrentConfigController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } if (dto.Enabled && client.TypeName is DownloadClientTypeName.rTorrent) { - return BadRequest(new { Message = "Dead torrent handling is not supported for rTorrent (no seeder count available)" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Dead torrent handling is not supported for rTorrent (no seeder count available)"); } var existing = await _dataContext.DeadTorrentConfigs diff --git a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DownloadCleanerConfigController.cs b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DownloadCleanerConfigController.cs index 98e5690d..7c4d9e7c 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DownloadCleanerConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/DownloadCleanerConfigController.cs @@ -136,15 +136,6 @@ public sealed class DownloadCleanerConfigController : ControllerBase return Ok(new { Message = "DownloadCleaner configuration updated successfully" }); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save DownloadCleaner configuration"); - throw; - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/OrphanedFilesConfigController.cs b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/OrphanedFilesConfigController.cs index b2767d06..451d703d 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/OrphanedFilesConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/OrphanedFilesConfigController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Requests; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Responses; using Cleanuparr.Persistence; @@ -36,7 +37,7 @@ public sealed class OrphanedFilesConfigController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var config = await _dataContext.OrphanedFilesConfigs @@ -54,11 +55,6 @@ public sealed class OrphanedFilesConfigController : ControllerBase [HttpPut("{downloadClientId}")] public async Task UpdateClientConfig(Guid downloadClientId, [FromBody] OrphanedFilesConfigRequest dto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -68,7 +64,7 @@ public sealed class OrphanedFilesConfigController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var existing = await _dataContext.OrphanedFilesConfigs diff --git a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/SeedingRulesController.cs b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/SeedingRulesController.cs index c20c2475..d7de8276 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/SeedingRulesController.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/SeedingRulesController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Requests; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Responses; using Cleanuparr.Domain.Enums; @@ -8,7 +9,6 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; -using ValidationException = Cleanuparr.Domain.Exceptions.ValidationException; namespace Cleanuparr.Api.Features.DownloadCleaner.Controllers; @@ -40,18 +40,13 @@ public class SeedingRulesController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var rules = await SeedingRuleHelper.GetForClientAsync(_dataContext, client); return Ok(rules.Select(SeedingRuleResponse.From)); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to retrieve seeding rules for client {ClientId}", downloadClientId); - return StatusCode(500, new { Message = "Failed to retrieve seeding rules", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -61,11 +56,6 @@ public class SeedingRulesController : ControllerBase [HttpPost("{downloadClientId}")] public async Task CreateSeedingRule(Guid downloadClientId, [FromBody] SeedingRuleRequest ruleDto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -75,14 +65,14 @@ public class SeedingRulesController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var existingRules = await SeedingRuleHelper.GetForClientAsync(_dataContext, client); if (ruleDto.Priority.HasValue && existingRules.Any(r => r.Priority == ruleDto.Priority.Value)) { - return BadRequest(new { Message = $"A seeding rule with priority {ruleDto.Priority.Value} already exists for this client" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"A seeding rule with priority {ruleDto.Priority.Value} already exists for this client"); } int priority = ruleDto.Priority ?? (existingRules.Count == 0 ? 1 : existingRules.Max(r => r.Priority) + 1); @@ -98,17 +88,6 @@ public class SeedingRulesController : ControllerBase return CreatedAtAction(nameof(GetSeedingRules), new { downloadClientId }, rule); } - catch (ValidationException ex) - { - _logger.LogWarning("Validation failed for seeding rule creation: {Message}", ex.Message); - return BadRequest(new { Message = ex.Message }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create seeding rule: {RuleName} for client {ClientId}", - ruleDto.Name, downloadClientId); - return StatusCode(500, new { Message = "Failed to create seeding rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -118,11 +97,6 @@ public class SeedingRulesController : ControllerBase [HttpPut("{id}")] public async Task UpdateSeedingRule(Guid id, [FromBody] SeedingRuleRequest ruleDto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -130,7 +104,7 @@ public class SeedingRulesController : ControllerBase if (existingRule is null) { - return NotFound(new { Message = $"Seeding rule with ID {id} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Seeding rule with ID {id} not found"); } existingRule.Name = ruleDto.Name.Trim(); @@ -162,16 +136,6 @@ public class SeedingRulesController : ControllerBase return Ok(existingRule); } - catch (ValidationException ex) - { - _logger.LogWarning("Validation failed for seeding rule update: {Message}", ex.Message); - return BadRequest(new { Message = ex.Message }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update seeding rule with ID: {RuleId}", id); - return StatusCode(500, new { Message = "Failed to update seeding rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -181,11 +145,6 @@ public class SeedingRulesController : ControllerBase [HttpPut("{downloadClientId}/reorder")] public async Task ReorderSeedingRules(Guid downloadClientId, [FromBody] ReorderSeedingRulesRequest request) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -195,24 +154,24 @@ public class SeedingRulesController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } List rules = await SeedingRuleHelper.GetForClientTrackedAsync(_dataContext, client); if (request.OrderedIds.Distinct().Count() != request.OrderedIds.Count) { - return BadRequest(new { Message = "Duplicate rule IDs are not allowed" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Duplicate rule IDs are not allowed"); } if (request.OrderedIds.Count != rules.Count) { - return BadRequest(new { Message = $"Expected {rules.Count} rule IDs but received {request.OrderedIds.Count}. All rules must be included." }); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Expected {rules.Count} rule IDs but received {request.OrderedIds.Count}. All rules must be included."); } foreach (Guid id in request.OrderedIds.Where(id => rules.All(r => r.Id != id))) { - return BadRequest(new { Message = $"Rule with ID {id} not found for client {downloadClientId}" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Rule with ID {id} not found for client {downloadClientId}"); } int priority = 1; @@ -229,11 +188,6 @@ public class SeedingRulesController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to reorder seeding rules for client {ClientId}", downloadClientId); - return StatusCode(500, new { Message = "Failed to reorder seeding rules", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -250,7 +204,7 @@ public class SeedingRulesController : ControllerBase if (existingRule is null) { - return NotFound(new { Message = $"Seeding rule with ID {id} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Seeding rule with ID {id} not found"); } RemoveRuleFromDbSet(existingRule); @@ -260,11 +214,6 @@ public class SeedingRulesController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to delete seeding rule with ID: {RuleId}", id); - return StatusCode(500, new { Message = "Failed to delete seeding rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/UnlinkedConfigController.cs b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/UnlinkedConfigController.cs index 1a2cd9a6..9ea58e25 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/UnlinkedConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadCleaner/Controllers/UnlinkedConfigController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Requests; using Cleanuparr.Api.Features.DownloadCleaner.Contracts.Responses; using Cleanuparr.Persistence; @@ -37,7 +38,7 @@ public class UnlinkedConfigController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var config = await _dataContext.UnlinkedConfigs @@ -55,11 +56,6 @@ public class UnlinkedConfigController : ControllerBase [HttpPut("{downloadClientId}")] public async Task UpdateUnlinkedConfig(Guid downloadClientId, [FromBody] UnlinkedConfigRequest dto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -69,7 +65,7 @@ public class UnlinkedConfigController : ControllerBase if (client is null) { - return NotFound(new { Message = $"Download client with ID {downloadClientId} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {downloadClientId} not found"); } var existing = await _dataContext.UnlinkedConfigs diff --git a/code/backend/Cleanuparr.Api/Features/DownloadClient/Controllers/DownloadClientController.cs b/code/backend/Cleanuparr.Api/Features/DownloadClient/Controllers/DownloadClientController.cs index a67b3093..74d432cb 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadClient/Controllers/DownloadClientController.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadClient/Controllers/DownloadClientController.cs @@ -1,6 +1,7 @@ using System; using System.Linq; +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.DownloadClient.Contracts.Requests; using Cleanuparr.Infrastructure.Features.DownloadClient; using Cleanuparr.Infrastructure.Http.DynamicHttpClientSystem; @@ -73,11 +74,6 @@ public sealed class DownloadClientController : ControllerBase return CreatedAtAction(nameof(GetDownloadClientConfig), new { id = clientConfig.Id }, clientConfig); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create download client"); - throw; - } finally { DataContext.Lock.Release(); @@ -97,7 +93,7 @@ public sealed class DownloadClientController : ControllerBase if (existingClient is null) { - return NotFound($"Download client with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {id} not found"); } var clientToPersist = updatedClient.ApplyTo(existingClient); @@ -108,11 +104,6 @@ public sealed class DownloadClientController : ControllerBase return Ok(clientToPersist); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update download client with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -130,7 +121,7 @@ public sealed class DownloadClientController : ControllerBase if (existingClient is null) { - return NotFound($"Download client with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {id} not found"); } _dataContext.DownloadClients.Remove(existingClient); @@ -143,11 +134,6 @@ public sealed class DownloadClientController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to delete download client with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -171,7 +157,7 @@ public sealed class DownloadClientController : ControllerBase if (existingClient is null) { - return NotFound($"Download client with ID {request.ClientId.Value} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Download client with ID {request.ClientId.Value} not found"); } resolvedPassword = existingClient.Password; @@ -190,12 +176,12 @@ public sealed class DownloadClientController : ControllerBase }); } - return BadRequest(new { Message = healthResult.ErrorMessage ?? "Connection failed" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, healthResult.ErrorMessage ?? "Connection failed"); } catch (Exception ex) { _logger.LogError(ex, "Failed to test {TypeName} client connection", request.TypeName); - return BadRequest(new { Message = $"Connection failed: {ex.Message}" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, $"Connection failed: {ex.Message}"); } } } diff --git a/code/backend/Cleanuparr.Api/Features/General/Controllers/GeneralConfigController.cs b/code/backend/Cleanuparr.Api/Features/General/Controllers/GeneralConfigController.cs index d987fbdc..67d3fb05 100644 --- a/code/backend/Cleanuparr.Api/Features/General/Controllers/GeneralConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/General/Controllers/GeneralConfigController.cs @@ -99,11 +99,6 @@ public sealed class GeneralConfigController : ControllerBase return Ok(new { Message = "General configuration updated successfully" }); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save General configuration"); - throw; - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/MalwareBlocker/Controllers/MalwareBlockerConfigController.cs b/code/backend/Cleanuparr.Api/Features/MalwareBlocker/Controllers/MalwareBlockerConfigController.cs index f4090164..b4508e79 100644 --- a/code/backend/Cleanuparr.Api/Features/MalwareBlocker/Controllers/MalwareBlockerConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/MalwareBlocker/Controllers/MalwareBlockerConfigController.cs @@ -74,15 +74,6 @@ public sealed class MalwareBlockerConfigController : ControllerBase return Ok(new { Message = "MalwareBlocker configuration updated successfully" }); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save MalwareBlocker configuration"); - throw; - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/Notifications/Controllers/NotificationProvidersController.cs b/code/backend/Cleanuparr.Api/Features/Notifications/Controllers/NotificationProvidersController.cs index 73291c16..07b1585b 100644 --- a/code/backend/Cleanuparr.Api/Features/Notifications/Controllers/NotificationProvidersController.cs +++ b/code/backend/Cleanuparr.Api/Features/Notifications/Controllers/NotificationProvidersController.cs @@ -1,14 +1,11 @@ -using System.Net; +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.Notifications.Contracts.Requests; using Cleanuparr.Api.Features.Notifications.Contracts.Responses; using Cleanuparr.Domain.Enums; using Cleanuparr.Domain.Exceptions; using Cleanuparr.Infrastructure.Features.Notifications; using Cleanuparr.Infrastructure.Features.Notifications.Apprise; -using Cleanuparr.Infrastructure.Features.Notifications.Discord; using Cleanuparr.Infrastructure.Features.Notifications.Models; -using Cleanuparr.Infrastructure.Features.Notifications.Telegram; -using Cleanuparr.Infrastructure.Features.Notifications.Gotify; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Models.Configuration.Notification; using Cleanuparr.Shared.Helpers; @@ -123,18 +120,18 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.ApiKey.IsPlaceholder()) { - return BadRequest("API key cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "API key cannot be a placeholder value"); } var notifiarrConfig = new NotifiarrConfig @@ -168,11 +165,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Notifiarr provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -187,23 +179,23 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.Key.IsPlaceholder()) { - return BadRequest("Key cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Key cannot be a placeholder value"); } if (newProvider.ServiceUrls.IsPlaceholder()) { - return BadRequest("Service URLs cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Service URLs cannot be a placeholder value"); } var appriseConfig = new AppriseConfig @@ -240,15 +232,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Apprise provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -263,23 +246,23 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.Password.IsPlaceholder()) { - return BadRequest("Password cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Password cannot be a placeholder value"); } if (newProvider.AccessToken.IsPlaceholder()) { - return BadRequest("Access token cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Access token cannot be a placeholder value"); } var ntfyConfig = new NtfyConfig @@ -319,15 +302,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Ntfy provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -342,18 +316,18 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.BotToken.IsPlaceholder()) { - return BadRequest("Bot token cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Bot token cannot be a placeholder value"); } var telegramConfig = new TelegramConfig @@ -389,15 +363,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Telegram provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -416,12 +381,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Notifiarr provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Notifiarr provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -430,7 +395,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var notifiarrConfig = new NotifiarrConfig @@ -472,15 +437,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Notifiarr provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -499,12 +455,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Apprise provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Apprise provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -513,7 +469,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var appriseConfig = new AppriseConfig @@ -560,15 +516,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Apprise provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -587,12 +534,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Ntfy provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Ntfy provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -601,7 +548,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var ntfyConfig = new NtfyConfig @@ -651,15 +598,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Ntfy provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -678,12 +616,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Telegram provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Telegram provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -692,7 +630,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var telegramConfig = new TelegramConfig @@ -736,15 +674,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Telegram provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -769,7 +698,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Notification provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Notification provider with ID {id} not found"); } _dataContext.NotificationConfigs.Remove(existingProvider); @@ -782,11 +711,6 @@ public sealed class NotificationProvidersController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to delete notification provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -807,7 +731,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existing is null) { - return BadRequest(new { Message = "API key cannot be a placeholder value" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "API key cannot be a placeholder value"); } apiKey = existing.ApiKey; @@ -845,8 +769,7 @@ public sealed class NotificationProvidersController : ControllerBase } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Notifiarr provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } @@ -865,7 +788,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existing is null) { - return BadRequest(new { Message = "Sensitive fields cannot be placeholder values" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Sensitive fields cannot be placeholder values"); } if (key.IsPlaceholder()) @@ -912,14 +835,9 @@ public sealed class NotificationProvidersController : ControllerBase await _notificationService.SendTestNotificationAsync(providerDto); return Ok(new { Message = "Test notification sent successfully" }); } - catch (AppriseException exception) - { - return StatusCode((int)HttpStatusCode.InternalServerError, exception.Message); - } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Apprise provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } @@ -938,7 +856,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existing is null) { - return BadRequest(new { Message = "Sensitive fields cannot be placeholder values" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Sensitive fields cannot be placeholder values"); } if (password.IsPlaceholder()) @@ -990,8 +908,7 @@ public sealed class NotificationProvidersController : ControllerBase } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Ntfy provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } @@ -1009,7 +926,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existing is null) { - return BadRequest(new { Message = "Bot token cannot be a placeholder value" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Bot token cannot be a placeholder value"); } botToken = existing.BotToken; @@ -1047,15 +964,9 @@ public sealed class NotificationProvidersController : ControllerBase await _notificationService.SendTestNotificationAsync(providerDto); return Ok(new { Message = "Test notification sent successfully" }); } - catch (TelegramException ex) - { - _logger.LogWarning(ex, "Failed to test Telegram provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); - } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Telegram provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } @@ -1100,18 +1011,18 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.WebhookUrl.IsPlaceholder()) { - return BadRequest("Webhook URL cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Webhook URL cannot be a placeholder value"); } var discordConfig = new DiscordConfig @@ -1146,15 +1057,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Discord provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -1173,12 +1075,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Discord provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Discord provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -1187,7 +1089,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var discordConfig = new DiscordConfig @@ -1230,15 +1132,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Discord provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -1259,7 +1152,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existing is null) { - return BadRequest(new { Message = "Webhook URL cannot be a placeholder value" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Webhook URL cannot be a placeholder value"); } webhookUrl = existing.WebhookUrl; @@ -1296,15 +1189,9 @@ public sealed class NotificationProvidersController : ControllerBase await _notificationService.SendTestNotificationAsync(providerDto); return Ok(new { Message = "Test notification sent successfully" }); } - catch (DiscordException ex) - { - _logger.LogWarning(ex, "Failed to test Discord provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); - } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Discord provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } @@ -1316,23 +1203,23 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.ApiToken.IsPlaceholder()) { - return BadRequest("API token cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "API token cannot be a placeholder value"); } if (newProvider.UserKey.IsPlaceholder()) { - return BadRequest("User key cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "User key cannot be a placeholder value"); } var pushoverConfig = new PushoverConfig @@ -1372,15 +1259,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Pushover provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -1399,12 +1277,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Pushover provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Pushover provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -1413,7 +1291,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var pushoverConfig = new PushoverConfig @@ -1463,15 +1341,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Pushover provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -1493,7 +1362,7 @@ public sealed class NotificationProvidersController : ControllerBase if (existing is null) { - return BadRequest(new { Message = "Sensitive fields cannot be placeholder values" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Sensitive fields cannot be placeholder values"); } if (apiToken.IsPlaceholder()) @@ -1545,8 +1414,7 @@ public sealed class NotificationProvidersController : ControllerBase } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Pushover provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } @@ -1558,18 +1426,18 @@ public sealed class NotificationProvidersController : ControllerBase { if (string.IsNullOrWhiteSpace(newProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs.CountAsync(x => x.Name == newProvider.Name); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } if (newProvider.ApplicationToken.IsPlaceholder()) { - return BadRequest("Application token cannot be a placeholder value"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Application token cannot be a placeholder value"); } var gotifyConfig = new GotifyConfig @@ -1604,15 +1472,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(provider); return CreatedAtAction(nameof(GetNotificationProviders), new { id = provider.Id }, providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create Gotify provider"); - throw; - } finally { DataContext.Lock.Release(); @@ -1631,12 +1490,12 @@ public sealed class NotificationProvidersController : ControllerBase if (existingProvider == null) { - return NotFound($"Gotify provider with ID {id} not found"); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Gotify provider with ID {id} not found"); } if (string.IsNullOrWhiteSpace(updatedProvider.Name)) { - return BadRequest("Provider name is required"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "Provider name is required"); } var duplicateConfig = await _dataContext.NotificationConfigs @@ -1645,7 +1504,7 @@ public sealed class NotificationProvidersController : ControllerBase .CountAsync(); if (duplicateConfig > 0) { - return BadRequest("A provider with this name already exists"); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A provider with this name already exists"); } var gotifyConfig = new GotifyConfig @@ -1688,15 +1547,6 @@ public sealed class NotificationProvidersController : ControllerBase var providerDto = MapProvider(newProvider); return Ok(providerDto); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update Gotify provider with ID {Id}", id); - throw; - } finally { DataContext.Lock.Release(); @@ -1716,7 +1566,9 @@ public sealed class NotificationProvidersController : ControllerBase testRequest.ProviderId, NotificationProviderType.Gotify, p => p.GotifyConfiguration); if (existing is null) - return BadRequest(new { Message = "Application token cannot be a placeholder value" }); + { + return this.ProblemResult(StatusCodes.Status400BadRequest, "Application token cannot be a placeholder value"); + } applicationToken = existing.ApplicationToken; } @@ -1752,15 +1604,9 @@ public sealed class NotificationProvidersController : ControllerBase await _notificationService.SendTestNotificationAsync(providerDto); return Ok(new { Message = "Test notification sent successfully" }); } - catch (GotifyException ex) - { - _logger.LogWarning(ex, "Failed to test Gotify provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); - } catch (Exception ex) { - _logger.LogError(ex, "Failed to test Gotify provider"); - return BadRequest(new { Message = $"Test failed: {ex.Message}" }); + throw new NotificationTestException($"Test failed: {ex.Message}", ex); } } diff --git a/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueCleanerConfigController.cs b/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueCleanerConfigController.cs index 4a56dcae..24610f5c 100644 --- a/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueCleanerConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueCleanerConfigController.cs @@ -1,5 +1,3 @@ -using System.ComponentModel.DataAnnotations; - using Cleanuparr.Api.Features.QueueCleaner.Contracts.Requests; using Cleanuparr.Domain.Enums; using Cleanuparr.Infrastructure.Services.Interfaces; @@ -80,15 +78,6 @@ public sealed class QueueCleanerConfigController : ControllerBase return Ok(new { Message = "QueueCleaner configuration updated successfully" }); } - catch (ValidationException ex) - { - return BadRequest(ex.Message); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save QueueCleaner configuration"); - throw; - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueRulesController.cs b/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueRulesController.cs index 35bb650c..5453ddd6 100644 --- a/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueRulesController.cs +++ b/code/backend/Cleanuparr.Api/Features/QueueCleaner/Controllers/QueueRulesController.cs @@ -1,5 +1,5 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.QueueCleaner.Contracts.Requests; -using Cleanuparr.Domain.Exceptions; using Cleanuparr.Infrastructure.Services.Interfaces; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Models.Configuration.QueueCleaner; @@ -43,11 +43,6 @@ public class QueueRulesController : ControllerBase return Ok(rules); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to retrieve stall rules"); - return StatusCode(500, new { Message = "Failed to retrieve stall rules", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -57,11 +52,6 @@ public class QueueRulesController : ControllerBase [HttpPost("stall")] public async Task CreateStallRule([FromBody] StallRuleDto ruleDto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -73,7 +63,7 @@ public class QueueRulesController : ControllerBase if (existingRule != null) { - return BadRequest(new { Message = "A stall rule with this name already exists" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A stall rule with this name already exists"); } var rule = new StallRule @@ -97,7 +87,7 @@ public class QueueRulesController : ControllerBase var intervalValidationResult = _ruleIntervalValidator.ValidateStallRuleIntervals(rule, existingRules); if (!intervalValidationResult.IsValid) { - return BadRequest(new { Message = intervalValidationResult.ErrorMessage }); + return this.ProblemResult(StatusCodes.Status400BadRequest, intervalValidationResult.ErrorMessage); } rule.Validate(); @@ -109,16 +99,6 @@ public class QueueRulesController : ControllerBase return CreatedAtAction(nameof(GetStallRules), new { id = rule.Id }, rule); } - catch (ValidationException ex) - { - _logger.LogWarning("Validation failed for stall rule creation: {Message}", ex.Message); - return BadRequest(new { Message = ex.Message }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create stall rule: {RuleName}", ruleDto.Name); - return StatusCode(500, new { Message = "Failed to create stall rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -128,11 +108,6 @@ public class QueueRulesController : ControllerBase [HttpPut("stall/{id}")] public async Task UpdateStallRule(Guid id, [FromBody] StallRuleDto ruleDto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -141,15 +116,15 @@ public class QueueRulesController : ControllerBase if (existingRule == null) { - return NotFound(new { Message = $"Stall rule with ID {id} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Stall rule with ID {id} not found"); } var duplicateRule = await _dataContext.StallRules .FirstOrDefaultAsync(r => r.Id != id && r.Name.ToLower() == ruleDto.Name.ToLower()); - + if (duplicateRule != null) { - return BadRequest(new { Message = "A stall rule with this name already exists" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A stall rule with this name already exists"); } var updatedRule = existingRule with @@ -173,7 +148,7 @@ public class QueueRulesController : ControllerBase var intervalValidationResult = _ruleIntervalValidator.ValidateStallRuleIntervals(updatedRule, existingRules); if (!intervalValidationResult.IsValid) { - return BadRequest(new { Message = intervalValidationResult.ErrorMessage }); + return this.ProblemResult(StatusCodes.Status400BadRequest, intervalValidationResult.ErrorMessage); } updatedRule.Validate(); @@ -185,16 +160,6 @@ public class QueueRulesController : ControllerBase return Ok(updatedRule); } - catch (ValidationException ex) - { - _logger.LogWarning("Validation failed for stall rule update: {Message}", ex.Message); - return BadRequest(new { Message = ex.Message }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update stall rule with ID: {RuleId}", id); - return StatusCode(500, new { Message = "Failed to update stall rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -212,7 +177,7 @@ public class QueueRulesController : ControllerBase if (existingRule == null) { - return NotFound(new { Message = $"Stall rule with ID {id} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Stall rule with ID {id} not found"); } _dataContext.StallRules.Remove(existingRule); @@ -222,11 +187,6 @@ public class QueueRulesController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to delete stall rule with ID: {RuleId}", id); - return StatusCode(500, new { Message = "Failed to delete stall rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -247,11 +207,6 @@ public class QueueRulesController : ControllerBase return Ok(rules); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to retrieve slow rules"); - return StatusCode(500, new { Message = "Failed to retrieve slow rules", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -261,11 +216,6 @@ public class QueueRulesController : ControllerBase [HttpPost("slow")] public async Task CreateSlowRule([FromBody] SlowRuleDto ruleDto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -277,7 +227,7 @@ public class QueueRulesController : ControllerBase if (existingRule != null) { - return BadRequest(new { Message = "A slow rule with this name already exists" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A slow rule with this name already exists"); } var rule = new SlowRule @@ -303,7 +253,7 @@ public class QueueRulesController : ControllerBase var intervalValidationResult = _ruleIntervalValidator.ValidateSlowRuleIntervals(rule, existingRules); if (!intervalValidationResult.IsValid) { - return BadRequest(new { Message = intervalValidationResult.ErrorMessage }); + return this.ProblemResult(StatusCodes.Status400BadRequest, intervalValidationResult.ErrorMessage); } rule.Validate(); @@ -315,16 +265,6 @@ public class QueueRulesController : ControllerBase return CreatedAtAction(nameof(GetSlowRules), new { id = rule.Id }, rule); } - catch (ValidationException ex) - { - _logger.LogWarning("Validation failed for slow rule creation: {Message}", ex.Message); - return BadRequest(new { Message = ex.Message }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create slow rule: {RuleName}", ruleDto.Name); - return StatusCode(500, new { Message = "Failed to create slow rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -334,11 +274,6 @@ public class QueueRulesController : ControllerBase [HttpPut("slow/{id}")] public async Task UpdateSlowRule(Guid id, [FromBody] SlowRuleDto ruleDto) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - await DataContext.Lock.WaitAsync(); try { @@ -347,15 +282,15 @@ public class QueueRulesController : ControllerBase if (existingRule == null) { - return NotFound(new { Message = $"Slow rule with ID {id} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Slow rule with ID {id} not found"); } var duplicateRule = await _dataContext.SlowRules .FirstOrDefaultAsync(r => r.Id != id && r.Name.ToLower() == ruleDto.Name.ToLower()); - + if (duplicateRule != null) { - return BadRequest(new { Message = "A slow rule with this name already exists" }); + return this.ProblemResult(StatusCodes.Status400BadRequest, "A slow rule with this name already exists"); } var updatedRule = existingRule with @@ -381,7 +316,7 @@ public class QueueRulesController : ControllerBase var intervalValidationResult = _ruleIntervalValidator.ValidateSlowRuleIntervals(updatedRule, existingRules); if (!intervalValidationResult.IsValid) { - return BadRequest(new { Message = intervalValidationResult.ErrorMessage }); + return this.ProblemResult(StatusCodes.Status400BadRequest, intervalValidationResult.ErrorMessage); } updatedRule.Validate(); @@ -393,16 +328,6 @@ public class QueueRulesController : ControllerBase return Ok(updatedRule); } - catch (ValidationException ex) - { - _logger.LogWarning("Validation failed for slow rule update: {Message}", ex.Message); - return BadRequest(new { Message = ex.Message }); - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to update slow rule with ID: {RuleId}", id); - return StatusCode(500, new { Message = "Failed to update slow rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); @@ -420,7 +345,7 @@ public class QueueRulesController : ControllerBase if (existingRule == null) { - return NotFound(new { Message = $"Slow rule with ID {id} not found" }); + return this.ProblemResult(StatusCodes.Status404NotFound, $"Slow rule with ID {id} not found"); } _dataContext.SlowRules.Remove(existingRule); @@ -430,11 +355,6 @@ public class QueueRulesController : ControllerBase return NoContent(); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to delete slow rule with ID: {RuleId}", id); - return StatusCode(500, new { Message = "Failed to delete slow rule", Error = ex.Message }); - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Features/Seeker/Controllers/SeekerConfigController.cs b/code/backend/Cleanuparr.Api/Features/Seeker/Controllers/SeekerConfigController.cs index 0ccf6045..803fe192 100644 --- a/code/backend/Cleanuparr.Api/Features/Seeker/Controllers/SeekerConfigController.cs +++ b/code/backend/Cleanuparr.Api/Features/Seeker/Controllers/SeekerConfigController.cs @@ -1,3 +1,4 @@ +using Cleanuparr.Api.Extensions; using Cleanuparr.Api.Features.Seeker.Contracts.Requests; using Cleanuparr.Shared.Helpers; using Cleanuparr.Api.Features.Seeker.Contracts.Responses; @@ -89,7 +90,7 @@ public sealed class SeekerConfigController : ControllerBase { if (!await DataContext.Lock.WaitAsync(TimeSpan.FromSeconds(30))) { - return StatusCode(503, "Database is busy, please try again"); + return this.ProblemResult(StatusCodes.Status503ServiceUnavailable, "Database is busy, please try again"); } try @@ -194,11 +195,6 @@ public sealed class SeekerConfigController : ControllerBase return Ok(new { Message = "Seeker configuration updated successfully" }); } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to save Seeker configuration"); - throw; - } finally { DataContext.Lock.Release(); diff --git a/code/backend/Cleanuparr.Api/Middleware/ExceptionMiddleware.cs b/code/backend/Cleanuparr.Api/Middleware/ExceptionMiddleware.cs deleted file mode 100644 index 1867176c..00000000 --- a/code/backend/Cleanuparr.Api/Middleware/ExceptionMiddleware.cs +++ /dev/null @@ -1,77 +0,0 @@ -using System.Net; -using System.Text.Json; -using Cleanuparr.Api.Models; -using Cleanuparr.Domain.Exceptions; - -namespace Cleanuparr.Api.Middleware; - -public class ExceptionMiddleware -{ - private readonly RequestDelegate _next; - private readonly ILogger _logger; - - public ExceptionMiddleware(RequestDelegate next, ILogger logger) - { - _next = next; - _logger = logger; - } - - public async Task InvokeAsync(HttpContext context) - { - try - { - await _next(context); - } - catch (Exception ex) - { - await HandleExceptionAsync(context, ex); - } - } - - private async Task HandleExceptionAsync(HttpContext context, Exception exception) - { - // Generate a unique identifier for this error - string traceId = Guid.NewGuid().ToString(); - - // Default status code and message - int statusCode = (int)HttpStatusCode.InternalServerError; - string message = "An unexpected error occurred"; - - switch (exception) - { - // Handle different exception types - case ValidationException: - statusCode = (int)HttpStatusCode.BadRequest; - message = exception.Message; // Use the validation message directly - - _logger.LogWarning(exception, - "Validation error {TraceId} occurred during request to {Path}", - traceId, context.Request.Path); - break; - - default: - // Log other exceptions as errors with more details - _logger.LogError(exception, - "Error {TraceId} occurred during request to {Path}: {Message}", - traceId, context.Request.Path, exception.Message); - break; - } - - // Create the error response - ErrorResponse errorResponse = new() - { - TraceId = traceId, - Error = message - }; - - // Set the response - context.Response.ContentType = "application/json"; - context.Response.StatusCode = statusCode; - - // Write the response - await context.Response.WriteAsync(JsonSerializer.Serialize(errorResponse, new JsonSerializerOptions - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase - })); - } -} diff --git a/code/backend/Cleanuparr.Api/Middleware/GlobalExceptionHandler.cs b/code/backend/Cleanuparr.Api/Middleware/GlobalExceptionHandler.cs new file mode 100644 index 00000000..b936ae42 --- /dev/null +++ b/code/backend/Cleanuparr.Api/Middleware/GlobalExceptionHandler.cs @@ -0,0 +1,76 @@ +using Cleanuparr.Domain.Exceptions; +using Microsoft.AspNetCore.Diagnostics; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Infrastructure; + +namespace Cleanuparr.Api.Middleware; + +/// +/// Single source of truth for mapping unhandled exceptions to RFC 9457 problem-details responses. +/// Registered via AddExceptionHandler + UseExceptionHandler. +/// +public sealed class GlobalExceptionHandler : IExceptionHandler +{ + private readonly IProblemDetailsService _problemDetailsService; + private readonly ProblemDetailsFactory _problemDetailsFactory; + private readonly ILogger _logger; + + public GlobalExceptionHandler( + IProblemDetailsService problemDetailsService, + ProblemDetailsFactory problemDetailsFactory, + ILogger logger) + { + _problemDetailsService = problemDetailsService; + _problemDetailsFactory = problemDetailsFactory; + _logger = logger; + } + + public async ValueTask TryHandleAsync(HttpContext context, Exception exception, CancellationToken cancellationToken) + { + (int status, string title, string detail) = exception switch + { + ValidationException => (StatusCodes.Status400BadRequest, "Validation failed", exception.Message), + NotificationTestException => (StatusCodes.Status400BadRequest, "Notification test failed", exception.Message), + RateLimitException => (StatusCodes.Status429TooManyRequests, "Too many requests", exception.Message), + _ => (StatusCodes.Status500InternalServerError, "An error occurred", "An unexpected error occurred"), + }; + + string path = Sanitize(context.Request.Path); + + if (status >= StatusCodes.Status500InternalServerError) + { + _logger.LogError(exception, "Unhandled error during request to {Path}", path); + } + else + { + _logger.LogWarning(exception, "Handled {Status} during request to {Path}: {Message}", + status, path, Sanitize(exception.Message)); + } + + context.Response.StatusCode = status; + + ProblemDetails problemDetails = _problemDetailsFactory.CreateProblemDetails( + context, statusCode: status, title: title, detail: detail); + + if (exception is RateLimitException { RetryAfterSeconds: > 0 } rateLimitException) + { + problemDetails.Extensions["retryAfterSeconds"] = rateLimitException.RetryAfterSeconds; + context.Response.Headers.RetryAfter = rateLimitException.RetryAfterSeconds.ToString(); + } + + return await _problemDetailsService.TryWriteAsync(new ProblemDetailsContext + { + HttpContext = context, + ProblemDetails = problemDetails, + Exception = exception, + }); + } + + /// + /// Strips line breaks from user-controlled values before they reach the logs to prevent log forging. + /// + private static string Sanitize(string? value) + { + return value is null ? string.Empty : value.Replace("\r", string.Empty).Replace("\n", string.Empty); + } +} diff --git a/code/backend/Cleanuparr.Api/Models/ErrorResponse.cs b/code/backend/Cleanuparr.Api/Models/ErrorResponse.cs deleted file mode 100644 index 5fafaf0d..00000000 --- a/code/backend/Cleanuparr.Api/Models/ErrorResponse.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace Cleanuparr.Api.Models; - -/// -/// Standardized error response model for API endpoints -/// -public class ErrorResponse -{ - /// - /// User-friendly error message - /// - public required string Error { get; set; } - - /// - /// Trace ID for error tracking (GUID) - /// - public required string TraceId { get; set; } -} diff --git a/code/backend/Cleanuparr.Domain/Exceptions/NotificationTestException.cs b/code/backend/Cleanuparr.Domain/Exceptions/NotificationTestException.cs new file mode 100644 index 00000000..d227b0cc --- /dev/null +++ b/code/backend/Cleanuparr.Domain/Exceptions/NotificationTestException.cs @@ -0,0 +1,16 @@ +namespace Cleanuparr.Domain.Exceptions; + +/// +/// Thrown when a notification provider connectivity test fails. Maps to HTTP 400 so a failed +/// test is reported as a bad request rather than an unexpected server error. +/// +public sealed class NotificationTestException : Exception +{ + public NotificationTestException(string message) : base(message) + { + } + + public NotificationTestException(string message, Exception inner) : base(message, inner) + { + } +} diff --git a/code/backend/Cleanuparr.Domain/Exceptions/RateLimitException.cs b/code/backend/Cleanuparr.Domain/Exceptions/RateLimitException.cs new file mode 100644 index 00000000..c283a182 --- /dev/null +++ b/code/backend/Cleanuparr.Domain/Exceptions/RateLimitException.cs @@ -0,0 +1,20 @@ +namespace Cleanuparr.Domain.Exceptions; + +/// +/// Thrown when a request is rejected due to rate limiting. Maps to HTTP 429 with a +/// retryAfterSeconds problem-details extension and a Retry-After header. +/// +public sealed class RateLimitException : Exception +{ + public int RetryAfterSeconds { get; } + + public RateLimitException(string message, int retryAfterSeconds = 0) : base(message) + { + RetryAfterSeconds = retryAfterSeconds; + } + + public RateLimitException(string message, Exception inner, int retryAfterSeconds = 0) : base(message, inner) + { + RetryAfterSeconds = retryAfterSeconds; + } +} diff --git a/code/frontend/src/app/core/interceptors/error.interceptor.ts b/code/frontend/src/app/core/interceptors/error.interceptor.ts index c00b9cf7..064f9216 100644 --- a/code/frontend/src/app/core/interceptors/error.interceptor.ts +++ b/code/frontend/src/app/core/interceptors/error.interceptor.ts @@ -1,33 +1,55 @@ import { HttpInterceptorFn, HttpErrorResponse } from '@angular/common/http'; import { catchError, throwError } from 'rxjs'; +interface ProblemDetails { + type?: string; + title?: string; + status?: number; + detail?: string; + traceId?: string; + retryAfterSeconds?: number; +} + export class ApiError extends Error { retryAfterSeconds?: number; statusCode?: number; + traceId?: string; +} + +function resolveMessage(error: HttpErrorResponse): string { + // Transport-level failure: no HTTP response reached the client (offline, CORS, DNS, ...). + if (error.status === 0 || error.error instanceof ProgressEvent) { + return 'Unable to reach the server'; + } + + // Client-side error raised while processing the request. + if (error.error instanceof ErrorEvent) { + return error.error.message; + } + + // Plain-string body. + if (typeof error.error === 'string' && error.error.length > 0) { + return error.error; + } + + // Structured body: prefer RFC 9457 ProblemDetails, then fall back to legacy { error } / { message } shapes. + const body = error.error as (ProblemDetails & { error?: string; message?: string }) | null; + return body?.detail ?? body?.title ?? body?.error ?? body?.message ?? `Error ${error.status}`; } export const errorInterceptor: HttpInterceptorFn = (req, next) => { return next(req).pipe( catchError((error: HttpErrorResponse) => { - let message = 'An unexpected error occurred'; + const apiError = new ApiError(resolveMessage(error)); + apiError.statusCode = error.status; - if (error.error instanceof ErrorEvent) { - // Client-side error - message = error.error.message; - } else if (typeof error.error === 'string') { - // Server-side error with plain string body - message = error.error; - } else { - // Server-side error with JSON body - message = error.error?.error - ?? error.error?.message - ?? error.message - ?? `Error ${error.status}`; + const body = error.error; + if (body && typeof body === 'object' && !(body instanceof ErrorEvent) && !(body instanceof ProgressEvent)) { + const problem = body as ProblemDetails; + apiError.retryAfterSeconds = problem.retryAfterSeconds; + apiError.traceId = problem.traceId; } - const apiError = new ApiError(message); - apiError.retryAfterSeconds = error.error?.retryAfterSeconds; - apiError.statusCode = error.status; return throwError(() => apiError); }), );