kovshenin / surge

Surge is a very simple and fast page caching plugin for WordPress.
GNU General Public License v3.0
152 stars 12 forks source link

Cache-Control: max-age not honored #12

Closed SilverMira closed 2 years ago

SilverMira commented 2 years ago

I'm trying to cache a WP Rest API response that calls to a 3rd party API, the 3rd party API takes few seconds to query.

Surge caches the response by default, everything is fine, except if I want the Rest API to hit the 3rd party API again after the max-age expires.

Looks like Surge only checks for no-cache and max-age=0, are there any plans to support variable number of max-age like header( 'Cache-Control: max-age=60' ); ?

kovshenin commented 2 years ago

Hmm, sounds like you're trying to use Surge as a way to cache the results of an external HTTP call. I'd recommend looking into something more suitable, like transients, or an option even, where you could be explicit about the duration of the cache and other factors (errors, rate limits, etc). If you would like to set the TTL based on the request URI, you could do it with a cache-config, here's an example:

https://roytanck.com/2022/01/13/tweaking-surge-by-adding-a-configuration-file/

Does that help?

SilverMira commented 2 years ago

Yes, in fact I'm using transient API currently to cache the 3rd party API call while using nocache_headers() to tell Surge to not cache the page, however, this raises another potential problem with downstream cache services like Cloudflare, as the no cache headers would tell them not to cache the response for X seconds and would require all request to hit the WP installation where the proper response gets fetched via transients, resulting in slower response.

To solve this, it seems like there is a need for a way to tell Surge not to cache a page other than the nocache_headers, so that the proper cache headers can reach downstream cache servers, or if Surge would follow the cache headers of max age so that it is in sync with downstream cache servers.

SilverMira commented 2 years ago

After reading the post, it looks like having a custom Surge config would indeed solve this problem, but however seems a little redundant than having Surge read the ttl from the header instead?

Any opinions?

kovshenin commented 2 years ago

My biggest concern really is that max-age is meant for clients, i.e. browsers, and not intermediate caches like Surge, proxy_cache or Cloudflare, so if something like this is supported, then probably through s-maxage and that whole family of directives designed for shared caches (which is somewhere on my list, but not a priority right now), but not max-age, so I'm not sure that will solve the problem you're facing.

The easiest way to deal with this (and other edge cases) is through a custom config. Thoughts?

SilverMira commented 2 years ago

Alright, custom config it is.

Thanks for clarifying, still, having the ttl for a request separated from the route response and needing to modify wp-config seems a little weird to me, but will do so for now.

kovshenin commented 2 years ago

Thanks. I'll give it a bit more thought and some more testing, ideally I want to make Surge compatible with upstream caches as well and respect different cache controls headers, both from the requester/proxy, as well as in the response. Thanks for bringing this up!