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

Easier way to attach ETag to If-None-Match after #3562 #3718

Closed
SukkaW opened this issue Oct 11, 2024 · 4 comments · Fixed by #3758
Closed

Easier way to attach ETag to If-None-Match after #3562 #3718

SukkaW opened this issue Oct 11, 2024 · 4 comments · Fixed by #3758
Labels
enhancement New feature or request

Comments

@SukkaW
Copy link
Contributor

SukkaW commented Oct 11, 2024

This would solve...

#3562 introduces HTTP Caching (RFC9110) to undici, which also implements HTTP 304 with If-Modified-Since. But it seems that ETag is missing.

cc @flakey5

The implementation should look like...

IMHO #3562 should not be delayed anymore, this could be implemented after #3562 is merged.

The undici could check the stored ETag header from the cache store and use that to negotiate with the If-None-Match in future requests. If the server returns HTTP 304, undici would then return the cached (transparent) HTTP 200 response to the client (the same behavior of fetch as in browsers).

Additional context

#3562

JakeChampion/fetch#241

ETag is described in RFC9110 here: https://httpwg.org/specs/rfc9110.html#field.etag
If-None-Match is described in RFC9110 here: https://httpwg.org/specs/rfc9110.html#field.if-none-match
HTTP 304 is described in RFC9110 here: https://httpwg.org/specs/rfc9110.html#status.304
The handling of HTTP 304 is described in RFC9110 here: https://httpwg.org/specs/rfc9111.html#freshening.responses

@SukkaW SukkaW added the enhancement New feature or request label Oct 11, 2024
@SukkaW
Copy link
Contributor Author

SukkaW commented Oct 14, 2024

I have a question. In the fetch spec, 304 (along with some other status codes like 101 and 204) are null body status (https://fetch.spec.whatwg.org/#concept-status).

undici could return a "transparent" cached Response object with 200 OKs when the server returns 304 (which is also what browsers do), which consists as I mentioned.

But in the context of Node.js, some people might also want to know whether the response is from cached or not.

@mcollina
Copy link
Member

We haven't really assessed if/how this would integrate with fetch.

flakey5 added a commit to flakey5/undici that referenced this issue Oct 21, 2024
@thewilkybarkid
Copy link

But in the context of Node.js, some people might also want to know whether the response is from cached or not.

We currently use make-fetch-happen and their X-Local-Cache-Status header to see if the response is stale is not (https://github.com/PREreview/prereview.org/blob/2fc7a676df9951494d67aa1fa3d275818fd6796d/src/fetch.ts#L27). While in this case it should be possible to use the Age header, it's generally far easier to have custom headers for this.

@SukkaW
Copy link
Contributor Author

SukkaW commented Oct 24, 2024

But in the context of Node.js, some people might also want to know whether the response is from cached or not.

We currently use make-fetch-happen and their X-Local-Cache-Status header to see if the response is stale is not (https://github.com/PREreview/prereview.org/blob/2fc7a676df9951494d67aa1fa3d275818fd6796d/src/fetch.ts#L27). While in this case it should be possible to use the Age header, it's generally far easier to have custom headers for this.

I am also using make-fetch-happen and X-Local-Cache-Status.

But with undici, it is possible to use undici.request over undici.fetch, and undici could add a cacheStatus property on the return value. We shall see!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants