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

Errors? #7

Closed
steveklabnik opened this issue May 4, 2013 · 56 comments
Closed

Errors? #7

steveklabnik opened this issue May 4, 2013 · 56 comments
Labels
Milestone

Comments

@steveklabnik
Copy link
Contributor

We should address how errors are returned.

This replaces rails-api/active_model_serializers#214 and rails-api/active_model_serializers#199

@wrightling
Copy link

I've been scouring the internet a bit as I work on my first JSON API (using rails-api). Two references I came across return errors in some varation of { error: "message" } or { errors: ["message1", "message2"] }. Is there a more standard way?

Referencing sample app from a rails 3 book sample app ticketee and a Travis Jeffery blog post.

@steveklabnik
Copy link
Contributor Author

I feel like there is some sort of interent-draft for an errors spec, but I can't find it at the moment.

@wrightling
Copy link

Here are a few more examples:

Looks like there is nothing if not variety in how folks are returning errors. Personally, I'm not sure I see a point in Google's recommendation to include the code in the response body - seems like duplication.

@bcardarella
Copy link

We've discussed this in the Ember CF quite a bit. The errors object should be a key/value of arrays. So { 'errors': { 'first_name': [] } }

What is not as clear is if full messages should be retured in the array or references to messages that will be determined by the client. The discussion seemed to be in favor of returning message keys that can then be rendered by the client's i18n message processor.

@wrightling
Copy link

The format you suggest would be pretty darn easy to support in rails or rails-api as well, as shown in this gist

@trek
Copy link

trek commented May 5, 2013

Please, for all that is good in the world, provide as detailed data as possible.

Working with APIs that return just a string (when I've specifically said "I'll only accept application/json') or only string messages inside JSON {errors: ["good luck figuring out what property is wrong", "hope you like regexps"] }` when the design calls for highlighting where on the page the user erred has got to be on of the most frustrating experiences as UI engineering.

For maximum usefulness, I usually at minimum want something like:

{
  "errors": {
    "firstName": ["cannot be blank", "must be 10+ characters"],
    "lastName": ["cannot contain symbols"]
  }
}

Given the format of what JSON API looks like so far though, I'd expect returned errors to behave exactly like any compound document and include as much about each error object as possible.

@bcardarella
Copy link

@trek would something like this be more to your liking?

{
  "errors": {
    "firstName: [{"key":"blank", "options"{}}, {"key":"tooShort", "options": {"count": 10, "value": 5}}]
  }
}

@wycats
Copy link
Contributor

wycats commented May 5, 2013

Can we discuss how to handle errors for compound requests? I'm really interested in people's feedback on that topic.

@paddycarver
Copy link

I have very strong opinions on this particular subject, because I flip a table in rage every time I have to interact with an API's errors, because every API sucks at returning useful information. And the number of strings returned above is really, really scaring me. So I will propose the solution I use, which I feel is pretty good, but I'll also share the principles I used to arrive at that solution and why I believe they're important.

Why are we providing information about what went wrong?

The first part of this is the part that gets me angry so often. Most APIs do not consider this question when designing errors, and that makes them a pain to work with.

I humbly submit that errors are returned to instruct the user on how to fix their error. This means that all of and only the user-relevant information should be returned, including the precise reason what the user did was not acceptable.

The reason this is important is because the API is not responsible for the user interface. That is why I'm against APIs passing back an error message that they expect to be displayed to the end user. Most of them do this as the only description of the problem you get, which makes it hard to have, e.g. in form validation, a good user experience. How do you display the error adjacent to the relevant field if there's no way to determine which field produced the error?

If your answer is "parse the string and look for information in that", I want to go cry in the corner now. When error messages are the only description of the error that occurred, you are essentially forcing every client library to just tell their users "Your request is bad and you should feel bad", because it's impossible for them to get more useful information than that.

The reason that you should only expose user-relevant information is because anything else bloats the client libraries for absolutely no reason. I was working on a company's API when I discovered a validation call returned two separate errors based on the way the call was invalid. When asked about it, the company explained that one error was when the invalidity was detected in their own system, the other when the invalidity was detected in the third party system they called in the background. They were unable to explain to me why I should care where the call was invalidated, because that made absolutely no difference to the end user. If you're returning that information for debugging purposes, it belongs in logs, not in things that client libraries have to deal with.

So here are the principles I came up with:

  1. Every error should prompt one, and only one, action that is necessary to correct the error.
  2. Error details should always be returned in a machine-readable format. Parsing error messages is crazy, brittle, and a code smell.
  3. Each request can have more than one error. Responses should expose as many errors as possible, so that users can fix their request once, not once per each mistake.

Now that my novel/rant/mission-to-civilise is finished, I'll share my proposed solution.

Errors are simply objects in the response body, similar to any other resource. They are returned in an array, with the key of errors (assuming, of course, we adopt #8). Each error object is made up of two pieces of information: a specific error code (not an HTTP error code. These error codes are something like "invalid format", "invalid value", "already in use", "too short", "too long", etc.) and the specific field that raised that error. I'm currently returning that second piece of information with two keys ("field" and "item"; field represents the specific field "post.title" and the item represents the position in the request. E.g., if three posts are created at once, and the error field is "post.title", the error code is "too_long", and the error item is 1, that means the second post's title is too long and needs to be shortened). I think a superior solution would be to combine them in one using RFC 6901 to point to the specific item in the request that was wrong.

So errors would look like this:

{
  "errors": [
    {
      "code": "too_long",
      "field": "/posts/0/title"
    },
    {
      "code": "invalid_format",
      "field": "/posts/1/slug"
    }
  ]
}

This would mean that the first post in the request had a title that was too long, and the application should tell the user to shorten that title, and that the second post has an invalid character in its slug, and the application should tell the user that there's an invalid character. The second example is not perfect; ideally, the response would tell the application which character threw the error (perhaps /posts/1/slug/4, to designate that the 5th character was the problem?) but that level of granularity is hard to obtain while not bloating other errors unnecessarily.

@hjdivad
Copy link

hjdivad commented May 5, 2013

I like @paddyforan's suggestion, although I would add a couple things:

  • It should be possible to specify related documents for cases like name conflicts.
  • It should be possible to specify meta information within errors to specify things like allowable characters or length requirements.
  • It should be possible to specify error messages. Although I agree that it's good to encourage codes on the server and expect clients to produce user-visible messages, I fear this may discourage adoption. If we allow meta information, it's easy enough to have the error messages there.

In the following examples code is required, everything else is optional.

Examples:

// compound request error involving a new resource and an existing resource
// POST /posts
errors: [{
  code: "name_conflict",
  field: "title",
  rels: {
    posts: [1]
  },
  meta: {
    message: "There is already a post with title 'Rails is Omakase'"
  }
}]

// compound request error involving a new resource and an existing resource of a different type
// POST /posts
errors: [{
  code: "insufficient_funds",
  rels: {
    accounts: [1]
  },
  meta: {
    message: "Your account does not have enough credits to create this post."
  }
}]

// metadata that makes the error code more specific
// POST /posts
errors: [{
  code: "insufficient_funds",
  rels: {
    accounts: [1]
  },
  meta: {
    cost: 200,
    balance: 100,
    message: "Your account does not have enough credits to create this post."
  }
}]

// Error not associated with a particular field
// POST /posts
errors: [{
  code: "unknown",
  meta: {
    message: "The server had an unexpected error.  Please try again later."
  }
}]

@litch
Copy link

litch commented May 6, 2013

I'm currently starting on an API using AMS, and encountered this issue. I like the errors attached to the object, I like the problematic attribute to be identifiable, and I want to a message (or messages) about the attribute displayed.

So I'm going to explore an errors "attribute" in my serializer like:

def errors
  if object.errors
    errors = Hash.new([])
    object.errors.each do |k,v|
      errors[k] << v
    end
  end
end

Which yields:

"order" => {
  "errors" => {
    "balance_due" => [
      "Order cannot be closed with balance due: $1.0"
    ]
  },
  "id" => 123477,
  "state" => "open",
  ...etc...

@luisobo
Copy link

luisobo commented May 6, 2013

My thoughts on errors

Errors are not always resource-centric

The way rails structure errors works great when an error is referring to a specific attribute of a resource but that's not always the case, like in bad requests, rate-limiting... you name it. The use of base is a workaround that a) does not scale if you want to report as many errors as possible per response and b) conflicts with a real attribute called base

I'd not stick with this specific way of structuring errors.

Error IDs

When consuming an API I always find very useful errors with a unique ID. Especially when all possible errors with their IDs are properly documented.

Most of the time only a subset of all the possible errors are recoverable by the user (i.e. a validation is recoverable but a malformed request is not). Note that this classification is client-specific. The error IDs come in handy to classify client-side which errors are user-recoverable and which are not so, when getting a error, I'd only prompt the user if the given error can be recovered, otherwise raise or fail gracefully.

Developer error message vs UI error message.

Returning two separate error messages will make the learning curve of the API at hand more moderate. Also it encourages the server to return a meaningful error message for the final user, instead of the current approach, where most services will return a message that falls in the middle and leaves no one happy. These messages can be overridden by the client using a client-side mapping between error IDs and the desired new message. But in my personal my experience, most of the time a well thought out message from the server will work for the UI.

Links to the documentation

Also, consider including a link to the documentation where the developer could find more info about the error. This could be in a separate attribute of the document of inside the developer message.

Some random example:

{
  "errors":[{
    "id": 1234,
    "developer_message": "Bro do u even oauth? Check the auth token",
    "user_message": "The provided credentials are invalid.",
    "more_info":"http//developers.example.com/docs/authentication",
    "rel": null
  },{
    "id": 2233,
    "developer_message": "Invalid name. It should match the regex '^\d.*$'",
    "user_message": "must start with a digit",
    "rel": {
      "dog": [1],
      "attribute": "name"
    },
    "more_info":"http//developers.example.com/v1/dog#name"
  }]
}

I'd be surprised to see these two errors together, but you get the idea.

I like the idea of referencing the invalid resource in the original request. I included @hjdivad's format and grouped everything into a rel node.

@paddycarver
Copy link

I can get behind @hjdivad's suggestions. I like meta, I'm uncomfortable but accepting of rel, and if someone wants to include a message in meta, who am I to stop them?

In response to @luisobo's excellent points:

Errors are not always resource-centric

I'm actually approaching this from a Go standpoint, not a Ruby one, but I'll address the main point anyways. According to RFC 6901, which is how I'd prefer to specify the object/field that caused the error, an empty string ("") represents the entire document. So how I handle this, personally, is setting the code to something that makes the error obvious (invalid_request_format for bad JSON formatting, act_of_god for internal server errors, etc.) and set the field property to an empty string. I think this addresses your concern that the error be bound to a specific resource?

Error IDs

Agreed. I believe that's the purpose of the proposed code field. You say tomato...

Developer error message vs UI error message.

I've stated my radical opinion on this before, and I'll state it again: I believe it is an anti-pattern to pass a string returned by the API directly to the user. UI error messages do not belong in the API, they belong in the UI. The only error messages that (I think) make sense in an API are developer error messages.

Links to the documentation

I would think this is something that an API could support on its own, using the meta field, without needing to make it part of the spec. I also think that standardising on this is a bad idea, because it is atrociously wasteful in performance-sensitive applications and bandwidth-sensitive applications. The documentation will only be used during development, and all that data is being shipped over the wire for every user in production. That's silly.

I'd also still advocate for using a JSON Pointer to identify the part of the request that caused the error, rather than embedding it back in the response. That seems wasteful, and runs afoul of your original point above because it becomes harder to signify the entire request, instead of a specific part.

@mamund
Copy link

mamund commented May 7, 2013

i usually just define an error object/block in my API message format:

also consider the following as possible models for reporting errors:

other things to keep in mind:

  • always report two levels of error information protocol-level (4xx and 5xx) and application-level (invalid inputs, data conflicts. etc.).
  • return a body w/ all 4xx & 5xx errors that include the app-level details.
  • for 503 errors consider add the Retry-After header (http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.37)

@luisobo
Copy link

luisobo commented May 7, 2013

@paddyforan

Errors are not always resource-centric

I don't have an strong opinion on the format used to point which object/field caused the problem as long as it's easy to express the scenarios described above.

Developer error message vs UI error message.

You mentioned that displaying API errors is an anti-pattern but I failed to understand your reasons, could you please rephrase them?

Personally, I'd consider it an anti-pattern if there is no such thing as a separation between user and dev messages and/or error messages are not properly documented, leaving no opportunity to the developer to check if these error messages are suitable for a UI or not. So yeah, if I have no effing idea about what is coming back from the server as an error I'd never put that in the UI.

Now, the point of including such separation is to force the service to return a good error message (meaningful, proper grammar, punctuation, no error codes in the message, etc). I'd also expect this errors to be properly documented. In this case, I'd not hesitate to take advantage of the error messages coming from the server.

Links to the documentation

I agree with you. This is a nice to have and probably based on necessities that will vary from business to business. (For your business performance is important, for mine it is to make the API easy to use for new developers). It could also be optionally included as part as the developer message as I suggested.

@paddycarver
Copy link

The reason I claim it is an anti-pattern to pass a string from the API directly to the user is that, in my opinion, an API exists to provide access to data, not an interface. I do not believe that internationalization or localization are concerns that should be applied on the API level. I also believe that different clients have different needs for displaying errors. Furthermore, if this is a third-party API, clients will each have a different tone as part of their user experience, and static error messages will not fit with that tone.

In brief, I believe that error messages are strictly the domain of a view, and APIs are decidedly models. I believe you should not pass through the API error message for the same reason you do not pass through your database's error message. The client has context the API does not, and therefore is better suited to creating a useful error message.

I'm not naive enough to believe I'll convince everyone that error messages should not be allowed. I think that developer error messages can be useful, assuming their target audience is client authors. If I were to be ambitious, I'd hope that I could convince people to relegate them to a relatively unobtrusive part of the spec as an optional component. Realistically, I just hope to convince people that they should not be a required component of the spec.

@wrightling
Copy link

It seems like, while strictly passing strings for display to the user is a bad idea, the ability to do so should be preserved.

An example: you have an application with multiple front-ends (iOS, Android, Windows Mobile, browser, OS X app). Functionality is nearly identical, and you want to avoid duplicating localization/internationalization of most of the text shown to the users.

I'd rather localize the text server-side and avoid duplicating the code to localize it all within each client. Each client could still override/ignore any localizations that aren't relevant to them or need to be tweaked.

Perhaps its about time to pick some suggestions, such as jsdvaid's, and submit a pull request to get things going?

@devinus
Copy link

devinus commented May 14, 2013

Please, for the love of all that is good and holy, include the key and the actual value that caused the error in the response.

@paddycarver
Copy link

@devinus that sounds like a recipe for trouble with very little gain. What does it enable that is otherwise impossible or difficult?

@steveklabnik
Copy link
Contributor Author

@kjwierenga
Copy link
Contributor

Looks like the http://tools.ietf.org/html/draft-nottingham-http-problem-03 draft handles this nicely indeed. It even handles internationalization. I don't like de camel-cased member names, but I guess that would be considered bikeshedding (sorry; had to use that word which I just learned about last weekend).

@steveklabnik
Copy link
Contributor Author

@wrightling
Copy link

Interesting that neither of those articles Steve linked, nor the http-problem-03 draft try to tackle a partial success scenario or even mention returning multiple errors.

I'm very interested to see how we recommend returning errors when, say, request is sent to update multiple resources at once, and 3 out of 5 succeed, while the other two error.

@steveklabnik
Copy link
Contributor Author

I think you found at least one of the right places. The place FOR SURE is on the IETF tracker.

@celkins
Copy link
Contributor

celkins commented Feb 11, 2014

@MajorBreakfast

Another thing I consider a mistake is that the "type" field has to hold an URL. It means that implementors have to create all these routes."

Except that it doesn't. Nothing in the spec excludes values for type such as the following:

Some values are obviously more useful than others.

@MajorBreakfast
Copy link
Contributor

You're of course right. It says that it should be an URI not an URL - my bad. What I'm proposing is still valid, though: I think that it should simply be a short string (that typically cannot be dereferenced). The example given should show something in camelCase, like "invalidEmail". "http://example.com/errors#invalidEmail" is super cumbersome by contrast. The "type" is meant to be used in your application code and/or language files. It's all about what's nicer to handle in code: A short string that brings it to the point wins, a long URI loses

@diosney
Copy link
Contributor

diosney commented Feb 12, 2014

I think we should already move on and define the errors within JSON+API standard itself like other media-types did instead of using another media-type for this, be draft-nottingham-http-problem-06 or another of the ones that currently are living out there.

This was already done with success by other media-types, like @mamund' collection+json, so there is no shame on doing that. I'm not saying that this is okey due other people did it, but there are problems with the other Errors standards that renders them incompletes as a json+api point of view and that some additions we would need for make them usable aren't desired parts of those standards or can have a bit of resistance to adopt it.

So , we can:

1- create another media-type for this and enlarge the zoo (paraphrasing @mamund).

or

2- include the errors standard in json+api itself.

I personally prefer 2, but I could live with whatever solution cames if it fill all the holes.

On draft-nottingham-http-problem-06, right now and after 6 revisions it still has several issues that slows its consideration to use it along json+api.

To sumarize some of them:

thanks to @guillec:

  • [its major hole IMHO] It only supports one single error per response and not an error collection, and according with the discussion, it is almost for sure it won't land into the media type. It was commented that was being into consideration the addition of a problems media-type, but I can't find any references to it 7 months later.

thanks to @MajorBreakfast:

  • The error type attribute (the one that holds the error code/id) must be an URI, which has some issues as pointed by @MajorBreakfast.

not commented here before (maybe are minor issues compared with the above) and that were pointed in the mnot blog comments:

  • Repetition of the HTTP status in the response body itself which can lead to out-of-sync states.
  • Force devs to use only camelCase attributes, which can be a source of discussion regarding the media-type adoption due conflicting code-style (that is a nonsense but can really happen).

So, what you think about all this nonsense?

@MajorBreakfast
Copy link
Contributor

I think that the standard MUST support camelCase, snake_case and dashes but it should RECOMMEND camelCase because that's very common in JavaScript (and the frontend is probably in JavaScript). I think it's a must because all three are quite common and frontend frameworks that support json api should be able to handle all three.

I agree that it's stupid to repeat the "statusCode" in the JSON. It simply doesn't belong there.

I posted new thoughts about the problems spec. here: mnot/I-D#45 (comment)

I think/agree that first and foremost it's essential for the errors spec we're going to end up using to be consistenent with the style of the rest of the spec and it should also address all the major use cases. That means it should support global errors, errors about an object in the request document and errors about a field of an object. In a year I'd like to be able to type something like {{input-field value=email errors=fieldErrors.email}} (Custom ember component that displays the errors right next to the input, controller is the UserController)

@mnot
Copy link

mnot commented Feb 13, 2014

Answered over at mnot/I-D#45; thanks for the heads-up.

FWIW - the latest draft goes from camelCase to single words to avoid stylistic clashes.

@steveklabnik
Copy link
Contributor Author

I would encourage you all to check out #208 .

Basically, after some discussion, adding a simple error type to JSON API directly seems good. This also does not prevent you from serving vnd.error if you wish, but requiring a dependency on an additional specification seems unwise.

@steveklabnik steveklabnik added this to the 1.0 milestone Mar 14, 2014
@steveklabnik
Copy link
Contributor Author

Adding to 1.0.

@mnot
Copy link

mnot commented Mar 14, 2014

@steveklabnik - I support your effort to eradicate all other specifications. When will you be taking HTTP off my hands?

@paddycarver
Copy link

srsly

@steveklabnik
Copy link
Contributor Author

Mark, that's not fair and you know it. JSON API is built on top of a ton of existing RFCs already. This decision does not block anyone who wants to use a specific error media type with JSON API.

@steveklabnik
Copy link
Contributor Author

I added a new line to explicitly call this out.

@devinus
Copy link

devinus commented Mar 15, 2014

@steveklabnik My fears haven't been assuaged. I think we need an errors key, i.e. multiple errors can come back from the backend.

@mnot
Copy link

mnot commented Mar 15, 2014

@steveklabnik Fair? There is no fair in API's....

Sorry (for both!), couldn't resist.

@wycats
Copy link
Contributor

wycats commented Mar 16, 2014

@mnot fwiw, I am in favor of making sure that JSON API is extensible with other media types that are richer versions of the default behavior of embedded content.

In this case, I have proposed (and @steveklabnik agrees) a header (JSONAPI-Error-Type) that a server implementation can use to inform the client that it is using an alternate media type for errors. This would let simple clients know to ignore the embedded errors, and will allow the community to coordinate around common alternates that we can bake into the commonly used tools.

@dgeb
Copy link
Member

dgeb commented Jul 5, 2014

#237 introduces a new error object. Error objects SHOULD be returned in a collection keyed by "errors".

I was certainly influenced by @mnot's http://tools.ietf.org/html/draft-nottingham-http-problem-06 when designing the error object. However, I also felt it important that errors be valid resource objects themselves, and reference other resources through links.

I believe more examples of errors are needed in the spec, as evidenced by the discussion in #234. Let's close this general discussion though and track specifics in new issues.

@dgeb dgeb closed this as completed Jul 5, 2014
@jwachira
Copy link

There is so much insights and stuff to learn from this discussion; I would love to see some of your implementations if anybody is willing to paste some code here 👍

@steveklabnik @dgeb

@zekefast
Copy link

I think as this issue was closed the (chapter in FAQ)[http://jsonapi.org/faq/#how-to-discover-resource-possible-actions] need to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.