undici
undici copied to clipboard
Cache cacheable response with an error status code
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 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,
- 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()
))
})
I'm aligned with the proposal, but have my concerns about going straight ahead into cashing non-2xx responses as it might be a breaking change for current usage.
I'd advice the exploration of extending the interceptors option, and make the caching of final responses the new default for further major
Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.
Sorry for the answer delay. Thanks for having done this. (Maybe I would have had a look on next Sunday, but that is not a certainty.)
I realized I was wrong stating it should be changed, "realign with the RFC", because the RFC does not mandate caching in any case (only write "may cache").