kit icon indicating copy to clipboard operation
kit copied to clipboard

Ability to customize caching behavior in `adapter-cloudflare`

Open pzuraq opened this issue 3 years ago • 8 comments

Describe the problem

The current Cloudflare adapter uses Cloudflare's Cache API to cache and return responses. This is really useful for cutting down on response time when pages are rendered often and are mostly static.

However, if you have a site which has different responses based on per-user state or settings, such as session state, then this caching behavior can't be used at all because any response could be different depending on the user. We can't even only cache when the user is logged out (often the most common case) because then a logged in user may receive a cached response where they are logged out.

This is partially resolved by setting the cache-control: private header, which bypasses the cache in Cloudflare and keeps local caching on the client, but it still means we can't use Cloudflare's cache and the logic for it just becomes dead weight for every request.

Describe the proposed solution

Ideally we would be able to add a function call prior to checking the cache to see if we should check the cache. For instance, in the user session case, you could add a call to check if the request has a session cookie, and if not check the cache for the not-logged-in version of the response.

I think generally most use cases would be around cookies, so the logic could just be checking for the presence of certain cookies, but it might be nice to have it be more generalized too.

Alternatives considered

Keep existing caching logic and use cache-control: private for all routes. If you happen to have a site without any session state, then the cache can be used, otherwise you can only rely on local caching.

Importance

would make my life easier

Additional Information

This could be applicable to other adapters, I'm not 100% sure as I haven't had a chance to check.

pzuraq avatar Apr 14 '22 12:04 pzuraq

I would also love to have this feature. Currently I’m dealing with a similar issue where user content is being cached no matter their session state, so they end up receiving incorrect data meant for another user.

As for your workaround using cache control, how did you manage to do it? I tried adding the private cache control header to my page endpoint but it didn’t help.

httpjamesm avatar Apr 20 '22 01:04 httpjamesm

@httpjamesm it's possible you still have values cached and serving up from before the header was added, also cache-control: private will still cache, but it will only cache locally in the browser not in cloudflare. If you don't want any caching at all, use cache-control: no-cache.

pzuraq avatar Apr 20 '22 19:04 pzuraq

After adding a hook, it worked. However doing it on a per route basis using page endpoints did not do it for me.

httpjamesm avatar Apr 20 '22 19:04 httpjamesm

If user content has a cache-control header, it must have a private directive (true whatever platform you're using), in which case the worker cache should ignore it — I'm not sure I see the need for a separate mechanism for opting out of caching.

Is this a documentation issue more than anything?

Rich-Harris avatar May 17 '22 20:05 Rich-Harris

No, this does need some extra functionality I believe. Consider the following scenario: You have a page which is a public page, that both logged in and logged out users can view. That gives us two possible response types:

  1. Logged out responses - these responses are all identical for all users, so they can be cached. In a page with high traffic volume, caching these pages may be very useful (we have massive on-sales where we'll be hit with thousands of requests in a matter of seconds, for instance).
  2. Logged in responses - these responses have differences for every user, such as the user's avatar in the nav bar and other user details. As such, we cannot return the cached response, and have to return the full response.

The issue with the current adapter is, if the logged out response is cached once, then all responses for that page will now short-circuit and return the logged out response. This is because there is no check to know if the user is logged in or not ahead of time, and the only way to get the cache-control header is to render the full page, which defeats the purpose of caching.

So we need some way to check ahead of time whether or not the cache should be used, which typically can be done based on the presence of a session cookie for instance.

pzuraq avatar May 18 '22 14:05 pzuraq

I believe Next.JS does this as well, which is why I don't have caching issues with it.

httpjamesm avatar May 18 '22 17:05 httpjamesm

Interesting. This feels like a challenge to do in a platform-agnostic way, do you have thoughts on what a good API would look like?

I wonder if caching it like this is too hacky:

let cache = new Map();

export async function handle({ event, resolve }) {
  if (can_used_cached_response(event)) {
    if (!cache.has(event.url.pathname)) {
      cache.set(event.url.pathname, resolve(event).then(response => {
        if (!response.ok) cache.delete(event.url.pathname);
        return response;
      }));
    }

    return cache.get(event.url.pathname);
  }

  return resolve(event);
}

Glossing over expiry, and obviously the cache would die with the worker, but it handles the 'thousands of requests in a matter of seconds' case.

Rich-Harris avatar May 24 '22 00:05 Rich-Harris

Yeah, I think the code for checking if it can be cached should be platform agnostic (e.g. a hook), but I also think that we should allow the cache to be pluggable. While an in memory store might work technically, I think it could change on each platform as the underlying implementation changes, and we really have no idea for sure how long it will cache for.

Maybe the platform/adapter could provide the implementation for the cache. I could imagine adding it in the adapter API, where if the file is present it would import it and pass it into the new Server() call somehow, otherwise it would default to the in-memory store. The plumbing for that could be useful for other platform specific functionality in the future as well.

pzuraq avatar May 24 '22 13:05 pzuraq