-
-
Notifications
You must be signed in to change notification settings - Fork 660
Description
This would solve...
#3231 introduced HTTP caching (great job!), but in my tests it does not cache cacheable responses with an error status code (specifically, 404 in my case, but all cacheable responses should could be cached, included 204, 400, ...), and responses with error status codes are cacheable according to RFC 9111.
The implementation should look like...
If there is actually a restriction on cacheable status codes in Undici going beyond what the RFC mandates:
- maybe realign with the spec, (Edit: I realized later it was still aligned since the spec allow to be more restrictive.)
- or introduce an option in the cache interceptor to relax this constraint.
I have also considered...
I have checked my test request gets a proper cache-control header in its 404 response (public,max-age=60
in my case), and repeatedly calling it from the node server does always actually call the external API. Doing the same test with a request, replied with a 200 and the same cache directive, does not always actually call the external API. Doing the same tests with these requests from a browser does actually cache both cases, 404 and 200 response cases.
Additional context
This is a nice to have in my opinion, because most gains are already there with the caching of 200 responses, which are usually much more common than other responses. Again, great job with this feature which I really appreciate, and many thanks for it.
These tests were done with Undici 7.10.0 on a Node 22.15.1, just overriding the global dispatcher, still using the default fetch (through ofetch actually, used through Nuxt useFetch). Caching was activated along with HTTP/2 in a Nitro plugin:
import { setGlobalDispatcher, Agent, interceptors } from 'undici'
export default defineNitroPlugin(() => {
setGlobalDispatcher((new Agent({
allowH2: true
})).compose(
interceptors.cache()
))
})