proudmonkey / AutoWrapper

A simple, yet customizable global exception handler and Http response wrapper for ASP.NET Core APIs.
MIT License
677 stars 82 forks source link

Schema for ApiResponse #6

Closed sondreb closed 5 years ago

sondreb commented 5 years ago

How much research on existing industry wide APIs and how they handle and return errors was done?

"responseException" doesn't really ring well in my head and can't remember seeing anyone else having the error like that.

Here are some results from some quick resource on the topic:

Microsoft Graph:

{
  "error": {
    "code": "invalidRange",
    "message": "Uploaded fragment overlaps with existing data.",
    "innerError": {
      "requestId": "request-id",
      "date": "date-time"
    }
  }
}

Salesforce example on StackExchange

public class ResponseWrapper{
    public list<Account> lstaccounts;
    public boolean isError ;
    public string errorCode;
    public integer statusCode;
       public ResponseWrapper(){
         lstaccounts = new list<Account>();
         isError= false;
    }
}

From an article on the topic:

image

How I design JSON API responses

{
    "status": "ok",
    "code": 200,
    "messages": [],
    "result": {
        "user": {
            "id": 123,
            "name": "shazow"
        }
    }
}

RESTful API Design Tips from Experience

This article uses texts for codes, such as: "EMAIL_ALREADY_EXISTS" and "FIELDS_VALIDATION_ERROR". This is a bit weird, if one really wants to copy the HTTP status codes (to make the response independent of HTTP protocol).

418: I'M A TEAPOT, AND OTHER BAD API RESPONSES - RESTFUL API DESIGN

This one argues against wrappers, but I'm leaning towards them due to a multitude of reasons.

RESTful API Design. Best Practices in a Nutshell.

// 400 Bad Request
{
  "errors": [
    {
      "status": 400,
      "detail": "Invalid state. Valid values are 'internal' or 'external'",
      "code": 352,
      "links": {
        "about": "http://www.domain.com/rest/errorcode/352"
      }
    }
  ]
}
proudmonkey commented 5 years ago

There you have the answer yourself. There's really no standard way to define API response schema and its names. This is true for Error/Exception schema too. For as long as you capture the most basic information that might be useful for clients to use, then there you hit the goal.

sondreb commented 5 years ago

Suggestion: Allow consumers of your library to provide an entity type that is used as response object, and use attributes to specify the property for where to put each of the values, such as error, status, code. That would make it flexible and avoid forking if the default doesn't fit everyone's need.

proudmonkey commented 5 years ago

@sondreb

This is noted. Thanks for the suggestion. I'll see what I can do to better improve this template.

mohitmunjal2801 commented 5 years ago

@proudmonkey
Can we make the APIResponse class generic or user-defined in AutoWrapper so that users can write their own APIResponse based on their needs and integrate with the AutoWrapper?

proudmonkey commented 5 years ago

@mohitmunjal2801 I'm working on it for v2. Thanks for your suggestion. :)

proudmonkey commented 5 years ago

@sondreb @mohitmunjal2801

Version 2 RC is now out here: https://www.nuget.org/packages/AutoWrapper.Core/2.0.0-rc

Release notes:

You can now map whatever names you want using the AutoWrapperPropertyMap attribute. For example:

Define your own response model for mapping:


public class MapResponseObject
{
    [AutoWrapperPropertyMap(Prop.StatusCode)]
    public int Code { get; set; }

    [AutoWrapperPropertyMap(Prop.Result)]
    public object Payload { get; set; }

    [AutoWrapperPropertyMap(Prop.ResponseException)]
    public object Error { get; set; }

    [AutoWrapperPropertyMap(Prop.ResponseException_ExceptionMessage)]
    public string Message { get; set; }

    [AutoWrapperPropertyMap(Prop.ResponseException_Details)]
    public string StackTrace { get; set; }

    [AutoWrapperPropertyMap(Prop.ResponseException_ReferenceErrorCode)]
    public string ErrorCode { get; set; }

    [AutoWrapperPropertyMap(Prop.ResponseException_ReferenceDocumentLink)]
    public string Link { get; set; }
}

Note that you are free to choose whatever field that you want to map. For example, if you only want to rename the default responseException name to error, then you can simply do:

public class MapResponseObject
{
    [AutoWrapperPropertyMap(Prop.ResponseException)]
    public object Error { get; set; }
}

At startup.cs, you can then pass the MapResponseObject in the middleware:

 app.UseApiResponseAndExceptionWrapper<MapResponseObject>();

That's it.

Here's a couple of examples:

Default success response:

{
    "message": "Request successful.",
    "isError": false,
    "result": [
        {
            "id": 7002,
            "firstName": "Vianne",
            "lastName": "Durano",
            "dateOfBirth": "2018-11-01T00:00:00"
        }
    ]
}

After mapping output:

{
    "message": "Request successful.",
    "isError": false,
    "payload": [
        {
            "id": 7002,
            "firstName": "Vianne",
            "lastName": "Durano",
            "dateOfBirth": "2018-11-01T00:00:00"
        }
    ]
}

The result field is now replaced with the payload field.

Default Error response:

For example, if you call:

throw new ApiException("Error blah",400,"Invalid param","http://blah.com/error/501");

It will result to something like this by default:

{
    "isError": true,
    "responseException": {
        "exceptionMessage": "Error blah",
        "referenceErrorCode": "Invalid param",
        "referenceDocumentLink": "http://blah.com/error/501"
    }
}

And here's the converted output after mapping:

{
    "isError": true,
    "error": {
        "message": "Error blah",
        "errorCode": "Invalid param",
        "link": "http://blah.com/error/501"
    }
}

You can also now define your own Error object and pass it to the ApiException method. For example, if you have the following Error model:

public class Error
{
    public string Message { get; set; }

    public string Code { get; set; }
    public InnerError InnerError { get; set; }

    public Error(string message, string code, InnerError inner)
    {
        this.Message = message;
        this.Code = code;
        this.InnerError = inner;
    }

}

public class InnerError
{
    public string RequestId { get; set; }
    public string Date { get; set; }

    public InnerError(string reqId, string reqDate)
    {
        this.RequestId = reqId;
        this.Date = reqDate;
    }
}

You can then throw an error like this:

throw new ApiException(new Error("An error blah.", "InvalidRange", new InnerError("Request ID", DateTime.Now.ToShortDateString())));

The response output will result to this:

{
    "isError": true,
    "error": {
        "message": "An error blah.",
        "code": "InvalidRange",
        "innerError": {
            "requestId": "Request ID",
            "date": "10/14/2019"
        }
    }
}

Thank you again for your valuable feedback. :)

-Vince

renatogbp commented 5 years ago

thank you for your great work @proudmonkey .

I was wondering if we can add more properties to this wrapper response. For example, I would like to add pagination properties. can we do something similar as you did for custom Error object?

proudmonkey commented 5 years ago

@renatogbp V2 should allow you to do that as you can now use your own custom Response Object. Once I'm done finalizing v2 for official release, I'll create an article about what's new in v2 for your reference. Hopefully within the week.

Thank you for your feedback and interest on this project. I appreciate it. :)

proudmonkey commented 5 years ago

@renatogbp - Here's an article I posted that highlight how you can use your own response schema to define your own properties: ASP.NET Core with AutoWrapper: Customizing the Default Response Output

@sondreb closing this now. Thank you all again for the feedback.