refactor/exception filter

coffeecard/CoffeeCard.Common/Errors/ApiException.cs

@@ -2,7 +2,6 @@
 
 namespace CoffeeCard.Common.Errors
 {
-    [Serializable]
     public class ApiException : Exception
     {
         public int StatusCode { get; }

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)
     {
     }
 }

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)

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)

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)

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)
         { }
     }
 }

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
 {
     /// <summary>
     /// Exception filter for handling API exceptions and returning appropriate HTTP status codes and error messages.
     /// </summary>
-    public class ApiExceptionFilter : ExceptionFilterAttribute
+    public class ApiExceptionFilter(IWebHostEnvironment environment, ILogger<ApiExceptionFilter> logger)
+        : ExceptionFilterAttribute
     {
+        private readonly ILogger<ApiExceptionFilter> _logger = logger;
+        private readonly IWebHostEnvironment _environment = environment;
+
         /// <inheritdoc/>
         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");
+            }
+        }
     }
 }

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<ApiExceptionFilter>();
                     options.Filters.Add(new ReadableBodyFilter());
                 })
                 .AddJsonOptions(options =>