jvalue / ods

Open Data Service - Make consuming open data easy, safe, and reliable
GNU Affero General Public License v3.0
36 stars 23 forks source link

API Design Discussion: Failing Delete and Update #388

Closed georg-schwarz closed 2 years ago

georg-schwarz commented 2 years ago

Hey, since it came up in #387, I want to discuss a small part of a potential API design guide.

What should response codes be in our RESTful HTTP API towards the UI/clients.

General

CREATE

UPDATE

DELETE

GET

Discussion Points

(1) The question still open is, how do we handle requests specifying an id that does not exist.

404 might be ambiguous since it is also returned if the endpoint does not exist at all. 400 indicates a malformed request, which also might mean something else is off. 204 indicates a success, but it wasn't really successful. I have read very different opinions on how to design it, so I'd like to have your thoughts and we should agree on one final result.

Thoughts?

@lunedis @MBuchalik @r21gh

lunedis commented 2 years ago

Really depends on how REST-ful you want to be, right? If I remember correctly, in "proper" REST-style, the existance of an URL like /data/123 implies the existance of the corresponding UPDATE and DELETE calls, and as such, they should return 404.

MBuchalik commented 2 years ago

If a request like "GET /data/123" points to a non-existing ID, I would probably just return a 404. Or, as described in https://stackoverflow.com/questions/20922543/correct-http-status-code-for-existent-resource-but-non-existent-entity, you could return 422. Just returning 400 would only make sense to me if you send a completely invalid identifier (like "hello" instead of a number). So, I would assume that a failing DELETE or UPDATE should behave the same, right?

The question is of course - shold we return 400 or 404 if an endpoint does not exist? I honestly don't know.

One of the reasons why I personally don't like using the http verbs and status codes :D

In some of my APIs, I literally always return 200, no matter what happended. The response contains a boolean field "success". So, a response may look like this: ``` js { success: true, data: { // ... } } ``` ``` js { success: false, error: /* Mabye, an entry from an enum. Or some additional data. Semantically, it is completely clear what the error means. Which is the ultimate goal of error handling for an API. */ } ``` This makes it super easy to create types in TS. And you don't need to think about status codes and the like. But it is probably the opposite of a proper REST API :D
georg-schwarz commented 2 years ago

I like the idea of the 422 code. Anyone disagreeing?

r21gh commented 2 years ago

I think HTTP code 422 is a more appropriate response if the data is understood by the server but is still not valid (invalid format). And in the case of an unavailable URL (with or without query params), I would say 404. https://datatracker.ietf.org/doc/html/rfc4918#section-11.2

georg-schwarz commented 2 years ago

Okay, I also had another read and I guess common sense is to use 404 in those cases (entity does not exist). If I remember correctly, this should be the case in most places of the APIs.

lunedis commented 2 years ago

Agreed, my stance was always to use 404 there.

MBuchalik commented 2 years ago

GitHub also uses 422 for non-existing IDs (or similar). See for example https://api.github.com/repos/jvalue/ods/commits/this-does-not-exist But I guess there is no "this is absolutely the correct status code" for this case, so 404 is probably fine as well.