Provide proper status codes for API operations

[javorszky]

@halfdan linked me http://tools.ietf.org/html/rfc2616. I was thinking whether it'd be a good idea / OCD / doing it right thing if we provided the proper status codes for API responses. Currently we're using 500, 404, 415, 413, 403, 401 and 200 (along with the 304 for caching).

The RFC allows for more granular bits, like

• 201 Created - could be used for new posts / users / tags
• 202 Accepted - could be used to get back to the browser once data is here, but before promises resolve and a proper response gets back (not sure though)

Thoughts?

[ErisDS]

@sebgie any thoughts on this?

201 seems reasonably sensible.
Perhaps the api should return 405 for unknown methods rather than 404s?

This is probably part of #2124

[sebgie]

100 % agree! Our status codes are very basic and not granular at the moment.

If the status codes are changed we'll have to adapt the front end as well. Currently the check for a failed request looks like this request.status !== 200 😉.

[ErisDS]

@sebgie lol.. yes.. we would need to change that.

I think Ember works with proper status codes though right? Might be worth checking.

[gotjosh]

Hey there! I'd love to help out with this one... Is it assigned to anyone or can I take it? 😄

[ErisDS]

It is assigned to @javorszky at the moment but he has quite a few so may be happy to drop this one? @javorszky?

[javorszky]

Nope, I'll get on with this tonight. Finally moved in, so I can focus on Getting Things Done:tm:

[sebgie]

Route	action	condition	sc now	sc new
/posts	GET	successful	200	-
/posts/	GET	successful	200	-
/posts	POST	successful	200	201 Created
/posts	POST	no JSON body	403	400 Bad request
/posts	POST	malformed JSON	500	422 Unprocessable Entity
/posts	POST	title too long	422	-
/posts/	PUT	successful	200	-
/posts/	PUT	no JSON body	403	400 Bad request
/posts/	DEL	successful	200	-
/users	GET	successful	200	-
/users/	GET	successful	200	-
/users/	PUT	successful	200	-
/users/	PUT	no JSON body	403	400 Bad request
/settings	GET	successful	200	-
/settings/	GET	successful	200	-
/settings/	PUT	successful	200	-
/settings/	PUT	no JSON body	500	400 Bad request
/notifications	GET	successful	200	-
/notifications	POST	successful	200	201 Created
/notifications/	DEL	successful	200	-
/db	GET	successful	200	-
/db	POST	successful	200	-
/db	POST	no json file	500	415 Unsupported Media Type
/db	DEL	successful	200	-

Oversights:

• If a method is used that is not supported by the route a 404 error is returned. From the available HTTP status codes I would say that 405 Method not allowed or 400 Bad Request would be a better fit? (e.g.: PUT /ghost/api/v0.1/posts/)
• IDs in routes are not checked against IDs in JSON body. As a rule of thumb I would say that the ID of the route should be used. If the JSON body holds an ID it has to match the one in the route. IDs in JSON body could be optional but that would be inconsistent with the internal API?
• Body of a JSON requests is not checked for required/allowed/malformed properties atm. These checks will probably return validation errors.
• Wrong or misused options are removed and the request is carried out without the options. This behavior makes our lives a bit easier but I think that wront request parameters should result into 400 Bad Request.

As a side product I uploaded a Ghost collection for Postman (https://gist.github.com/sebgie/7a44f4021bebbc078897). If there is interest I'll keep it updated.

[ErisDS]

IDs in routes are not checked against IDs in JSON body. As a rule of thumb I would say that the ID of the route should be used. If the JSON body holds an ID it has to match the one in the route. IDs in JSON body could be optional but that would be inconsistent with the internal API?

This is not true as of my PR #2732 - this enforces that the parameter (id, slug, key) is passed as part of the options object.

However, it doesn't properly ignore what is in the body, so that's worth considering.

[sebgie]

@ErisDS yeah, I know that you took care of it 👍.

I was thinking doing a PR for the 201 and 415 from the table and open new issues for the list at the bottom if thats okay for you? None of them are critical to the ember project I guess. It would be nice if they are done before we label the API v1.0.

[ErisDS]

👍