Add error support for primitive fields

[mreichelt]

As documented on http://jsonapi.org/format/#errors a response can have an array of error objects. First we should add support for the most basic primitive error fields (all optional):

• String id
• String status
• String code
• String title
• String detail

Later (in a new issue) we can add support for the more complex error fields:

• links
• source
• meta

[mreichelt]

I started implementing the 'errors' field and found an issue. The following test code will fail because article is null (because the document has no data):

Article article = moshi().adapter(Article.class).fromJson(getErrorsEmptySample());
assertNotNull(article._doc.errors);

Overall, the problem is that the document is only exposed to the caller if the data is available and can be parsed. Only then the reference _doc can be used.

I started a new branch for this, but it's a work-in-progress. First commit: https://github.com/mreichelt/moshi-jsonapi/commit/0d23ba1ee9635b9cea0797aa4654116632c9df6c Complete diff: https://github.com/mreichelt/moshi-jsonapi/compare/fix-7-allow-private-and-protected-fields...mreichelt:fix-11-support-primitive-error-fields

[kamikat]

Maybe we can have a separated adapter for Error.class registered to moshi adapter factory.

Considering a typical use case with Retrofit. When client receives a non-2xx HTTP response, The call will end with an exception and we can get a raw body from that exception. There, parse a raw response body like moshi.adapter(moe.banana.jsonapi2.Error.class).fromJson(...) can get the Error object we need.

Parse all JSON as Document will also work so that we can access doc.errors and doc.data from parsed Document object. But that can lead to an unreadable code... (the problem we have with 1.x version)

[mreichelt]

I also think throwing an exception is the way to go - at least in the short term. Note that jsonapi might return an error array even when http code is fine. The easiest way to implement this would be throwing an exception during the parsing process. In the long term, it would be awesome if we could use Document<Article> or Document<List<Article>> in Retrofit.

[kamikat]

Note that jsonapi might return an error array even when http code is fine.

Could you please provide more details about that? I just can't find that in specification document...

[mreichelt]

In the error section the spec says:

When a server encounters multiple problems for a single request, the most generally applicable HTTP error code SHOULD be used in the response.

The spec also says at the beginning:

A document MUST contain at least one of the following top-level members:

• data: the document’s “primary data”
• errors: an array of error objects
• meta: a meta object that contains non-standard meta-information.

The members data and errors MUST NOT coexist in the same document.

This means that even a response with a 200 status code that only contains an error member is valid. By reading the spec, I see that a response containing only a 'meta' field is also valid. Currenty, moshi-jsonapi would not be able to return the data of a meta field when the data field is omitted, as the reference to the _doc would be null.

[kamikat]

Emm... By saying

most generally applicable HTTP error code SHOULD be used in the response

means that if the server occurs several error in a request, let's say an error object array containing both of 404 and 403 status. Then the status of the request should be 400 (which is generally applicable). But the specification does not mention details such as whether a 5xx error takes a higher priority over 4xx.

And, yes, a response containing only meta field is valid. And another tradeoff about usability. BTW, version 1.x supports a full JSON API specification but is too verbose to use... 2.0 improves that to provide a cleaner interface by sacrificing part of features supported by JSON API.

[mreichelt]

Yep, that's what the world 'should' means. It's not a 100% guarantee. :) I also like the idea of a cleaner interface - but not if this means if certain features of the spec can't be used. This probably means I will have to use a different solution. Thanks a lot!

[kamikat]

16 is merged and let's close this issue.