remotestorage / spec

remoteStorage Protocol Specification
https://tools.ietf.org/html/draft-dejong-remotestorage
87 stars 5 forks source link

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

Open michielbdejong opened 3 months ago

michielbdejong commented 3 months ago

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 commented 3 months ago
  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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.