Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarity around errors, particularly HTTP errors #1122

Closed
joeshaw opened this issue Nov 23, 2016 · 9 comments
Closed

Clarity around errors, particularly HTTP errors #1122

joeshaw opened this issue Nov 23, 2016 · 9 comments

Comments

@joeshaw
Copy link

joeshaw commented Nov 23, 2016

I feel that the Errors section of the spec could use some clarity on what constitutes an error, and when error objects should be returned. The spec seems to tackle errors from the perspective of processing of the API itself, and not bigger picture HTTP errors.

Are error objects required whenever an HTTP error is encountered? It is not explicit in the spec.

For instance, if an API requires authorization and the credentials are incorrect or omitted, the HTTP server would be expected to return a 401 status code. Should it also provide an error object in that case?

If a server encounters an internal error and needs to return a 500 status code, should (MUST?) it return an error object?

What about a 404 when someone hits an undefined endpoint? Is there a "scope" in which the API lives? ie, say that the API is GET /v1/frobs and someone accidentally GET /v1/frob -- should that 404 response include an error object?

@bf4
Copy link
Contributor

bf4 commented Nov 27, 2016

@joeshaw dumb question, but have you looked at the examples?

I think the general answer is:

  1. HTTP defines errors as in the 400-500 range
  2. For the most part, you get to decide when to return these errors
  3. JSON API defines requests as transactional, such that requests either succeed entirely or fail entirely
  4. When returning errors, you get a whole bunch of choices on how to represent them. None of the options are required. In my own experience, I use status, details and source, and like the idea of code, but haven't used it much.

@joeshaw
Copy link
Author

joeshaw commented Dec 7, 2016

@bf4 I think you're assuming that an HTTP error should return a JSON API error. But that's the question I am asking (should it? must it?), and that I don't think is addressed enough in the spec.

@bf4
Copy link
Contributor

bf4 commented Dec 7, 2016

@joeshaw if you're returning resources to http error codes and errors to http success codes I suppose that is up to you, but jsonapi spec prefers http semantics.

Is your question that you're not sure what to do or why the spec doesn't explicitly say that http errors response documents should always have json api error objects?

@joeshaw
Copy link
Author

joeshaw commented Dec 7, 2016

@bf4 The question is, should JSON API error objects be returned from HTTP errors? If so, always? If not always, when?

Regardless of the answers, I feel that the spec is underspecified on what to do on HTTP errors.

If you're writing an API client, it's a lot easier to deal with things if all your responses are always JSON and that the error objects are common and consistent. It's not helpful if a server returns HTML when it throws a 500 error -- IMO the spec should be clear that in this example case it should return a JSON document with an error object.

(The reason I bring it up is from the issue I linked to this one. The Go library for JSON API, https://github.com/manyminds/api2go, makes it surprisingly difficult to return error objects from middleware. That makes it difficult to return JSON API error objects for things like panics, auth errors, etc. I was hoping to point to the spec as a basis to make the case for adding that functionality.)

@bf4
Copy link
Contributor

bf4 commented Dec 7, 2016 via email

@ethanresnick
Copy link
Member

JSON:API error objects are meant to cover all types of errors, including 401, 403, 500 etc—not just errors that have to do with the JSON the client sent (i.e., 400/422). This is shown in the examples, and we do say in the spec (with reference to non 400/422 errors) that "A server MAY include error details with error responses.", where "error details" links to the error object section of the spec.

Servers, of course, don't have to include any body with their error responses, though I think they should (why not give the client more info?). If they do, and the resource that triggered the error is within the "scope" of the API, then it makes the most sense that that body would use the JSON:API format by default. So I would be in favor, I think, of a PR changing those "MAY"s to "SHOULD"s, or adding other changes that you think would clarify things.

Now, the concept of the "scope" of the API is vague, and the little that the JSON:API spec does say about it is pretty unhelpful and probably not actually what we mean. (I'd be open to a PR revising that definition, too.) For now, though, I'd say to just interpret the scope of the API as some set of URIs defined by the API creator and presented to users as a whole.

If you need more evidence to take back to the Go library authors that returning JSON:API error objects should be possible for more cases, @bf4's argument could also work pretty well, i.e.: if the client's explicitly asked for JSON:API through the Accept header, the library should make it possible to serve that for the error body.

@bf4
Copy link
Contributor

bf4 commented Dec 15, 2016

@joeshaw anything new?

@joeshaw
Copy link
Author

joeshaw commented Dec 16, 2016

@bf4 nothing at the moment. i haven't looked into a PR to change the spec yet. i haven't heard anything back from the api2go folks, but I haven't pressed them on it yet.

@jelhan
Copy link
Contributor

jelhan commented Feb 1, 2022

I'm going to close this issue for now. @ethanresnick very well answered the question. Also the issue in the api2go library, which motivated the question, seems to be stale.

@jelhan jelhan closed this as completed Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants