diff --git a/coffeecard/CoffeeCard.Common/Errors/ApiException.cs b/coffeecard/CoffeeCard.Common/Errors/ApiException.cs index 48c38013..3a5ff64c 100644 --- a/coffeecard/CoffeeCard.Common/Errors/ApiException.cs +++ b/coffeecard/CoffeeCard.Common/Errors/ApiException.cs @@ -2,7 +2,6 @@ namespace CoffeeCard.Common.Errors { - [Serializable] public class ApiException : Exception { public int StatusCode { get; } diff --git a/coffeecard/CoffeeCard.Common/Errors/BadRequestException.cs b/coffeecard/CoffeeCard.Common/Errors/BadRequestException.cs index 352ed997..28c67ced 100644 --- a/coffeecard/CoffeeCard.Common/Errors/BadRequestException.cs +++ b/coffeecard/CoffeeCard.Common/Errors/BadRequestException.cs @@ -1,14 +1,15 @@ using System; +using Microsoft.AspNetCore.Http; namespace CoffeeCard.Common.Errors; public class BadRequestException : ApiException { - public BadRequestException(string message, int statusCode = 400) : base(message, statusCode) + public BadRequestException(string message) : base(message, StatusCodes.Status400BadRequest) { } - public BadRequestException(Exception ex, int statusCode = 400) : base(ex, statusCode) + public BadRequestException(Exception ex) : base(ex, StatusCodes.Status400BadRequest) { } } \ No newline at end of file diff --git a/coffeecard/CoffeeCard.Common/Errors/ConflictException.cs b/coffeecard/CoffeeCard.Common/Errors/ConflictException.cs index 83d336de..3923aefd 100644 --- a/coffeecard/CoffeeCard.Common/Errors/ConflictException.cs +++ b/coffeecard/CoffeeCard.Common/Errors/ConflictException.cs @@ -1,9 +1,7 @@ -using System; using Microsoft.AspNetCore.Http; namespace CoffeeCard.Common.Errors { - [Serializable] public class ConflictException : ApiException { public ConflictException(string message) : base(message, statusCode: StatusCodes.Status409Conflict) diff --git a/coffeecard/CoffeeCard.Common/Errors/EntityNotFoundException.cs b/coffeecard/CoffeeCard.Common/Errors/EntityNotFoundException.cs index 06f93788..a019bdc8 100644 --- a/coffeecard/CoffeeCard.Common/Errors/EntityNotFoundException.cs +++ b/coffeecard/CoffeeCard.Common/Errors/EntityNotFoundException.cs @@ -1,9 +1,7 @@ -using System; using Microsoft.AspNetCore.Http; namespace CoffeeCard.Common.Errors { - [Serializable] public class EntityNotFoundException : ApiException { public EntityNotFoundException(string message) : base(message, statusCode: StatusCodes.Status404NotFound) diff --git a/coffeecard/CoffeeCard.Common/Errors/IllegalUserOperationException.cs b/coffeecard/CoffeeCard.Common/Errors/IllegalUserOperationException.cs index 803f9ded..bb3fd868 100644 --- a/coffeecard/CoffeeCard.Common/Errors/IllegalUserOperationException.cs +++ b/coffeecard/CoffeeCard.Common/Errors/IllegalUserOperationException.cs @@ -1,9 +1,7 @@ -using System; using Microsoft.AspNetCore.Http; namespace CoffeeCard.Common.Errors { - [Serializable] public class IllegalUserOperationException : ApiException { public IllegalUserOperationException(string message) : base(message, statusCode: StatusCodes.Status403Forbidden) diff --git a/coffeecard/CoffeeCard.Common/Errors/UnauthorizedException.cs b/coffeecard/CoffeeCard.Common/Errors/UnauthorizedException.cs index 29506782..6cfe5c5a 100644 --- a/coffeecard/CoffeeCard.Common/Errors/UnauthorizedException.cs +++ b/coffeecard/CoffeeCard.Common/Errors/UnauthorizedException.cs @@ -1,15 +1,14 @@ using System; -using System.Runtime.Serialization; +using Microsoft.AspNetCore.Http; namespace CoffeeCard.Common.Errors { - [Serializable] public class UnauthorizedException : ApiException { - public UnauthorizedException(string message) : base(message, 401) + public UnauthorizedException(string message) : base(message, StatusCodes.Status401Unauthorized) { } - public UnauthorizedException(Exception ex) : base(ex.Message, 401) + public UnauthorizedException(Exception ex) : base(ex.Message, StatusCodes.Status401Unauthorized) { } } } \ No newline at end of file diff --git a/coffeecard/CoffeeCard.WebApi/Helpers/ApiExceptionFilter.cs b/coffeecard/CoffeeCard.WebApi/Helpers/ApiExceptionFilter.cs index 6456337f..321301e3 100644 --- a/coffeecard/CoffeeCard.WebApi/Helpers/ApiExceptionFilter.cs +++ b/coffeecard/CoffeeCard.WebApi/Helpers/ApiExceptionFilter.cs @@ -1,57 +1,72 @@ using System; using CoffeeCard.Common.Errors; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; -using Serilog; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; namespace CoffeeCard.WebApi.Helpers { /// /// Exception filter for handling API exceptions and returning appropriate HTTP status codes and error messages. /// - public class ApiExceptionFilter : ExceptionFilterAttribute + public class ApiExceptionFilter(IWebHostEnvironment environment, ILogger logger) + : ExceptionFilterAttribute { + private readonly ILogger _logger = logger; + private readonly IWebHostEnvironment _environment = environment; + /// public override void OnException(ExceptionContext context) { - ApiError apiError; - switch (context.Exception) - { - case ConflictException exception: - apiError = new ApiError(exception.Message); - context.HttpContext.Response.StatusCode = StatusCodes.Status409Conflict; - break; - case ApiException exception: - { - apiError = new ApiError(exception.Message); - context.HttpContext.Response.StatusCode = exception.StatusCode; - break; - } - case UnauthorizedAccessException _: - apiError = new ApiError("Unauthorized Access"); - context.HttpContext.Response.StatusCode = StatusCodes.Status401Unauthorized; - break; - default: - { - Log.Error(context.Exception, "Unhandled exception caught"); - -#if !DEBUG - var msg = "An unhandled error occurred."; -#else - var msg = context.Exception.GetBaseException().Message; -#endif - - apiError = new ApiError(msg); - context.HttpContext.Response.StatusCode = StatusCodes.Status500InternalServerError; - break; - } - } + var exception = context.Exception; + + var responseMessage = GetResponseMessage(exception); + var apiError = new ApiError(responseMessage); + context.HttpContext.Response.StatusCode = GetStatusCode(exception); + LogException(exception); // always return a JSON result context.Result = new JsonResult(apiError); + context.ExceptionHandled = true; base.OnException(context); } + + private string GetResponseMessage(Exception exception) + { + return exception switch + { + // We consider our own exceptions fine for sending to users + ApiException apiException => apiException.Message, + // To avoid leaking internal errors, we only show the exception message in dev + _ => _environment.IsDevelopment() + ? exception.GetBaseException().Message + : "Unhandled exception caught" + }; + } + + private int GetStatusCode(Exception exception) + { + return exception switch + { + ApiException apiException => apiException.StatusCode, + _ => StatusCodes.Status500InternalServerError + }; + } + + private void LogException(Exception exception) + { + if (exception is ApiException apiException) + { + _logger.LogWarning(apiException, "Unintended user flow occured"); + } + else + { + _logger.LogError(exception, "Unhandled exception caught"); + } + } } } \ No newline at end of file diff --git a/coffeecard/CoffeeCard.WebApi/Startup.cs b/coffeecard/CoffeeCard.WebApi/Startup.cs index 2271d630..8d790f16 100644 --- a/coffeecard/CoffeeCard.WebApi/Startup.cs +++ b/coffeecard/CoffeeCard.WebApi/Startup.cs @@ -184,7 +184,7 @@ public void ConfigureServices(IServiceCollection services) // Setup filter to catch outgoing exceptions services.AddControllers(options => { - options.Filters.Add(new ApiExceptionFilter()); + options.Filters.Add(); options.Filters.Add(new ReadableBodyFilter()); }) .AddJsonOptions(options =>