Architectural discussion

Hello!

We talked on reddit a while back, I told you that I would share the error contract model that I use. I have shared it below. In my opinion my error contract model can certainly be improved with error codes and types of validation results. I have added some notes and comments to my code.

Im looking forward to diving in and reviewing your code today! Thanks for this cool library!


public class ValidationErrorResponse
{
    private ValidationErrorResponse()
    {
        this.IsEmpty = true;
        this.PropertyErrors = new List<PropertyError>();
        this.HttpStatus = string.Empty;
        this.HttpStatusCode = -1;
        this.ErrorCount = -1;
    }

    // TODO: Improve this
    public ValidationErrorResponse(Result<string> validationResult)
    {
        if (validationResult.IsSuccess)
        {
            this.IsEmpty = true;
            this.PropertyErrors = new List<PropertyError>();
            this.HttpStatus = string.Empty;
            this.HttpStatusCode = -1;
            this.ErrorCount = -1;
        }
        else
        {
            var httpStatusCode = System.Net.HttpStatusCode.BadRequest;
            this.IsEmpty = false;
            this.HttpStatus = httpStatusCode.ToString()!;
            this.HttpStatusCode = (int)httpStatusCode;
            this.ErrorCount = 1;
            this.PropertyErrors = validationResult.Errors.Select(_ => new PropertyError(_))
                .ToList();
            this.PropertyErrors = new List<PropertyError>();

        }
    }

    // TODO: Improve this
    public ValidationErrorResponse(Result<string> validationResult, IAeonicResultError error)
    {
        if (validationResult.IsSuccess)
        {
            this.IsEmpty = true;
            this.PropertyErrors = new List<PropertyError>();
            this.HttpStatus = string.Empty;
            this.HttpStatusCode = -1;
            this.ErrorCount = -1;
        }
        else
        {
            var httpStatusCode = System.Net.HttpStatusCode.BadRequest;
            this.IsEmpty = false;
            this.HttpStatus = httpStatusCode.ToString()!;
            this.HttpStatusCode = (int)httpStatusCode;
            this.ErrorCount = 1;

            var headerError = new PropertyError(error);
            this.PropertyErrors = new List<PropertyError>();
            this.PropertyErrors.Add(headerError);
        }
    }

    public ValidationErrorResponse(HttpStatusCode? httpStatusCode, ValidationResult? validationResult)
    {
        if (httpStatusCode == null)
        {
            httpStatusCode = System.Net.HttpStatusCode.BadRequest;
        }

        this.IsEmpty = false;
        this.HttpStatus = httpStatusCode.ToString()!;
        this.HttpStatusCode = (int)httpStatusCode;

        if (validationResult == null)
        {
            this.ErrorCount = -1;
            this.PropertyErrors = new List<PropertyError>();
        }
        else
        {
            if (validationResult.IsValid)
            {
                throw new InvalidDataException("Only invalid validation results can be used to create Validation Error Responses.  Instead we saw 'Valid' and we expected 'InValid");
            }

            if (validationResult.Errors == null)
            {
                throw new NullReferenceException("The validation error list cannot be null");
            }

            this.ErrorCount = validationResult.Errors.Count;
            this.PropertyErrors = validationResult.Errors.Select(_ => new PropertyError(_))
                .ToList();
        }
    }

    [JsonIgnore]

    public bool IsFailed => this.IsEmpty == false;

    [JsonIgnore]

    public bool IsSuccess => this.IsEmpty;

    [JsonIgnore]
    public bool IsEmpty { get; }

    [OpenApiProperty(Description = "The returned Http Status.", Nullable = false)]
    public string HttpStatus { get; }

    [OpenApiProperty(Description = "The returned Http Status Code.", Nullable = false)]
    public int HttpStatusCode { get; }

    [OpenApiProperty(Description = "The number of validation errors which occurred on the request object.", Nullable = false)]
    public int ErrorCount { get; }

    public ICollection<PropertyError> PropertyErrors { get; set; }

    #region Nested type: PropertyError

    public class PropertyError
    {

        public PropertyError(IError error)
        {
            this.PropertyName = error.Message;
            this.AttemptedValue = error.Message;
            this.ErrorMessage = error.Message;
            this.ErrorCode = "101";
        }
        public PropertyError(IAeonicResultError error)
        {
            this.PropertyName = error.ExpectedHeaderValue;
            this.AttemptedValue = "Value was missing";
            this.ErrorCode = "101";
            this.ErrorMessage = error.ErrorMessage;
        }

        public PropertyError(ValidationFailure validationFailure)
        {
            this.PropertyName = validationFailure.PropertyName;
            this.AttemptedValue = validationFailure.AttemptedValue;
            this.ErrorCode = validationFailure.ErrorCode;
            this.ErrorMessage = validationFailure.ErrorMessage;
        }

        [OpenApiProperty(Description = "The request object property in which the property validation error occurred.", Nullable = false)]
        public string PropertyName { get; }

        [OpenApiProperty(Description = "The associated  error code with the property validation error.", Nullable = false)]
        public string ErrorCode { get; set; }

        [OpenApiProperty(Description = "The value contained in the request object which incurred the error.", Nullable = false)]
        public object AttemptedValue { get; set; }

        [OpenApiProperty(Description = "The associated error message with the property validation error.", Nullable = false)]
        public string ErrorMessage { get; set; }
    }

    #endregion

    public static ValidationErrorResponse AsEmpty()
    {
        return new ValidationErrorResponse();
    }

    public static ValidationErrorResponse AsSuccess()
    {
        return new ValidationErrorResponse();
    }
}

@jeffward01 thank you for your feedback.

First of all, after looking at the code snippet, I would suggest that you consider switching from your existing response contract to a ProblemDetails, as ProblemDetails is becoming the industry standard for error response format. The main idea behind ProblemDetails format is to eliminate the need to adapt to every individual API implementing their own error contract by making it uniform.

If you do decide to switch, I have a snippet that I used when validating input with FluentValidation. I would suggest you take a look at it and consider adapting this approach to your needs. This approach uses Interceptor pattern provided by FluentValidation:

public class ValidatorInterceptor : IValidatorInterceptor
{
    public IValidationContext BeforeAspNetValidation(ActionContext actionContext, IValidationContext commonContext)
    {
        return commonContext;
    }

    public ValidationResult AfterAspNetValidation(ActionContext actionContext, IValidationContext validationContext,
        ValidationResult result)
    {
        if (!result.IsValid) throw ApiException.BadRequest(result.Errors.First().ErrorMessage);
    }
}

(the main difference that I see here is that this snippet only returns the first error, while yours returns the whole list. But you can update it so that it returns the whole list like in your approach)

Pls do not hesitate to post updates with your opinions on all this.

I do think that integrating this package with input validation in some way, shape or form is a topic worth at least thinking about. Maybe it would be worth implementing this code that I posted above as a separate nuget package, say something like BitzArt.ApiExceptions.FluentValidation or even BitzArt.ApiExceptions.AspNetCore.FluentValidation as this integration will be specifically for AspNetCore.

But I don't feel like this is urgently necessary as you can already implement any validation integration "on top" of the current package (see snippet above). I would not want this package to take too much unneseccary responsibilities if those can be shifted onto specific implementations.

@jeffward01 With your permission, I am renaming this Issue as I think this would be a good place to discuss integration of this package with validation in general.

Hello!

Thanks for your quick response, I appreciate it alot.

Maybe it would be worth implementing this code that I posted above as a separate nuget package, say something like BitzArt.ApiExceptions.FluentValidation or even BitzArt.ApiExceptions.AspNetCore.FluentValidation as this integration will be specifically for AspNetCore.

I agree with this very much, I think its a great suggestion and much cleaner than combining the two libraries.

I would suggest that you consider switching from your existing response contract to a ProblemDetails, as ProblemDetails is becoming the industry standard for error response format. The main idea behind ProblemDetails format is to eliminate the need to adapt to every individual API implementing their own error contract by making it uniform.

I was not aware of this, this makes alot of sense. Thanks for mentioning this, I find that this is a difficult topic to research, I really appreciate the tip and advice. I will be refactoring this now

Problem I am trying to solve: Note: I have my questions in summary below

Regarding your library, what do you see as the best use case for this library? Such as, would a user use your library 'instead of FluentResults, or perhaps in combination with FluentResults?

I think the benefit of your library is that it is targeted towards HTTP responses, while FluentResults is targeted at general responses (it can be used for an internal service method for example). I think that if these two libraries were merged together and perhaps some mapping was done between the IError in FluentResults and your ProblemDetail, the library could be easier integrated with 'deeper' layers of the application. If that makes sense.

You obviously know much more about HTTP Response handling than I do. My main issue I am trying to solve is:

I want to create uniform response messages (errors and responses will look the same, relatively speaking)

Example: https://rapidapi.com/wirefreethought/api/geodb-cities

Response:

{
  "data": [
    {
      "id": 3386644,
      "wikiDataId": "Q3694483",
      "name": "Ab Band District",
      "country": "Afghanistan",
      "countryCode": "AF",
      "region": "Ghazni",
      "regionCode": "GHA",
      "latitude": 32.983,
      "longitude": 67.967,
      "population": 0
    },
    {
      "id": 3228129,
      "wikiDataId": "Q1650529",
      "name": "Achin",
      "country": "Afghanistan",
      "countryCode": "AF",
      "region": "Nangarhar",
      "regionCode": "NAN",
      "latitude": 34.0894,
      "longitude": 70.683,
      "population": 0
    },
    {
      "id": 3223788,
      "wikiDataId": "Q2674014",
      "name": "Ajristan District",
      "country": "Afghanistan",
      "countryCode": "AF",
      "region": "Ghazni",
      "regionCode": "GHA",
      "latitude": 33.466962,
      "longitude": 67.238846,
      "population": 0
    },
    {
      "id": 3382471,
      "wikiDataId": "Q2830672",
      "name": "Alasay",
      "country": "Afghanistan",
      "countryCode": "AF",
      "region": "Kapisa",
      "regionCode": "KAP",
      "latitude": 34.925555555,
      "longitude": 69.809166666,
      "population": 0
    },
    {
      "id": 3348637,
      "wikiDataId": "Q1229725",
      "name": "Alingar",
      "country": "Afghanistan",
      "countryCode": "AF",
      "region": "Laghman",
      "regionCode": "LAG",
      "latitude": 34.8347,
      "longitude": 70.3603,
      "population": 0
    }
  ],
  "links": [
    {
      "rel": "first",
      "href": "/v1/geo/cities?offset=0&limit=5"
    },
    {
      "rel": "next",
      "href": "/v1/geo/cities?offset=5&limit=5"
    },
    {
      "rel": "last",
      "href": "/v1/geo/cities?offset=68820&limit=5"
    }
  ],
  "metadata": {
    "currentOffset": 0,
    "totalCount": 68823
  }
}

Error Response:


{
  "errors": [
    {
      "code": "PARAM_INVALID",
      "message": "Param 'maxPopulation[-1]' has invalid value. Must be at least 0"
    }
  ]
}

This is your response contract structure:

The goal I am trying to achieve, is a simple 'plugin library' that I can use on all my API's that will allow my API's to have a uniform structure without performing much boilerplate code.

Of course the uniform structure does not need to look like above, its only examples.

Questions:

What do you think about your library vs FluentResults (Can the compliment eachother and should we commit some integrations for it, I can help with this if yes - would love to know your thoughts)
What do you think about the Uniform Error and Response problem I am trying to solve?

Thanks!

Here is an example of results JSON from 'FluentResults':

@jeffward01

Regarding your library, what do you see as the best use case for this library? Such as, would a user use your library 'instead of FluentResults, or perhaps in combination with FluentResults?

These are kind of competing approaches, although can be used in combination. I would suggest you to watch this video to get a better understanding regarding the differences between the approaches. The current library implements the exceptions approach, and provides the framework for a more advanced one, than what is shown in the video.

To put it simply, the current approach is:

A special type of exception (that derives from ApiExceptionBase) is thrown somewhere in the app.
This exception itself acts as a payload, containing all the information necessary for the handler (status code that it suggests, error message, error details, and any other info).
This exception then bubbles up the current stack.
At the very top of the stack it gets caught and handled by some kind of handler. The package BitzArt.ApiExceptions.AspNetCore implements a default handler for you to use, but you can skip using it altogether and can create your own handler if you want it to have some other behavior.

I think the benefit of your library is that it is targeted towards HTTP responses, while FluentResults is targeted at general responses (it can be used for an internal service method for example)

The main difference that I see is that:

With approaches like FluentResults or ErrorOr you have to validate your results on every layer of your app (Controller calls Service, then validates the result, Service calls Repository, then validates the result, Repository calls some Utility, then validates the result)
While with the exceptions approach, your exception automatically bubbles up the stack, and then gets caught at the very top by the handler, and no intermediary actions need to be done about it. It is kind of a 'glorified' go-to.

To clear any potential misconseptions, this package in general is not necessarily targeted toward http. The BitzArt.ApiExceptions.AspNetCore indeed is, but the base package BitzArt.ApiExceptions has nothing to do with http. This library has several layers, and the base layer does not have any http dependencies. This is why the ApiExceptionBase does not have a property called HttpStatusCode but just StatusCode, and the idea behind this is that this StatusCode can be then converted either into HttpStatusCode or into something else.

This allows you to use the package in any other environments, not just Asp.Net Core HTTP WebAPI. (Console apps, Blazor, WPF, Xamarin, MAUI, Message Queue handlers, etc.) Obviously, every environment will have it's own way of handling these exceptions. Http is just one of them (although the only one currently implemented)

@jeffward01

What do you think about the Uniform Error and Response problem I am trying to solve?

Not sure what the end goal is, but I would suggest against the idea of having both the error response and the proper result response having the same uniform structure. The HttpResponse already has an HttpStatusCode, which already indicates if the request processed properly or failed. I would suggest having a ProblemDetails for your errors (which the package already does for you, you just need to add a handler in your program.cs and then throw some exceptions) and just the Data (from your current response model) as your results. At least this is what I am doing in my projects.

Not sure what else to suggest though, as I am not sure what exactly is the problem that you are trying to solve.

BitzArt / ApiExceptions

Architectural discussion #6