altmann / FluentResults

A generalised Result object implementation for .NET/C#
MIT License
2.07k stars 115 forks source link

Idea: Simple and good Integration with ASP.NET Core #149

Open altmann opened 2 years ago

altmann commented 2 years ago

If you are interested like the issue :) If you have input to this topic please leave a comment.

Todos

eltoncezar commented 2 years ago

I've created something very simple to deal with Results and HTTP Status Codes, please take a look: https://github.com/eltoncezar/FluentResults.StatusCodes

altmann commented 2 years ago

Cool. I was not aware that something similar already exists. I had a look at it - I think that the new aspnetcore integration package will be similar to your code but in total very different. I spiked already a little bit. :)

altmann commented 2 years ago

I pushed currently my first version of the aspnetcore integation. https://github.com/altmann/FluentResults/tree/features/AspNetCoreIntegration/src/FluentResults.Samples.WebHost/Controllers

Public APIs

Feedback welcome!

awaters-meltin commented 2 years ago

I recently went down the rabbit hole with this and come up with something somewhat similar to @eltoncezar w.r.t. custom error types (but errors just implementing an IStatusCodeError). But rather than using a Validate method or .ToActionResult() extension method I'm using a ResultFilter (I very quickly got tired of calling Validate and/or .ToActionResult() everywhere).

This might be a bit too much for the library's purposes, but I wanted all the Result info (stack traces etc.) in the Asp .NET standard ProblemDetails response (it's an internal project so better for logging).

Here's a code dump for ya lol - cleaned it up a bit, but still using various internal extension methods, but the general gist should be understandable:

builder.Services.AddControllers(options => { options.Filters.Add<ResultTypeFilter>(); })

// ...

internal class ResultTypeFilter : IAsyncResultFilter
{
    public async Task OnResultExecutionAsync(ResultExecutingContext context, ResultExecutionDelegate next)
    {
        if (context.Result is ObjectResult { Value: IResultBase result })
            context.Result = ToActionResult(result);

        await next();
    }

    private static ActionResult ToActionResult(IResultBase result)
    {
        if (result.IsSuccess)
            return new OkObjectResult((result as IResult<object>)?.Value);

        IEnumerable<string> ExceptionToStringList(Exception e)
            => new[] { $"{e.GetType().Name}: {e.Message}" }.Concat((e.StackTrace ?? "").Split(Environment.NewLine));

        var awsException = result.GetException<AmazonServiceException>();
        var statusCode = result.Errors.OfType<IStatusCodeError>().FirstOrDefault()?.StatusCode ??
                         (int?) awsException?.StatusCode;

        var e = awsException ?? result.GetException();
        var problemDetails = new ProblemDetails
        {
            Title = e?.Message ?? result.Errors.FirstOrDefault()?.Message ?? "Unknown Error",
            Status = statusCode ?? (e == null ? 400 : 500),
            Detail = result.Reasons
                           .Select(reason => reason.Message)
                           .JoinToString(sep: ", ", prefix: "[", suffix: "]", affixEmpty: false, affixSingle: false),
            Extensions =
            {
                ["errors"] =
                    result.Errors
                          .OfType<Error>() // using "Error" type for automatic JSON conversion
                          .Select(
                              err =>
                              {
                                  if (err is IExceptionalError exErr)
                                      err.Metadata["_stackTrace"] = ExceptionToStringList(exErr.Exception);

                                  return err;
                              }
                          )
            }
        };

        return new ObjectResult(problemDetails) { StatusCode = problemDetails.Status };
    }
}

Then my controller endpoints are:

    [HttpPost, Route("do/something")]
    public async Task<Result<SomeType>> DoSomething([FromBody] SomeArgs args)
        => await _store.DoSomething(args.Arg1, args.Arg2);

The one downside to this was the Swagger generator needed to be tweaked to handle the Result<T> types a bit better (using a IOperationFilter on SwaggerGen), but other than that it's been working quite well.

altmann commented 2 years ago

@awaters-meltin Thank you for your comment. I also played around a bit with an aspnetcore filter but then I deleted the code.

This biggest disadvantage of filters is the intransparency - the controller api is lying - in background the return value (=result object) of the controller is translated into something else. You write it already, swagger need because of that some custom configuration to work correctly. The benefit is that you don't have to write ToActionResult() again and again.

Do other people have some opinions about this topic?

altmann commented 2 years ago

Is the usage of the ProblemDetails class a MUST-HAVE in the default logic of FluentResults.Extensions.AspNetCore? For sure the default logic can be overwritten.

What is the experience with ProblemDetails and swagger in combination?

awaters-meltin commented 2 years ago

If they're trying to follow the official RFCs etc. (https://www.rfc-editor.org/rfc/rfc7807) then they'd want the ProblemDetails, but some companies love to roll their own error format (of course lol).

So I don't know about a "must have", but I don't think there's a reason not to. 🤷

altmann commented 2 years ago

Haha, in my current project in the job we also have a custom error model. ;)

I will have a look at the ProblemDetails class in the next days and I hope I can release a first preview package of the aspnetcore integration until the end of october. Thanks for your comment.

thisispaulsmith commented 2 years ago

This will be an excellent addition.

It would be good if ToActionResult on Result<T> returned ActionResult<T> rather than ActionResult.

altmann commented 1 year ago

This will be an excellent addition.

It would be good if ToActionResult on Result<T> returned ActionResult<T> rather than ActionResult.

I tried it but then it is not possible to change the resulting dto in the background by using the extension points.

altmann commented 1 year ago

Thanks for all the feedback.

Because I'am out of time I released today the v0.1.0. Its on nuget, so you should be able to use it. If you use it read the documentation

If you have feedback to the functionality/api or the documentation please let me know.

eltoncezar commented 1 year ago

@altmann would you give us permission to edit wiki pages?

altmann commented 1 year ago

I don't know how to so it. Maybe you have to clone the wiki repo and make a pr https://github.com/altmann/FluentResults.wiki.git

saborrie commented 1 year ago

Does this also cover minimal APIs too? Microsoft does not use ActionResults for those, instead you are supposed to return a Microsoft.AspNetCore.Http.IResult, usually from the factory Microsoft.AspNetCore.Http.Results, e.g.

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.MapGet("/", () => Results.BadRequest("Some error message."));

app.Run();
billhong-just commented 12 months ago

Does this also cover minimal APIs too? Microsoft does not use ActionResults for those, instead you are supposed to return a Microsoft.AspNetCore.Http.IResult, usually from the factory Microsoft.AspNetCore.Http.Results, e.g.

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.MapGet("/", () => Results.BadRequest("Some error message."));

app.Run();

@altmann Do you have any thoughts on this?

angusbreno commented 10 months ago

Hey guys take a look:

https://github.com/ElysiumLabs/FluentProblemDetails/

angusbreno commented 6 months ago

Does this also cover minimal APIs too?

@billhong-just take a look at the sample, minimal apis

https://github.com/ElysiumLabs/FluentResults.Extensions.AspNetCore