mnot / I-D

My Internet-Drafts
https://mnot.github.io/I-D/
Other
98 stars 38 forks source link

draft-nottingham-http-problem-06 #45

Closed MajorBreakfast closed 9 years ago

MajorBreakfast commented 10 years ago

@steveklabnik, the person who is currently leading the efforts of developing the json-api spec suggested that I should post my concerns about the http-problem specification right at the source (I'm guessing that's here).

I've written a lengthy comment about it: https://github.com/json-api/json-api/issues/7#issuecomment-34739285. It would be great if you were willing to discuss any improvements! (Here)

steveklabnik commented 10 years ago

Yes, I would really like to use http-problem in JSON-API, but there are a few questions.

diosney commented 10 years ago

It seems that there is an old discussion about some of @MajorBreakfast points at http://www.mnot.net/blog/2013/05/15/http_problem

Maybe we can get into it to clarify some of them?

MajorBreakfast commented 10 years ago

"errors" -> "problems" not a big deal

My biggest concern is the "problemType" being an URI. It should be something short and sweet. However I see now why Mark Nottingham decided to make it so. But I think the link should be put into a "doc", "docs", "documentation", "description", "moreInfo", "info" or "href". field. And I agree that it's a SHOULD field for mysterious errors like "csrfTokenInvalid".

The supplied example shows the "instance" is an absolute url. At least for json api it's probably best if it showed the path of the problematic field in the request document like "path": "/posts/2/title" because that's what we need to provide the context. I previously said that it should be called "path". I'm not so sure about that now because it does not quite the same as the "path" in the JSON patch standard in which the "path" is the path to a resource field, not a field in the request document.

And now the array thing. One solution to this was described by Mark Nottingham himself in the comments and it is to introduce another problem type. I'm not sure. What are the Pros and Cons? At least in json api everything as a plural top level key (like "posts": [...]) proved to be rather simpler.

mnot commented 10 years ago

Hi,

Thanks for the input :)

My biggest concern is the "problemType" being an URI. It should be something short and sweet. However I see now why Mark Nottingham decided to make it so. But I think the link should be put into a "doc", "docs", "documentation", "description", "moreInfo", "info" or "href". field. And I agree that it's a SHOULD field for mysterious errors like "csrfTokenInvalid".

So, the type is crucial to uniquely identifying the problem. If we make it a short string, clients will need some context to understand what that short string is -- i.e., you need to know that you're "in" a particular API.

That's not very Web-like. Good APIs should be able to be mixed up and un-ambiguously identify what they mean.

What's the problem :) with having a URI to identify it? After all, these are going to be consumed and generated by machines, not people.

The supplied example shows the "instance" is an absolute url. At least for json api it's probably best if it showed the path of the problematic field in the request document like "path": "/posts/2/title" because that's what we need to provide the context. I previously said that it should be called "path". I'm not so sure about that now because it does not quite the same as the "path" in the JSON patch standard in which the "path" is the path to a resource field, not a field in the request document.

Instance gives an identifier for the occurrence of the problem, not where in the request it was a problem (we don't assume the request was JSON). Think of it as a support ticket identifier.

If you want to define a problem type with a "path" extension that identifies where in the request there was a problem, that's totally cool.

And now the array thing. One solution to this was described by Mark Nottingham himself in the comments and it is to introduce another problem type. I'm not sure. What are the Pros and Cons? At least in json api everything as a plural top level key (like "posts": [...]) proved to be rather simpler.

Many people have brought this up.

The whole idea behind the problem spec is that it's a refinement of the HTTP status code, not a replacement for it. When you allow an array there, it means that you have multiple errors, some of which may conflict with that of the HTTP status code.

WebDAV tried to address this with the 207 Multi-Status status code, and all of the WebDAV folks I talk to wish they hadn't done it; it's caused them a number of problems, mostly because it requires infrastructure to look inside the payload to figure out what's happening.

If you want to communicate a number of related problems, this can already be accommodated. For example, if your problem type is "validation error", you can put an array or object in the extensions that lists all of the different validation problems.

You can also make a new problem type that combines the semantics of two similar problem types, if necessary.

It's only when you need to communicate fundamentally different problem types on the same response that you'd need an array, and if they're fundamentally different, they don't fit into HTTP very well.

Do you have use cases that aren't covered by the solutions above? Would love to hear about them.

BTW, the most recent draft when from camelCase to single words to avoid stylistic clashes.

MajorBreakfast commented 10 years ago

@mnot I'm very happy that your're willing to discuss these problems!

First I'd like to restate why I'm so sure that the (now called) "type" needs to be something short. You say that it's generated by a machine and also consumed by a machine. That is of course correct but it's also a half truth. When developping an application you're going to touch the type quite often because it's the primary key since the other fields are allowed to be localized. It's the thing that will most certainly pop up in the language files (rich client apps/mobile apps) and in the places of the application that decide how to react to the error. You say that it's benefical that it's an URI because you can dereference it. But the support document that it leads to is written for people anyway. So what's left and I think you're after is that if there are errors from multiple APIs, you'll know which error is from which API. But it's from the API you sent the request to, isn't it? We really should standardize a "documention"/"reference" field.

Could you outline a common use case for the "instance" field. I don't seem to get what it's for. Could you give a concrete real world example? If it's a support identifier, then why isn't it called "supportIdentifier"?

I get the multi status code thing. We certainly could introduce our "type": "validation" and standardize a "violations" array with objects that hold our beloved "path" property (-> https://gist.github.com/MajorBreakfast/8884211) My concern is that 90% of the errors end up with "type": "validation" then. I have a bad feeling there. So I'm really not talking about multi status code errors but of this problem.

Any chance we could define a root key "problem(s)"? Having such a root key makes it possible to sideload things. In json api it's the standard layout. And of course: The plural/singular could solve the array/object problem (See example below).

The "status" field has no place whatsoever in the JSON. It's repetition. The people who demanded of you to put it in should put it in because it falls into the arbitrary field category. The standard should not mention it because it's a crap idea. (@diosney agrees)

{
  "problem": { 
    "type": "csrfTokenInvalid",
    "documentation": "http://developers.example.com/manual#csrfTokenInvalid"
  }
}
{
  "problems": [
    { "type": "invalidEmail", "path": "/users/0/email" }
    { "type": "required", "path": "/users/0/givenName" }
  ]
}

And we should add the "path" property to the standard. It refers to the content of the request. I think a standardized name is very useful and we should define its layout if the request was in JSON.

Edit: This post criticizes a lot. But just the draft you know :)

steveklabnik commented 10 years ago

If you want to communicate a number of related problems, this can already be accommodated. For example, if your problem type is "validation error", you can put an array or object in the extensions that lists all of the different validation problems.

Seems good to me!

steveklabnik commented 10 years ago

If you want to communicate a number of related problems, this can already be accommodated. For example, if your problem type is "validation error", you can put an array or object in the extensions that lists all of the different validation problems.

Seems good to me!

diosney commented 10 years ago

The "status" field has no place whatsoever in the JSON. It's repetition. The people who >demanded of you to put it in should put it in because it falls into the arbitrary field category. The > standard should not mention it because it's a crap idea. (@diosney agrees)

@MajorBreakfast Take it easy :) Is true that there is little information about that design "feature", but maybe there is a hidden conclusion behind it that we don't know.

That being told, I do think that it should at least be optional and not a MUST so we can get rid of it.

MajorBreakfast commented 10 years ago

@diosney I'm a bit of a tldr; fan. I'm just saying how I see it there. Maybe I shouldn't call it a crap idea, let's say ill advised :) Not meant in an aggressive way though...

robertoandrade commented 10 years ago

First I'd like to restate why I'm so sure that the (now called) "type" needs to be something short. You say that it's generated by a machine and also consumed by a machine. That is of course correct but it's also a half truth. When developping an application you're going to touch the type quite often because it's the primary key since the other fields are allowed to be localized. It's the thing that will most certainly pop up in the language files (rich client apps/mobile apps) and in the places of the application that decide how to react to the error. You say that it's benefical that it's an URI because you can dereference it. But the support document that it leads to is written for people anyway. So what's left and I think you're after is that if there are errors from multiple APIs, you'll know which error is from which API. But it's from the API you sent the request to, isn't it? We really should standardize a "documention"/"reference" field.

@MajorBreakfast not sure if you're understanding the URI paradigm right, but just in case I'm misunderstanding your point, I believe URI here doesn't mean URL necessarily. This is very common in concepts such as XML namespaces and other URI-based/driven systems such as some mobile frameworks like Android and iOS (where the URI can be an application-scope identifier such as app://authority/path#fragment?query), used to identify a particular "thing" as part of that problem-domain :)

diosney commented 10 years ago

:)

@mnot, @steveklabnik What do you think about having the redundant http status code be optional?

diosney commented 10 years ago

@MajorBreakfast I know ;), just wanted to make sure we are not getting bad humors here, after all we are visitors and wanted the most of our host :)

guillec commented 10 years ago

@diosney the 'status' member is optional.

The status member, if present, is only advisory; it conveys the HTTP status code used for the convenience of the consumer. Generators MUST use the same status code in the actual HTTP response, to assure that generic HTTP software that does not understand this format still behaves correctly.
diosney commented 10 years ago

@guillec Thanks! Somehow I missed that :O

I will go to bed now for several days to recover. This was a huge mistake...Thanks again.

MajorBreakfast commented 10 years ago

@robertoandrade "invalidEmail" is not an URI

robertoandrade commented 10 years ago

@majorbreakfast hence the draft suggesting something like http://api.domain.TLD/errors/validation#invalid-email?code=000 instead.

MajorBreakfast commented 10 years ago

@robertoandrade Yes it suggest that. I think it's too verbose for code. I suggest to have a clear recommendation for something short in the spec. (You can't program Java without an IDE but you sure can program JavaScript with any text editor. Reason is that it's impossible to remember the java package names - unless you have a super brain :) ) Short and concise that's the way to go.

robertoandrade commented 10 years ago

@MajorBreakfast I think you're talking about two different things, one thing is a protocol level (HTTP/API server with HTTP/API client), the other thing is the interface of the HTTP/API client with the developer himself.

In many platforms you could return the user a representation of a URI/URL object to which they can inquiry the "short" piece of the URL that is of interest, but still allowing him to check for the other parts, i.e.: in JS the Location interface allows you to represent a fully-qualified URI/URL and fetch the interesting bits, i.e.: Location.hash in the case of the previous example.

So if a http-problem client API (such as the ones json-api suggests using) for the particular language you're consuming the HTTP API, would return the "Problem" object, with a property called "Type" which is of type "URL" or "Location", from which you can pull the short piece you're interested in, such as the hash or the code. just like you would if you were doing straight JSON object manipulation and fetching the properties yourself.

So at the end, problemObject.type.<hash | fragment | etc> = "invalid-email"

MajorBreakfast commented 10 years ago

@robertoandrade In my sample responses above the problem objects are one liners. A quick glance at each tells you in an instance what went wrong. This is what I want to see when I inspect the traffic in chrome dev tools.

It's still possible to access the URL. It's in the request meta data, too. Maybe not the URL to an error description. But that is why I'm proposing a "documentation" field - not necessary for the "invalidEmail" error, but interesting for more complex errors.

mnot commented 10 years ago

@MajorBreakfast - that's what "title" is for.

MajorBreakfast commented 10 years ago

@mnot Please elaborate. I addressed the "title" thing in one of my posts above. This discussion kind of needs a detailed response from you now.

diosney commented 10 years ago

Hehe, ironically, I moved from hating the status code in the standard to support it :) (always as an optionally attribute).

I've been researching a lot and found that it could be useful in some rare cases, like the scenario when the platform/app is intercepting the HTTP errors and taking action before the developer can get into it (fi, can happen in flash apps). That way the api can always send the HTTP 200 status code and then in the payload specify the real HTTP status code associated with the response. I'm not saying this is the right way to do it, but a real example of the usefulness of the status code.

@mnot Maybe is worth posting this somewhere to let others know when can be useful? This way you can get rid of lot of hate about this attribute.

mnot commented 9 years ago

We're trying to get the spec to WGLC in the apps area WG, and I realised we never closed this discussion out. Sorry.

AFAICT, the major points left:

Anything else?

Thanks,

mnot commented 9 years ago

Not hearing anything, so closing.