Skip to content

refactor/exception filter #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion coffeecard/CoffeeCard.Common/Errors/ApiException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace CoffeeCard.Common.Errors
{
[Serializable]
public class ApiException : Exception
{
public int StatusCode { get; }
Expand Down
5 changes: 3 additions & 2 deletions coffeecard/CoffeeCard.Common/Errors/BadRequestException.cs
Original file line number Diff line number Diff line change
@@ -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)
{
}
}
2 changes: 0 additions & 2 deletions coffeecard/CoffeeCard.Common/Errors/ConflictException.cs
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
7 changes: 3 additions & 4 deletions coffeecard/CoffeeCard.Common/Errors/UnauthorizedException.cs
Original file line number Diff line number Diff line change
@@ -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)
{ }
}
}
83 changes: 49 additions & 34 deletions coffeecard/CoffeeCard.WebApi/Helpers/ApiExceptionFilter.cs
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
}
2 changes: 1 addition & 1 deletion coffeecard/CoffeeCard.WebApi/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
Loading