dotnet / aspnet-api-versioning

Provides a set of libraries which add service API versioning to ASP.NET Web API, OData with ASP.NET Web API, and ASP.NET Core.
MIT License
3.06k stars 704 forks source link

Default error response provider should return errors in ProblemDetails format #612

Closed Mubashwer closed 4 years ago

Mubashwer commented 4 years ago

Default error response provider should return errors in ProblemDetails format in compliance with RFC 7807.

ASP.NET Core has first-class support for ProblemDetails.

commonsensesoftware commented 4 years ago

I doubt the default error response provider would change in the near future as this would result in a breaking change for existing clients. It's worth considering in the next major version release however. The current, default implementation is based on the Microsoft REST Guidelines, which happens to be the same format that OData uses. That predates RFC 7807 by many years.

That being said, there's nothing wrong with creating a compliant Error Response Provider and swapping it out in the configuration. You should have no problem being able to create and configure that today. It's common enough that it's worth including as something that you can easily configure, but you need not wait on me to add support in your application today.

I'll track this for 4.2

NetTecture commented 4 years ago

@commonsensesoftware Where in the odata specs does it give any indication about how to return errors? All I found was "return http status code, the content is whatever you want to return", which opens the door for ProblemDetails.

commonsensesoftware commented 4 years ago

@NetTecture, you're probably looking at the protocol spec, but OData is divided into several specs. The OData error response is defined in §19 Error Response in the OData JSON Format v4.0 and OData Atom Format v4.0 specifications, both of which are linked to in the protocol spec.

I fully support using ProblemDetails, but making it the default is another matter because it changes client expectations. There is probably a larger discussion about when the default would change and how it should change. For example, if the OData libraries don't universally adopt ProblemDetails on .NET Core, then perhaps the OData implementation should continue it's present course for error response consistency.

These libraries also still support Web API, so I have to consider the impact there as well. I've long tried to maintain parity between the flavors to the greatest extend possible. A key reason is to support when service authors jump frameworks that things should continue to largely look and work the same way.

NetTecture commented 4 years ago

@commonsensesoftware THANKS - was totally not aware of that one. Damn. Funny is that basically I am writing odata code daily, and I have never seen a library that handles error repsonses - mostly they bubble up and I handle them further up. I was really not even aware of it. You are right ;)

commonsensesoftware commented 4 years ago

No problem.

Enter op-ed...

Sadly, the various implementations of OData have been unfairly lobbed in with the protocol itself. IMO, OData has defined many, many sensible guardrails in authoring web services with an emphasis on trying to be RESTful. This is often overlooked. As an OData advocate and having read the specs many times, there are a lot of valuable lessons I take from it, even for non-OData services. We continue to see new innovations such as ProblemDetails, JSON-LD, and so on, but OData defined most of those - in some form - years ago.

When you look at the Microsoft REST guidelines, anyone familiar with the OData protocol will recognize it being leveraged, even if it isn't cited. From my understanding, there are historical politics as to why OData didn't become the de facto protocol approach. Nevertheless, the best parts continue to proliferate, even if OData doesn't get all the credit it deserves.

Turnerj commented 4 years ago

This just caught me out with my own API and while I can roll my own easy enough, would it be worth while having it as a second implementation of IErrorResponseProvider in this library?

Something basically like:

public class Rfc7807ErrorResponseProvider : IErrorResponseProvider
{
    public IActionResult CreateResponse(ErrorResponseContext context)
    {
        return new ObjectResult(new ProblemDetails
        {
            Title = context.ErrorCode,
            Detail = context.Message,
            Status = context.StatusCode
        })
        {
            StatusCode = context.StatusCode
        };
    }
}

That way you have the default backwards compatibility for now while also having easy access to the way that Microsoft seems to indicate is the norm for WebAPIs.

Happy to do that as a PR and anything related to it (eg. tests) if that is helpful?