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

Should 304/412 response take preference over 404? #191

Open
michielbdejong opened this issue Jun 12, 2024 · 5 comments
Open

Should 304/412 response take preference over 404? #191

michielbdejong opened this issue Jun 12, 2024 · 5 comments

Comments

@michielbdejong
Copy link
Member

michielbdejong commented Jun 12, 2024

As reported via email by Walson Low:

section 5 of draft-22 [...] states:
...
404 for all DELETE, GET and HEAD requests to documents that do not exist on the storage
412 for a conditional PUT or DELETE request whose precondition fails (see "Versioning" below)
...

How should the storage respond to PUT requests with an "If-Match" header for documents that do not exist on the storage? The clients, in this case, expect the document to exist (due to the "If-Match" header), but the document has been removed from storage.
For DELETE requests with an "If-Match" header, the storage responds with a 404 status code if the document does not exist.
Should the storage respond with a 404 or 412 status code for PUT requests with an "If-Match" header for documents that do not exist? I believe it would be consistent to respond with a 404, similar to the behavior for DELETE requests.

My answer:

My intuition would be that 412 takes preference over 404, but I don't have a good reason for it.
I guess it would require less code on the client because they already have to handle 412 anyway, for the case where the resource does exist but has a different ETag.

Maybe we should change that first line to say "404 for all unconditional DELETE, GET and HEAD requests to documents that do not exist on the storage"?

@raucao
Copy link
Member

raucao commented Jun 12, 2024

  1. Wouldn't conditional GET/HEAD requests still require a 404? (If-Modified-Since, If-None-Match to non-existing resource)
  2. As you say, after a 412, the client has to find out what the conflict is by fetching the resource anyway, which is when it would see the 404.

@darkdread
Copy link

I think status code 404 should take precedence over 412. The reason for it is because with 404, the client knows​ that it was removed.
The removal of a document gives client more information than modification, because with modification, it could be either:

  1. It was removed
  2. It was modified

This allows the client to decide their next course of action with a single request.

@michielbdejong michielbdejong changed the title Should 412 response take preference over 404? Should 304/412 response take preference over 404? Jun 13, 2024
@michielbdejong
Copy link
Member Author

Right, if we let 412 take preference or not over 404 for methods that apply server-side changes, then we should probably take the same decision for letting 304 take preference or not over 404 for GET and HEAD.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match

When the condition fails for GET and HEAD methods, then the server must return HTTP status code 304 (Not Modified). For methods that apply server-side changes, the status code 412 (Precondition Failed) is used.

I see @darkdread's point (and I think @raucao is making the same point in his point 2.) that a 404 is more useful because it gives more specific information than a 412. After all, the information that the precondition failed can be derived from the information that the resource doesn't exist, but the reverse doesn't hold true, so that would in some cases be a waste of roundtrips.

OTOH, 404 is categorised as a "client error" by https://www.rfc-editor.org/rfc/rfc9110#name-client-error-4xx which also feels like the wrong category.

I think the balance is tipping towards letting 404 take preference, but I feel like we don't have a definitive basis for a decision yet.

We could also explicitly leave it up to the server but that would mean some clients would have to write extra code paths to deal with different server behaviours which I think is not nice.

@toomim you know a lot about conditional HTTP requests, what do you think the remoteStorage spec should say about this?

@DougReeder
Copy link
Contributor

I think a better way to think about HTTP status categories is: Who must act to resolve the issue? For 4xx errors, the client must take action; for 5xx errors, the server (or gateway) must take action. So, 404 isn't an "error" by the client so much as the indication that its model is out-of-date

FWIW, S3 storage favors 404 over 412, when a resource doesn't exist:
NoSuchVersion The version ID specified in the request does not match an existing version. 404 Not Found
from https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html

I would favor 404 over 412, when a resource doesn't exist.

@raucao
Copy link
Member

raucao commented Jun 13, 2024

I think a better way to think about HTTP status categories is: Who must act to resolve the issue? For 4xx errors, the client must take action; for 5xx errors, the server (or gateway) must take action.

That's a great explanation!

It seems to me like 404 is the way to go then.

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

No branches or pull requests

4 participants