Skip to content
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

Question: Best way to map failure results to ActionResults? #484

Open
DerStimmler opened this issue Jan 12, 2023 · 3 comments
Open

Question: Best way to map failure results to ActionResults? #484

DerStimmler opened this issue Jan 12, 2023 · 3 comments

Comments

@DerStimmler
Copy link
Contributor

DerStimmler commented Jan 12, 2023

Hello, I was asking myself how I can improve my error handling in a web api with CSharpFunctionalExtensions. Since there is the possibility to use an error object in failure results instead of a simple string, I think there are several opportunities for improvement available. For example, you can provide additional info like the HTTP status code beside the error message based on the context.

Currently, I'm using a custom error object for failure results which contains a HTTP status code, error title, error messages and a trace id. Result<T,HttpError>

public class HttpError {
  public int StatusCode { get; }
  public string Title { get; }
  public List<string> Messages { get; }
  public Guid TraceId { get; }

  ...
}

Then in my controller I call an extension method .Envelope() on the result, which maps it to a ObjectResult respectively ActionResult. When the result is a success the status code is 200 by default but can be adjusted by passing the code to the .Envelope() method.

  internal static ActionResult<T> Envelope<T>(this Result<T, HttpError> result, int successStatusCode = 200) =>
    result.IsSuccess
      ? new ObjectResult(result.Value) { StatusCode = successStatusCode }
      : new ObjectResult(result.Error) { StatusCode = result.Error.StatusCode };

This solution works fine in the beginning, but it soon gets to its limits, especially when your action method on the controller can return different types. Then you may need a method which returns a non-generic ActionResult like this:

  internal static ActionResult EnvelopeNonGeneric<T>(this Result<T, HttpError> result, int successStatusCode = 200) =>
    result.IsSuccess
      ? new ObjectResult(result.Value) { StatusCode = successStatusCode }
      : new ObjectResult(result.Error) { StatusCode = result.Error.StatusCode };

Same problem when you want to return a file:

internal static ActionResult EnvelopeFile(this Result<byte[], HttpError> result, string mediaType) =>
    result.IsSuccess
      ? new FileContentResult(result.Value, mediaType)
      : new ObjectResult(result.Error) { StatusCode = result.Error.StatusCode };

Or you want to return a CreatedAtActionResult to properly populate the location header when creating a resource:

  internal static ActionResult<T> EnvelopeAsCreatedAtAction<T>(this Result<T, HttpError> result, [AspMvcAction] string? actionName,
    [AspMvcController] string? controllerName, Func<T, Guid> id) =>
    result.IsSuccess
      ? new CreatedAtActionResult(actionName, controllerName, new { id = id(result.Value) }, result.Value)
      : new ObjectResult(result.Error) { StatusCode = result.Error.StatusCode };

So you have to implement many different methods for each use case.

Does anybody have a better idea on how to map failure results to ActionResults?

@vkhorikov
Copy link
Owner

vkhorikov commented Jan 20, 2023

All these CreatedAtActionResult etc create the same object in the end, which differs only by the HTTP code. Here's an example of mapping that I normally use:

protected new IActionResult Ok()
        {
            return base.Ok(Envelope.Ok());
        }

        protected IActionResult Ok<T>(T result)
        {
            return base.Ok(Envelope.Ok(result));
        }

        protected IActionResult Error(Error error)
        {
            _logger.LogInformation($"Error: {error}");
            return BadRequest(Envelope.Error(error));
        }

        protected IActionResult FromResult(Result result)
        {
            return result.IsSuccess ? Ok() : Error(result.Error);
        }

    public sealed class Envelope
    {
        public object Result { get; }
        public string ErrorMessage { get; }
        public string ErrorCode { get; }
        public DateTime TimeGenerated { get; }
        
        private Envelope(object result, Error error)
        {
            Result = result;
            ErrorMessage = error?.Message;
            ErrorCode = error?.Code;
            TimeGenerated = DateTime.Now;
        }

        public static Envelope Ok(object result = null)
        {
            return new Envelope(result, null);
        }

        public static Envelope Error(Error error)
        {
            return new Envelope(null, error);
        }
    }

You can also use the base ObjectResult instead of Ok and BadRequest and specify the http code directly.

@DerStimmler
Copy link
Contributor Author

Thanks for your example.
This works fine if indeed only the status code differs. But actually the different Result types I'm returning have further differences.
E.g. the CreatedAtActionResult writes the route where the new resource can be fetched to the location header in the HTTP response.
The FileContentResult sets the correct content-type header based on the provided media type.

That's why I think the mapping is not that easy, as in your reply.

@vkhorikov
Copy link
Owner

For MVC (not just API) responses, there could be additional properties needed, indeed. You can keep helper methods like Ok in my example in the base controller class and use them as necessary from the deriving controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants