Only set cache for 200, 301 and 302 response

[adityapatadia]

Currently cache-control header is sent for even 404 response. Can we avoid it?

[mcollina]

Thanks for reporting this!

It definitely should, would you like to send a PR?

You should add a check for the status codes in https://github.com/fastify/fastify-caching/blob/master/plugin.js#L43-L52.

[adityapatadia]

I am talking about this line: https://github.com/fastify/fastify-caching/blob/master/plugin.js#L72

[mcollina]

I think that should move to a onSend hook instead.

[adityapatadia]

Well I thought about it and current one seems fine to me. I set cache control to private by default and when I need the response to be cached, I set the header with reply.header. If we move it there, there must be a check that it should not set cache-control header if something in the app has already set it.

[mcollina]

Yes exactly.

[prevostc]

I did run into an issue with this plugin caching 429 responses from https://github.com/fastify/fastify-rate-limit
Still trying to bypass this behavior.

@mcollina I would like to contribute a PR for this. What option do you most like

• add a config to enable/disable caching for specific http codes
• add a new config like "shouldCache: (req, reply) => boolean"
• only set cache headers only if they have not been set
• something else?

[mcollina]

only set cache headers only if they have not been set

I think this is the best option.