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

Introduce error elements. #208

Closed
wants to merge 3 commits into from
Closed

Introduce error elements. #208

wants to merge 3 commits into from

Conversation

steveklabnik
Copy link
Contributor

Closes #7.

This was taken basically wholesale from Collection+JSON

/cc @wycats

@steveklabnik steveklabnik mentioned this pull request Mar 14, 2014
@devinus
Copy link

devinus commented Mar 14, 2014

@steveklabnik What if multiple fields have errors, e.g. phone and email were invalid. I would encourage you to make the key errors (optional) that can be either [] or [Error, ...].

@stevenharman
Copy link
Contributor

As this is lifted almost directly from Collection+JSON this might not be the correct venue but... why only a single error vs a collection (even if it only contains a single item)? For self-consistency within this spec, I was expecting to see an errors key with a collection, but I'm willing to adjust my expectations.

@locks
Copy link

locks commented Mar 14, 2014

@devinus why null? just omit the key entirely.

@tpitale
Copy link

tpitale commented Mar 14, 2014

I believe this is only a top level error, and doesn't account for "validation errors". Is that correct?

@devinus
Copy link

devinus commented Mar 14, 2014

@locks I misspoke, I had updated it already.

@steveklabnik
Copy link
Contributor Author

I lifted it from CJ to give us a straw man to discuss, @stevenharman. I always try to START with what others have done, and evolve from there. @mamund, can you address why this isn't a collection? Do you regret that decision?

@steveklabnik
Copy link
Contributor Author

@tpitale yes, it s a top-level error. That doesn't mean it couldn't account for certain kinds of internal errors, though.


## Errors

The error object contains addiitional information on the latest error condition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo addiitional

@steveklabnik
Copy link
Contributor Author

I fixed the typo, thanks.

@ahx
Copy link
Contributor

ahx commented Mar 14, 2014

Say there is a thing that aggregates data from different places and i get /posts/42?include=author, but there was a server error while getting the author (status 503 or whatever).
How can we make clear that the error belongs to the author, but not to the post?

{
  error: { title: 'sorry', code: 503 },
  posts: [ { id: 42, title: 'Yay Post' }]
}

}
```

Implementors **MAY** choose to not use this optional feature and use another media type for errors if they so choose. vnd.error and HTTP Problem are two examples.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we probably want to specify a header that implementors can use to indicate an alternate media type for errors:

JSONAPI-Error-Type: application/vnd.error+json

This kind of thing is probably how, in general, we can support extensibility when we chose an existing media type as a default that makes sense to swap out. Obviously, swapping something out will break compatibility with API browsers and clients not explicitly designed to support the alternate, but it will at least make existing clients know to ignore the data instead of trying to interpret it as the default.

@dgeb
Copy link
Member

dgeb commented Mar 16, 2014

I'm also in favor of using an array for errors. It's a mess to munge multiple errors into one and a bigger mess to extract them into something meaningful on the client.

@joshdholtz
Copy link
Contributor

Also in favor of using array for errors. I would prefer to not have form validation errors (as an example) to be mashed into one single error.

@wycats
Copy link
Contributor

wycats commented Mar 16, 2014

I agree. Are there any specifications that looks like Rails' format (key: [error, error]). If not, we should probably just make a separate mini-spec that we can refer to (and let people use JSONAPI-Error-Type to experiment with alternatives if they like)

@samwgoldman
Copy link

I'd like to throw out a use case: I use nested objects in my API (not nested entities with identity, but nested compound objects) and the error representation is similarly nested. Rails' errors object doesn't support nested errors, but I hope nesting would be considered in a mini-spec as well.

Here is an example of a request and the associated error response from the app: https://gist.github.com/samwgoldman/9592722

If we are targeting Rails' error format, don't forget that Rails has it's own nested error handling that uses dot-separated keys in the errors object. I found this unsuitable for my own project, but I suspect many people use this feature.

@wycats
Copy link
Contributor

wycats commented Mar 17, 2014

@samwgoldman good points. I don't think I necessarily want to target Rails' exact format. It may, however, be useful to have MIME types for each variant so that servers can be explicit and client libraries don't have to guess.

@joshdholtz
Copy link
Contributor

@wycats Flexibilty with different MIME types is ideal sounding but I could see that creating a pretty good amount of discussion about all different types that all of us want added. It seems like the format with the most flexibilty would be...

{
     "errors" : [
          {
               // Could look like anything
          },
          {
               // Could look like anything
          },
          {
               // Could look like anything
          }
     ]
}

where the only change would be "errors" is a reserved key holding an array of resources.

@wycats
Copy link
Contributor

wycats commented Mar 17, 2014

I think we should go with a single, well-specified error object (perhaps derived from Rails, perhaps extended), and then explicitly support extensibility via MIME types. The point of extensibility is to allow people to try out other things without needing them to be blessed right away.

@diosney
Copy link
Contributor

diosney commented Apr 3, 2014

Some thoughts from previous discussion at #7:

  • It could be great if the errors is an array collection of errors, which match json-api nomenclature for resources and allows to specify multiple errors in a nice way without bringing further complexity to clients, despite the fact that errors are not resources per se.
  • Would be nice to have a field or path or pointer property to hold the specific resource/field that triggered the error, hence, will be allowed per error instance. If the error is top-level the property can be empty or not be at all. We can use JSON-Pointer for this, since even linked resources can be specified on this (this addresses what @ahx asked).

@feelepxyz
Copy link
Contributor

There's a spec being worked on for errors: http://tools.ietf.org/html/draft-nottingham-http-problem-06
We are also thinking of adding path (Same as the JSON-Patch JSON-Pointer field) to indicate what field caused the error.

Fully extended:

{
   "type": "https://developer.example.com/errors#invalid_params",
   "title": "Validation Failed",
   "detail": "Title is missing",
   "code_type": "invalid_params",
   "instance": "https://api.example.com/requests/REQUEST_ID",
   "errors": [{
       "type": "https://developer.example.com/errors#missing_field?path=/title&resource=payments",
       "title": "Title is missing",
       "detail": "Title must be a string longer than 5 characters",
       "code_type": "missing_field",
       "path": "/title"
   }]
 }

@steveklabnik
Copy link
Contributor Author

HTTP-problem was explicitly discussed in #7.

@feelepxyz
Copy link
Contributor

Totally missed this.. ignore:)
I'm also trying to think of a way to represent errors for doing a POST with multiple elements, or similarly doing a GET for multiple ids. Facebook solves the GET by returning null at the array position. Not too sure about this though..
See: https://developers.facebook.com/docs/graph-api/making-multiple-requests/
Leaning towards an approach using JSON Pointer to reference the array element in the POST/GET body. Need to give this some more thought though..
Anyone implemented anything similar?

@krainboltgreene
Copy link
Contributor

Some things to consider about the structure of an error object:

  1. SOMETIMES the client speaks many languages, so message, title, and detail from the server become useless.
  2. SOMETIMES have multiple resources.
  3. SOMETIMES you don't have the same type of resource.
  4. SOMETIMES you don't have a 1:1 ratio for field:column.
  5. SOMETIMES you are returning an error tangentially related to the original core request. (Example: Attempt to update an account you don't own returns an error about authentication, not your update)
  6. SOMETIMES the user entered value isn't the same value that failed validation.

@stevenharman
Copy link
Contributor

@krainboltgreene, I would expect some of those issues to be solved with mechanisms other than just error messages returned in the body; e.g., HTTP Status Codes. And I'm not sure I follow some of your other points. I've addressed each inline, below.

  1. SOMETIMES the client speaks many languages, so message, title, and detail from the server become useless.

As those are keys, they'd not be I18n'ed, nor would any other keys in your response; translating keys in a data structure makes for a real BAD TIME™.

  1. SOMETIMES have multiple resources.

Not sure I follow what you're talking about here. If the request were for a collection, it would make sense for error messages to either be about the collection, or some how identify the problematic members. Maybe a concrete example would allow for more precise discussion here?

  1. SOMETIMES you don't have the same type of resource.

Similar to above - not sure I follow. Perhaps a concrete example?

  1. SOMETIMES you don't have a 1:1 ratio for field:column.

AMS doesn't tie you to just database columns; you can serialize nearly anything you wish as a value under a key.

  1. SOMETIMES you are returning an error tangentially related to the original core request. (Example: Attempt to update an account you don't own returns an error about authentication, not your update)

Per your example, I would expect such a request to result in either a 404 (to prevent leaking information) or a 401 if it's really an authorization issue. And the error message could/would be in respect to that, not the resource (which to prevent leaking information, the server would treat as non-existent until you've authenticated).

  1. SOMETIMES the user entered value isn't the same value that failed validation.

That sounds like a 422, Bad Request, no?

@krainboltgreene
Copy link
Contributor

As those are keys, they'd not be I18n'ed, nor would any other keys in your response; translating keys in a data structure makes for a real BAD TIME™.

I don't think you understand: Keys tell the client what value to look up in their i18n dictionary.

Maybe a concrete example would allow for more precise discussion here?

  • Batch updating
  • Updating A which has a relationship and validation associated with that relationship
  • Updating A and B together, in a joint update
  • Inserting A which produces B, C, and D (all with different errors).
  • Upserting A which tangentially updates B, which causes an error in B.

And the error message could/would be in respect to that, not the resource (which to prevent leaking information, the server would treat as non-existent until you've authenticated).

My point still stands, it's an error that is tangentially related to the original request.

That sounds like a 422, Bad Request, no?

No, you misunderstand. Sometimes intermediaries change user input and the client needs to know exactly what the server tried to process. Example: User submits a file object, which goes to a file uploading api, which gets turned into a URL, which gets submitted to the server. The server got a URL, which was somehow invalid, and that has to go to the client who can display why (Size problems, file type, etc).

@BRMatt
Copy link

BRMatt commented Apr 28, 2014

As those are keys, they'd not be I18n'ed, nor would any other keys in your response; translating keys in a data structure makes for a real BAD TIME™.

I think @krainboltgreene means that if the users of the client application speak French but the API server returns "end user friendly" error messages in English, then the messages aren't really appropriate? Maybe there should be some clarification as to who these messages are aimed at.

@samwgoldman
Copy link

For strings delivered by the API which are intended for the user directly, the Accept-Language mechanism can be used.

@krainboltgreene
Copy link
Contributor

I recently had a twitter convo about this: https://twitter.com/stephencelis/status/460809826616889344

@ahacking
Copy link

ahacking commented Jun 3, 2014

In my JSON API's I implement errors within a _meta key on each object.

The _meta key supports meta data transfer on all requests/responses for any object and has proven to be a convenient place to put processing and validation errors. It avoids the problem of referencing what object an error belongs to and it supports arbitrary object nesting or referencing strategies. It is also convenient to serialize since on the server you have the model object right there and can just dump errors under the _meta.error key. No need to collect errors and construct some error key path on serialization. Same deal on the client, errors get associated to the model object they relate to with minimum fuss. The HTTP response code indicates that errors are present for wholesale 'did the operation succeed or not logic' so there is no need to hunt for errors to detect an error condition.

In my scheme the error key refers to an object and not an array, instead each key of the error object represents a respective key in the object whilst the actual error value is an array of objects to support multiple errors, with each error object having arbitrary error data, reason codes, descriptions etc. I reserve the _object key for errors that pertain to the object overall and which are not specific to any particular key.

Example:

{
  "posts": [{
    "id": "550e8400-e29b-41d4-a716-446655440000",
    "title": "Welcome",
    "src": "http://example.com/images/mustaches.png",
    "_meta": {
      "error": {
          "_object": [{
            "code": "limit_reached",
            "desc": "Take a break and come back tomorrow."
          }], "title": [{
            "code": "reserved",
            "extra": "whatever as required..."
          }], "src": [{
            "code": "license_violation",
            "desc": "Copyrighted material"
          }, {
            "code": "too_big",
            "max": "1Mb"
          }]
      }
    }
  }]
}

I would be interested to get feedback/opinions on the approach I use above.

I think it is imperative that JSON-API supports arbitary error information being returned in a structured fashion and straight-forward/easy processing on both client and server. If it doesn't deliver the goods on error handling I doubt it can ever be useful for my applications.

@ahacking
Copy link

ahacking commented Jun 4, 2014

I should also add that draft-nottingham-http-problem-06 has what I regard as "stake through the heart" issues AFAIKT:

  • I don't want a different media type for error responses on MY API.
  • My APIs include the API version in the Accept headers and I don't want some foreign "nottingham media type" interfering with the protocol expectations. The content of the errors in my api's are also subject to API versioning as I don't want to break older clients with unknown error codes, or error details. I believe the use of a different media type for "problems" is full of problems as it attempts to "escape out of the api".
  • it makes validation error handling a right royal pain because it has only a single problem and forces you to define your own "bag of problems type".
  • you have to stuff the real problem collection under a sub key, and then have to figure out some property naming / JSON pointer / XPath type scheme to resolve what each error actually pertains to.

I believe the draft will have difficulty gaining acceptance because it presumes to codify an error handling approach for API's in isolation to the various JSON API styles that exist. There are a myriad of ways of defining a JSON API and error handling needs to be a core part of that particular API style and in line with the way that API approach handles meta information including schema description. Using URIs for problem types is rubbish when an API has documented schema and/or includes schema references. The draft needs to do this because it needs global types as it switches media types away from your API into its own world. Bad idea IMO.

Its just my 2c but I think json-api would do well to ignore draft-nottingham-http-problem-06 and devise an approach that is simple to implement and one which avoids the need for json pointer / XPointer / Xpath like approaches when possible and one that doesn't switch media types!!!

@steveklabnik
Copy link
Contributor Author

This has been superceded by #234, please go check it out.

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

Successfully merging this pull request may close these issues.

Errors?