non-2xx (including 5xx) responses are being cached

[benarmston]

When cachePolicy is set to cache-first, non-2xx responses are cached. I want to be able to avoid caching non-2xx response.

The codesandbox uses a 404 response as that is what was easiest for me to use there. However, the actual issue I ran into was with a 503 response. Caching 5xx responses is a bad idea.

** Codesandbox **

https://codesandbox.io/s/kind-christian-x2he1?file=/src/App.js

To Reproduce
Steps to reproduce the behavior:

1. Go to https://codesandbox.io/s/kind-christian-x2he1?file=/src/App.js
2. Look in the console
3. See that the 404 response was cached.

Expected behavior

Either non-2xx responses do not get cached; or there should be an option so that only 2xx responses are cached.

[iamthesiz]

Try

const { cache } = useFetch()
cache.delete('cache-key')

[benarmston]

Thanks for the response @alex-cory. I'm not sure that I fully understand how to use cache.delete.

Perhaps my codesandbox was a little too simple. I've updated the codesandbox to be a little closer to how I am using useFetch. I have a useFetchMyResource function which encapsulates various logic around making the request; namely which headers get set, which data the request depends on, cache policy, etc..

It is in the useFetchMyResource function that I'd like to add the policy of not caching non-2xx responses. I've added a response interceptor which checks if the response was OK and deletes it from the cache if not. However, it appears that the response is cached after the interceptors run so the interceptor has no effect.

A trimmed down version of useFetchMyResource has been added to the code sandbox and is repeated here. How can I delete the response from the cache inside of useFetchMyResource?

function useFetchMyResource() {
  const { cache } = useFetch({ persist: true, cachePolicy: 'cache-first' });
  return useFetch(
    "/my-url",
    {
      persist: true,
      cacheLife: 10 * 60 * 1000, // 10 minutes in ms
      cachePolicy: 'cache-first',
      interceptors: {
        response: ({ response }) => {
          if (!response.ok) {
            cache.delete('/my-url');
          }
          return response;
        },
      },
    },
    []);
}

Thanks for your help.

[benarmston]

After more experimentation, I have something that kind of works. I've updated the codesandbox and I'll repeat the useFetchMyResource function here.

function useFetchMyResource() {
  const request = useFetch(
    url,
    {
      persist: true,
      cacheLife: 10 * 60 * 1000, // 10 minutes in ms
      cachePolicy: "cache-first"
    },
    []
  );

  if (request.error) {
    // Using `url` does not work.
    // request.cache.delete(url);

    // Need to reconstruct the cache key manually.
   const cacheKey = `url:${url}||method:GET||body:`;
   request.cache.delete(cacheKey)
  }

  return request;
}

Unfortunately, getting this to work requires that the cache key is manually reconstructed. Explicitly removing the cache for bad responses would be OK if I had access to the response id. Would it be possible for you to expose the response id (#316)?

I still think that 4xx and 5xx responses should not be cached, or that there should be some option to not cache them. Perhaps a cacheWhen function taking a response and returning a boolean?

[iamthesiz]

Hm... does the normal cache-first caching strategy not cache 4xx and 5xx responses? I guess that would make more sense. I'd accept a PR for this to not cache on 4xx and 5xx. My only concern is, is this standard. Also, I'd accept a PR for exposing id. Maybe add that ID onto request.id

[SyFlo]

@alex-cory any news on that one? I'm currently struggling with the same issues as @benarmston