artsy / force

The Artsy.net website
https://www.artsy.net
MIT License
595 stars 152 forks source link

chore: set Cache-Control: no-cache header appropriately on proxied MP req #14417

Closed mzikherman closed 2 weeks ago

mzikherman commented 2 weeks ago

What this does

In preparation for experimenting with a CDN in front of Metaphysics (which would eventually supplant Force's proxy + cache setup) - this adds 'standard' caching headers when we want to skip the cache.

We've experimented with proving out that our CDN will respect this standard header.

So, effectively mimicking Force's determination on whether the cache should be skipped for a particular query - logged in (which the CDN will also skip via existence of the access token header - but might as well include that check) - as well as queries that have been set to skip the cache via config.

In practice - there's not a ton of places that skip the cache b/c of cacheConfig while logged out:

Question for you @damassi - empirically based on experimentation - we see the behavior we want with this PR. Namely, the above logged-out MP queries do get the Cache-Control: no-cache header added, and have that force: true thing specified. This is good as they won't get cached in the CDN, and aren't being cached in Force. Without this PR, that cache header isn't set, and I can reproduce the CDN caching these!

However - I can't quite see where that happens - the auth query specifies { networkCacheConfig: { force: true } } (networkCacheConfig over cacheConfig), the time query specifies fetchPolicy: "network-only" (no cacheConfig at all), and the mutation doesn't seem to specify anything.

Yet - they all work as expected! Even though one is logged out, the cacheConfig: { force: true } value is set properly on all of these, and thus with this PR we wind up correctly setting Cache-Control: no-cache for these requests. Would love to know where that winds up happening though - mutations as well as these other two queries, time + authentication status, all have the force: true correctly propagating - is this somewhere in our Relay network layer/lib?

Next steps

With this PR merged, while there's a lot of overlap between the CDN cache and Force's cache - you can now point Force (via updating its METAPHYSICS_ENDPOINT) to an 'edge-cached' Metaphysics endpoint that has a Cloudflare worker doing some custom caching (digest of POST body, etc.)!

mzikherman commented 2 weeks ago

I'll merge this as it seems innocuous-enough to add this header to some proxied MP requests (based on the criteria).

damassi commented 1 week ago

Yet all of these work as expected

In relay there's multiple ways to skip the cache! All of these options are standard parts of the lib.