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

Status code 404 should not be used for empty lists #207

Closed
StijnGroenen opened this issue Jun 18, 2020 · 5 comments · Fixed by #248
Closed

Status code 404 should not be used for empty lists #207

StijnGroenen opened this issue Jun 18, 2020 · 5 comments · Fixed by #248
Assignees
Labels
bug Something isn't working Planned Feature is planned for a future release

Comments

@StijnGroenen
Copy link
Member

StijnGroenen commented Jun 18, 2020

Describe the bug
Error code 404 should not be used when there is nothing to return when getting a list. The 400-499 range of HTTP status codes indicate a client error. When the list of highlights is empty, it is not a client error. When the request specifies a specific ID that does not exist. The 404 Not found status code is correct.
In my opinion, status code 200 (OK) with an empty array as the body or status code 204 (No content) should be used.

To Reproduce
Steps to reproduce the behaviour:

  1. Make a request to the get all highlights endpoint when there are no highlights.
  2. A 404 Not Found status code is returned

Expected behaviour
Status code 200 OK should be returned. The body of this response can contain an (empty) array just like when there are highlights in the database.

Edit: This was discussed in a meeting with the majority of the backend developers. We decided that using 404 was not practical for our implementation.

@StijnGroenen StijnGroenen added bug Something isn't working Planned Feature is planned for a future release labels Jun 18, 2020
@Brend-Smits Brend-Smits added this to To do in Sprint 5 - Backend via automation Sep 15, 2020
@Brend-Smits
Copy link
Member

The postman tests will also need to be changed accordingly.

@Brend-Smits
Copy link
Member

This one I really want to get done during this sprint. Anyone that wants to fix this from @DigitalExcellence/backend ?

@niraymak niraymak self-assigned this Sep 22, 2020
@Brend-Smits Brend-Smits moved this from To do to In progress in Sprint 5 - Backend Sep 29, 2020
Sprint 5 - Backend automation moved this from In progress to Done Oct 9, 2020
@jcoenen96
Copy link
Contributor

jcoenen96 commented Oct 26, 2020

This actually has to be 404 and not 204. This has been discussed multiple times last semester. This has to do that it is an API and not a website.

https://developer.mozilla.org/nl/docs/Web/HTTP/Status
Check 404 not found

@Brend-Smits
Copy link
Member

This actually has to be 404 and not 204. This has been discussed multiple times last semester. This has to do that it is an API and not a website.

Yes, it was also discussed that it was not practical to enforce this.

@jcoenen96
Copy link
Contributor

Then update the issue that this has been discussed. Currently, the issue indicates that the protocol has been implemented incorrectly which is not true. I can't know when something has been discussed when it isn't described in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Planned Feature is planned for a future release
Projects
No open projects
4 participants