SGTM

Ok I will follow-up with a PR

I probably should have mentioned; for figuring out what to align to, you'll want to refer to this, not 7234.

@mnot thanks for pointing that out. That doc seems to clarify in more details which headers to update.

So I'm trying to understand how to best phrase this in Fetch, also in the light of 4.3.3.

My first question: is it possible for a server to respond with a 304, but with a different strong validator than the one found in the request? Even if possible, should the cache actually do something in such a case? That is actually the case I was thinking about initially, and my hunch was that the cache in such a case(received a 304, but cannot find any matching stored response) should make a new request that is not a validation request(See the current wording in the PR).

However, looking more closely at 4.3.3, it seems that the server in such a case would instead respond with a full response, and the cache could then use it(and MAY replace the stored once).

And that seems already covered by Fetch, via step 7.5 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch.

So perhaps receiving a 304, but being unable to match it with a stored response for update, is an edge case and perhaps a bug on the server(which should really send a full response back if it is unable to send a 304 matching the validator found in the validation request), and not something we should address in fetch? It doesn't seem to be addressed in the cache spec either...

One thing we can definitely do is flesh out 7.4 a bit more to align it with the https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.4.3.3, to handle the following case:

1. revalidating flag is set, but we get a "full response" => "the cache MUST use the full response to satisfy the request and MAY replace the stored response"
2. revalidating flag is set, but we get an 5xx back => "either forward this response to the requesting client, or act as if the server failed to respond. In the latter case, the cache MAY send a previously stored response"

And I think the "full response" seems to answer my previous question: a server should not send a 304 back if it would not match the validators found on the request. Although that is perhaps something the cache spec should address, essentially what to do with a 304 response that doesn't seem to match any of the stored responses, but I don't think it's something Fetch should address first.

@annevk perhaps the most effective change would be to remove the reference to 304 in Step 7.4 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch, and instead write something like: "if revalidationFlag is set: set response to the result of handling forwardResponse according to https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.4.3.3"?

We should handle server errors in a standardized manner. Returning the 304 as-is in that case seems reasonable. I do think it makes sense for Fetch to mention 304 specifically, so it's clear when CORS checks and such happen relative to it being handled.

Ok so on second thoughts, I think the fetch wording is actually pretty good as is. The 304 is handled explicitly, and the other cases are dealt with implicitly with Step 7.5, which would also cover the server error case(which would be forwarded as the response).

The edge case I mentioned initially(a 304 that does not match any stored response) doesn't seem to be handled by the http-cache spec yet, so I don't see how it could be handled explicitly in fetch either.

One thing we could do is maybe update the links to point to https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html ?

How does that handle it? I think it's better to wait for that to be published as RFC, unless this is particularly pressing.

Ok, should we then close this issue and the associated PR?

I think we should keep it open to track the issue where we set the revalidatingFlag and get a 304 but somehow not have a valid cached response. That needs addressing somehow and I don't think it can be fixed solely upstream given our current logic.

It's certainly possible for a 304 to contain a non-matching Etag, but the cache shouldn't update the response. Whether or not the client retries the request is up to it -- but usually it will, if it still wants a response. HTTP doesn't specify it because it's pretty generic, but I could see Fetch doing so (if you can get everyone to agree).

^ above comment made without refreshing the issue ...

HTTP doesn't specify it because it's pretty generic, but I could see Fetch doing so (if you can get everyone to agree).

Ok thanks for clarifying, so then Fetch is indeed the right place to handle this(opposite of what I suggested earlier).

Whether or not the client retries the request is up to it -- but usually it will

Just to clarify "retrying" in this context, would it mean sending the initial request, but stripped of any re-validation headers(essentially switching to a "normal" request for the full resource)?

This is sort of what I tried to sketch over at #1041

Yep. The details are what headers to strip; you could just strip the offending one, or all of them.