dotnet-starter-kit icon indicating copy to clipboard operation
dotnet-starter-kit copied to clipboard

Generalize Api responses

Open CanMehmetK opened this issue 2 years ago • 7 comments

Is your feature request related to a problem? Please describe. For Front End (specially angular) getting different result models is a problem

Describe the solution you'd like There should be a general result to standardize all reponses.. Simple PaginationResponse does the job but some result may not require to return data.

Describe alternatives you've considered

Something like this classes and also changes needed in Exception middleware accordingly.

    public class PaginationResponse
    {
        [JsonConverter(typeof(JsonStringEnumConverter))]
        public toast Toast { get; set; }
        public bool HasError { get; set; } = false;

        public string? MessageTitle { get; set; }
        public string? Message { get; set; }
    }

    public class PaginationResponse<T> : PaginationResponse
    {

        public PaginationResponse(List<T> data, int count, int page, int pageSize)
        {
            Data = data;
            CurrentPage = page;
            PageSize = pageSize;
            TotalPages = (int)Math.Ceiling(count / (double)pageSize);
            TotalCount = count;
        }
        public PaginationResponse(List<T> data, toast toast, string message, bool hasError = false)
        {
            Data = data;
            Toast = toast;
            Message = message;
            HasError = hasError;
        }

        public PaginationResponse(string message)
        {
            Message = message;
        }

        public PaginationResponse(toast toast, string message, bool hasError = false)
        {
            Toast = toast;
            Message = message;
            HasError = hasError;
        }

        public List<T>? Data { get; set; }

        public int? CurrentPage { get; set; }

        public int? TotalPages { get; set; }

        public int? TotalCount { get; set; }

        public int? PageSize { get; set; }

        public bool? HasPreviousPage => CurrentPage > 1;

        public bool? HasNextPage => CurrentPage < TotalPages;
    }

CanMehmetK avatar Apr 18 '22 11:04 CanMehmetK

Has anyone managed to generalize error results?

Currently exception middleware produces this response:

{
    "messages": [
        "Some error message"
    ],
    "source": "FSH.WebApi.Application.Catalog.Brands.CreateBrandRequestHandler+<Handle>d__5",
    "exception": "The exception message",
    "errorId": "d5a86f52-1859-43f4-a4bc-2c862dec5840",
    "supportMessage": "Provide the ErrorId to the support team for further analysis.",
    "statusCode": 404
}

But this does not catch fluent validation ValidationException which produces this response:

{
    "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
    "title": "One or more validation errors occurred.",
    "status": 400,
    "traceId": "00-e3eb4e0c174ebed8898e6a3df9c26f2d-106e54ff47355f70-00",
    "errors": {
        "Name": [
            "The Name field is required.",
            "'Name' must not be empty."
        ]
    }
}

It would be nice to have validation error transformed to the ErrorResult

snax4a avatar Jun 06 '22 17:06 snax4a

@snax4a

I have actually eliminated the ErrorResult and ExceptionMiddleware, and replaced it with a combination of ProblemDetails Middleware from https://github.com/khellang/Middleware and a "SerilogLoggingActionFilter" (Mvc Actionfilter) to add some action properties and the tenant to the serilog logcontext.

I was planning to make a pr for it, but my codebase has diverged a lot at this point, so making pr's is taking longer. Also I'm still very busy with my daytime job for the foreseeable future and really need to spend the free time I have left AFK ;-)

I can post some code snippits though:

// relevant part of program.cs in the Host project (could also be integrated into infrastructure)
    ...
    builder.Services.AddProblemDetails(options =>
    {
        options.ValidationProblemStatusCode = 400;
        options.Map<Exception>(ex => FSHProblemDetails.Create(ex));
    });

    builder.Services
        .AddControllers(options => options
                .Filters.Add<SerilogLoggingActionFilter>()) // Adds some action properties and the tenant to the log context
            .AddFluentValidation()
            .AddProblemDetailsConventions(); // Adds MVC conventions to work better with the ProblemDetails middleware.

    builder.Services.AddInfrastructure(builder.Configuration);
    builder.Services.AddApplication();

    var app = builder.Build();

    await app.Services.InitializeDatabasesAsync();

    app.UseProblemDetails();
    ...
public class FSHProblemDetails : StatusCodeProblemDetails
{
    protected FSHProblemDetails(int statusCode, Exception ex)
        : base(statusCode)
    {
        Detail = ex.Message;
        if (ex.TargetSite?.DeclaringType?.FullName is { } source)
        {
            Extensions["Source"] = source;
        }
    }

    public static ProblemDetails Create(Exception exception)
    {
        var statusCode = exception is CustomException customException
            ? customException.StatusCode
            : HttpStatusCode.InternalServerError;

        return new FSHProblemDetails((int)statusCode, exception);
    }
}

public class SerilogLoggingActionFilter : IActionFilter
{
    private readonly LoggerSettings _loggerSettings;
    private readonly IDiagnosticContext _diagnosticContext;

    public SerilogLoggingActionFilter(IOptions<LoggerSettings> loggerSettings, IDiagnosticContext diagnosticContext) =>
        (_loggerSettings, _diagnosticContext) = (loggerSettings.Value, diagnosticContext);

    public void OnActionExecuting(ActionExecutingContext context)
    {
        if (_loggerSettings.EnableRequestLogging)
        {
            _diagnosticContext.Set("RouteData", context.ActionDescriptor.RouteValues);
            _diagnosticContext.Set("ActionName", context.ActionDescriptor.DisplayName);
            _diagnosticContext.Set("ActionId", context.ActionDescriptor.Id);
            _diagnosticContext.Set("ValidationState", context.ModelState.IsValid);
            if (context.HttpContext.GetMultiTenantContext<FSHTenantInfo>()?.TenantInfo?.Identifier is { } tenant)
            {
                _diagnosticContext.Set("Tenant", tenant);
            }
        }
    }

    // Required by the interface
    public void OnActionExecuted(ActionExecutedContext context)
    {
    }
}

There's probably more to it... but that's what I can come up with in short notice ;-)

fretje avatar Jun 11 '22 13:06 fretje

Hey can i take this issue? and work on it?

gkhedekar5758 avatar Jun 16 '22 14:06 gkhedekar5758

@gkhedekar5758 sure! I'll gladly review a pr for this ;-)

fretje avatar Jun 16 '22 17:06 fretje

@fretje thanks a lot sir, I am new to git, I would like to understand more about the problem/requirement here. how can i reach out to someone for clarification. @snax4a @CanMehmetK

gkhedekar5758 avatar Jun 17 '22 04:06 gkhedekar5758

@gkhedekar5758 Not sure this is the best issue to get started...

As I understand it, we're actually still in the "design" phase here... If I read it right, @snax4a would like to have all "error" responses as "ErrorResult" (which is a custom object we "rolled our own"), while my suggestion is the other way around... having all "error" responses as "ProblemDetails" (which is the more "standard" way for handling errors in rest api's and for which there actually exists infrastructure within aspnetcore mvc itself).

Actually, if you want to have the error messages shown with their respective controls in the blazor front-end you need to have a problemdetails response (which includes de propertyname with its individual validation messages). If that would be "transformed" to an "ErrorResult", there wouldn't be a way to map the error messages to their respective properties anymore...

fretje avatar Jun 17 '22 09:06 fretje

https://github.com/fullstackhero/dotnet-webapi-boilerplate/pull/693 Since we are with this Pr, could we say that the error messages for validation and exceptions could be normalized?

frankyjquintero avatar Jul 06 '22 05:07 frankyjquintero

AddFluentValidation

fixed this by adding validation behavior and catching it at the middleware

image

iammukeshm avatar Apr 07 '23 02:04 iammukeshm

Frontend can deserialize the response into ErrorResult if the status code is 400, 500, 404 or similar.

iammukeshm avatar Apr 07 '23 02:04 iammukeshm