C# – Mapping exceptions to error response

cdesignexceptions

Imagine a program which exposes a REST service/gRPC service/whatever service, which uses several 3rd party libraries. These libraries can of course throw exceptions if something goes wrong, for example if the user tried to access something which he isn't allowed to.

The problem here is that it isn't possible to check beforehand if the request is correct or if the user has enough rights. So we send the request to the 3rd party library and get an exception.

Is it good practice to catch these exceptions at the top level and map them to status codes like this?

var statusCode = HttpStatusCode.InternalServerError;
if (ex is ArgumentException || ex is ResourceDoesntExistException)
    statusCode = HttpStatusCode.BadRequest;
else if (ex is UnauthorizedAccessException)
    statusCode = HttpStatusCode.Forbidden;
else if (ex is CustomerNotFoundException)
    statusCode = HttpStatusCode.NotFound;

and then return the status code with an error object:

return new ErrorResponse
{
    Error = new ErrorDescription
    {
        Message = ex.Message,
        Type = ex.GetType().Name
    }
};

Advantages I see by using this approach:

  • In the program, we don't have to care if we expose a REST service or a SOAP service or whatever. Simply throw an exception, it will be handled correctly later.
  • The caller gets enough and correct information if something goes wrong (as long as the exceptions have meaningful names and information).
  • Logging can also be centralised. All unhandled exceptions will be logged right where we convert them into error responses.

Disadvantages:

  • It feels a little "hacky".
  • ?

What is the correct way to do this?

Edit: Since it was requested, here is what I do for gRPC services, where the error mapping is quite similar:

private RpcException GenerateRpcException(Exception ex)
{
    var statusCode = StatusCode.Unknown;

    if (ex is ArgumentException || ex is ResourceDoesntExistException)
        statusCode = StatusCode.InvalidArgument;
    else if (ex is UnauthorizedAccessException)
        statusCode = StatusCode.PermissionDenied;
    else if (ex is CustomerNotFoundException)
        statusCode = StatusCode.NotFound;

    var status = new Status(statusCode, ex.Message);
    var exceptionName = ex.GetType().Name;

    var rpcMetadata = new Metadata
    {
        { "exception_name", exceptionName }
    };
    return new RpcException(status, rpcMetadata);
}

Best Answer

There is nothing wrong with normalizing exceptions and handling them properly. Your class ErrorResponse seems to be the right approach for handling a call to your REST service.

If there is something here which could be improved, it's a static error handling approach. The list of possible exceptions/error codes could easily grow tomorrow and your if else is going to grow to be monstrous at this rate.

Consider creating a ErrorManager class which holds a list of ErrorHandler interfaces with two methods: bool CanHandle(Exception) and ErrorResponse Handle(Exception)

Your ErrorManager when given an Exception will stupidly check each registered ErrorHandler with a call to CanHandle with the exception. When it returns true, the check stops and you return your ErrorHandler from calling your handler's Handle method. You then create an ErrorHandler for each possible ErrorResponse (and not necessarily for each exception).

This dynamic approach gives you the flexibility to programmatically register the ErrorHandler classes, or load it from a database if you so choose. More importantly than this, if you decide to programmatically register the ErrorHandler classes now, should you decide to change how you load these handlers, you can do so with little to no impact on your program as is. In essence, you're future-proofing your error handling.

A word to the wise: have a plan for when you find no handler for an exception. This is probably best left to be a HttpStatusCode.InternalServerError. While you could create a handler which "always handles" exceptions, I would advise against it, since technically we're talking about managing known possible exceptions and any exception which isn't expected is an unknown exception and should most certainly be red flagged and not merely treated like any other error.

Related Topic