ConsumerDataStandardsAustralia / standards

Work space for data standards development in Australia under the Consumer Data Right regime
Other
321 stars 56 forks source link

Decision Proposal 011 - Error Handling #11

Closed JamesMBligh closed 6 years ago

JamesMBligh commented 6 years ago

This decision proposal outlines a recommendation for the usage of HTTP error codes in the API standard.

Feedback is now open for this proposal. Feedback is planned to be closed on the 14th September. Decision Proposal 011 - Error Handling.pdf

JamesMBligh commented 6 years ago

In this proposal I used 501 Not Implemented as the response to an unsupported end point version. I was torn between 501 and 404 Resource Not Found. I'm interested in what people think on this point.

-JB-

bfarrelly commented 6 years ago

@JamesMBligh I would suggest that 501 should be reserved for early/partial implementation of the standard in which all endpoints may not yet be implemented, where a 501 response would indicate future capability. I don't see there being a need to treat a "Not implemented" endpoint and a "Not implemented" version of the standard any differently.

404 is useful for when an endpoint is "Not found" due to a malformed or non-existent URI. 501 lets consumers know that they have found the right place (i.e. correct URI) but the functionality they expected to see hasn't yet been implemented. I think to differentiate between the two would certainly be useful for consumers down the track.

da-banking commented 6 years ago

The UK standards had many optional endpoints, for example: GET /transactions

While it may benefit the data provider to implement this by reducing the server resources required to give a data consumer the complete transaction history, the data consumer could get the same data via the chattier and mandatory GET /accounts/{AccountId}/transactions endpoint.

So if an optional endpoint may never be implemented, it would be good to indicate that to a caller. Data consumers could attempt to use the bulk methods when supported, and fallback to the expensive methods if they can detect the difference.

However, 501 Not Implemented is meant to mean the request method (PUT/POST/DELETE) is not implemented anywhere on the server for any endpoint. The HTTP specs don't allow 501 to be returned on GET/HEAD as support for these methods is mandatory for an HTTP server.

The 501 (Not Implemented) status code indicates that the server does not support the functionality required to fulfill the request. This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource. (https://tools.ietf.org/html/rfc7231#section-6.6.2)

If there is no implementation at the end of a URI, which may include a versioning segment, we would recommend a 404 Not Found.

The URIs will be static and defined by Data 61, so there is no ambiguity about what a 404 would mean to a data consumer. If a client requests a non existent resource, this is a client error.

Many of the decision proposals overlap to some degree. Apologies for muddying the waters, but it seems appropriate to touch on versioning here also.

With the endpoint versioning recommendation siding with HTTP Headers (as opposed to query strings #4 ), neither 404 nor 501 seem appropriate if the server does not support the requested minimum version. 404 is for a URI, and 501 is a server error - neither applies to HTTP Headers.

We think that the versioning negotiation could be considered a subset of content negotiation.

For example: Accept: application/vnd.data61.v2+json;q=0.1, application/vnd.foo.v5+json;q=0.9. Here the data consumer is requesting v2 of the Data61 APIs, prefering v5 of the vendor specific API from vendor foo (which is compatible with v2 of the Data61 APIs).

By negotiating versions with this mechanism, the response in the case where the server cannot find a good match would be 406 Not Acceptable.

The 406 (Not Acceptable) status code indicates that the target resource does not have a current representation that would be acceptable to the user agent, according to the proactive negotiation header fields received in the request (Section 5.3), and the server is unwilling to supply a default representation. (https://tools.ietf.org/html/rfc7231#section-6.5.6)

JamesMBligh commented 6 years ago

Thanks for both of these excellent comments. I had not considered 406, more that 404 seemed wrong. It seems like a good alternative.

-JB-

ajmcmiddlin commented 6 years ago

Is there a reason for only permitting a 400 Bad Request in response to a POST request? Section 6.5 of the HTTP RFC says that the 4xx codes are "applicable to any request method". It also seems reasonable that a GET or DELETE might have invalid query parameters and therefore warrant such a response.

This also raises the question for 415 Unsupported Media Type, which is also listed in the proposal as only applying to the GET and DELETE methods. However, unlike 400, I don't see 415 responses making sense for the GET or DELETE methods.

onereddogmedia commented 6 years ago

Will a standard error response payload be defined? eg

{“error”:400,”message”:”invalid parameters”}

or similar.. this will allow the client to make certain decisions based on the error response, or provide some useful information.

da-banking commented 6 years ago

Agree with @ajmcmiddlin - 400 Bad Request should be returned in other scenarios. A missing mandatory query param, or invalid value on a GET request.

@onereddogmedia - we would discourage this approach. While these errors are helpful to developers at design time, the API standards, sample code, along with experimentation make it easy to work out what is wrong with a request that returns a 400 response.

Providing a discrete response puts the onus on the standards body to identify each way a request can be bad ahead of time, and introduces a breaking change issue if new ways are identified. For example a data consumer could switch on the message and build a behaviour on that, and would be broken if a new unhandled reason was returned for an issue that was not considered.

If a data provider unilaterally added some custom validation to reject intentionally corrupt values because of a security concern, then it would be good to role this out without breaking valid clients. A generic 400 response would cover this.

If there is a decision to support discrete error messages, then it should be for runtime concerns that data consumers cannot avoid through better design. And we would expect there to be a generic catchall error message for the purpose of dealing with unanticipated requirements.

JamesMBligh commented 6 years ago

Thoughts on feedback to date:

@ajmcmiddlin I will look into your feedback. To be honest I lifted the commentary for 400 and 415 directly from the UK standards as I didn’t have a reason to diverge (this was possibly a little lazy on my part). I would welcome comment from others on the feedback presented.

I will address the concept of an error payload in the payload standards proposal. I am expecting that some form of inclusion is appropriate though. The UK standard and JSONAPI.org both include support for an error object as a substitute for the data object when an error occurs. This capability is extremely useful in some scenarios but can be misused or used to cover over design deficiencies as @da-banking has correctly observed.

-JB-

JamesMBligh commented 6 years ago

Feedback will close tonight. Much of the feedback represent improvements to the recommendation so, at this stage, the final decision will largely be as proposed with feedback incorporated as much as possible.

BTW, the payloads decision proposal is now up and includes an error message standard.

-JB-

NationalAustraliaBank commented 6 years ago

Some other error codes which are worthy of consideration:

202 Accepted The request has been accepted for processing, but the processing has not been completed. The request might or might not be eventually acted upon, and may be disallowed when processing occurs. (Maybe not required for Day 1 of Open Banking but potentially useful is we do async processing in future calls). Also, notices

206 Partial Content (RFC 7233) The server is delivering only part of the resource (byte serving) due to a range header sent by the client. The range header is used by HTTP clients to enable resuming of interrupted downloads, or split a download into multiple simultaneous streams.

409 Conflict Indicates that the request could not be processed because of conflict in the current state of the resource, such as an edit conflict between multiple simultaneous updates. (Depends if resource creation/editing is included in Open Banking Day 1)

422 Unprocessable Entity (RFC 4918) The request was well-formed but was unable to be followed due to semantic errors (eg business errors)

503 Service Unavailable The server is currently unavailable (because it is overloaded or down for maintenance)

504 Gateway Timeout The server was acting as a gateway or proxy and did not receive a timely response from the upstream server.

As a nice to have, it would be nice to collaborate on building out an HTTP Decision Diagram for the error codes. This will be useful down the track when it comes to industry testing, and will be useful now to ensure there are no logical gaps in the thinking.

An example can be forked from this repo:

https://github.com/for-GET/http-decision-diagram

bazzat commented 6 years ago

The consensus of the ABA Online Banking Technical Working Group is that we collectively support Data61's recommended HTTP error codes (possibly with some additions as noted previously in this thread - e.g. 202 Accepted).

TKCOBA commented 6 years ago

COBA’s view is that it would be best to adhere to common standards and avoid the creation of any granular custom error codes. COBA appreciates that while it is important, in some situations, to provide a reason why a certain request failed, this could be used to enumerate what features are available via certain API. On this basis, COBA would prefer the use of a single error code, such as ‘404 Not Found’.

JamesMBligh commented 6 years ago

I have now closed consultation on this decision. A recommendation incorporating feedback will be made to the Chair in due course.

-JB-

JamesMBligh commented 6 years ago

The finalised decision for this topic has been endorsed. Please refer to the attached document. Decision 011 - Error Handling.pdf

-JB-