use-http icon indicating copy to clipboard operation
use-http copied to clipboard

Discussion with cache stale and network error

Open SrMouraSilva opened this issue 4 years ago • 8 comments

From what I understand from the documentation, when cachePolicy = cache-first and cacheLife > 0, the cached data is returned if it has not expired. If the cache has expired, a request is made on the server.

Doubt: If there is an error in the request (such as, for example, a cell phone went offline), the error is returned, but not the cache?

Another question, where is the cache? In memory? If I use the library in react native and place a 24-hour cache, after obtaining the data in a request I close the application and open it again, is a request made?

Another thing, how does the cache with pagination work? If the data is not stale, is only the first page returned?

That said, I would find it interesting that the library would allow:

  1. Allow the creation of different forms of cache persistence, so someone could create a plugin for react-native that uses async-storage, for example
  2. Add some configuration that allows the cache data to be returned when there is an error in the request
  3. Add some return attribute of object response (such as loading, error ...) that informs you if the data in question is new or is a cached data
  4. Inform the time of the returned response. I think that's possible, but I haven't figured it out yet

SrMouraSilva avatar Mar 06 '20 20:03 SrMouraSilva

From what I understand from the documentation, when cachePolicy = cache-first and cacheLife > 0, the cached data is returned if it has not expired. If the cache has expired, a request is made on the server.

This is correct.

Doubt: If there is an error in the request (such as, for example, a cell phone went offline), the error is returned, but not the cache?

False. The cached data will be returned. If there is an error, it should be reset on each request. Right now it only get's reset if there is no cached response or in other words, it's making another http request (which I believe is a bug that I might need to change).

Another question, where is the cache? In memory? If I use the library in react native and place a 24-hour cache, after obtaining the data in a request I close the application and open it again, is a request made?

I have plans to allow the user to pass their own cache into useFetch or <Provider cache={new Map()} /> but haven't gotten there yet. Right now it's just in memory. I also have plans to build this persistent cache (see this issue), but haven't gotten to it yet. I've been prioritizing current bugs and Suspense support.

Another thing, how does the cache with pagination work? If the data is not stale, is only the first page returned?

Look at it like this.

// it's actually a js Map, but for understanding we'll just use an object
const cache = {}

// 1. fetch page 1 - the `response` below is a Promise
cache['url:https://example.com?page=1||method:GET||body:'] = response

// 2. fetch page 2
cache['url:https://example.com?page=2||method:GET||body:'] = response

// 3. fetch page 1 again, but we pull from the cache this time because we already have this in the cache

This might not be a good explanation. If you need more clarification, let me know. There are also probably some additional cases that need to be more thought out.

  1. I have plans to build this type of storage that has the same syntax for both react-native and basic browser with use-react-storge.
  2. Hm... can you elaborate more?
  3. That's an interesting idea. I'm open to this idea.
  4. We save this information in the cache as well so this can be done.

alex-cory avatar Mar 06 '20 20:03 alex-cory

Doubt: If there is an error in the request (such as, for example, a cell phone went offline), the error is returned, but not the cache? False. The cached data will be returned. If there is an error, it should be reset on each request. Right now it only get's reset if there is no cached response or in other words, it's making another http request (which I believe is a bug that I might need to change).

  1. Add some configuration that allows the cache data to be returned when there is an error in the request

If there was an error in the request and there was a cache, it should return the cache, even if it is slate. You have confirmed that this does happen, so it is already ok. I didn't understand what this reset is.

Look at it like this. code This might not be a good explanation. If you need more clarification, let me know. There are also probably some additional cases that need to be more thought out.

I understand the example. But consider this case:

I have a mobile application that has an infinite scrollable list, that is:

  • When the list is visible, the first page is requested.
  • When the scroll approaches the end of the list, the next page is loaded. With the new data, the list increases a little more and I can scroll more until I reach the end, and;
  • When the scroll approaches the end of the list, the next page is loaded ...

In an example case, I have a 2 minute cache. If I exit the app, open it again and go to the list screen (in less than 2 minutes), I hope the full list will be loaded (as far as the cache saved). I have a simple useFetch implementation that it:

  • Concatenates the answer with the data on the new page;
  • Stores the concatenated response in the cache, but the cache time is not updated to +2 minutes;
  • If I want to reload the entire list (pull to update), my useFetch returns the reload function for this;
  • If by chance when obtaining a new page I give an internet error and I want to manually try the last request again, my useFetch returns the retry function for this;

My implementation is simple and I am thinking of leaving it if I could use use-http. For now, as I need to save the cache, I still can't.

SrMouraSilva avatar Mar 06 '20 22:03 SrMouraSilva

If there was an error in the request and there was a cache, it should return the cache, even if it is slate. You have confirmed that this does happen, so it is already ok. I didn't understand what this reset is.

Ahh, I see what you are saying.

  1. make a request, cache the response
  2. cacheLife expires
  3. make same request, if it errors, use the cached response, otherwise replace the cached response with the new response.

This is not implemented this way currently. When you make a request, if the age of the cache < cacheLife we immediately delete the cached response and retry the request. Do you think this should be the default behavior? If not, do you have any ideas for how syntax should look to make this work? I need to think about this some more...

Regarding your example, does something like this solve some of your problems?

const [page, setPage] useState(1)
const perPage = 15

// could probably add `cache` (NOT IMPLEMENTED)
const { loading, error, data: todos, get, cache } = useFetch({
  path: `/todos?page=${page}&perPage=${perPage}`,
  onNewData: (currTodos, newTodos) => [...currTodos, ...newTodos],
  perPage,
  data: [],
  useCacheOnError: true, // maybe an option like this? (NOT IMPLEMENTED)
  persist: true, // keep the cache in localStorage (NOT IMPLEMENTED)
  cacheLife: 2 * 60 * 1000 // 2 minutes until we remove the cache from localStorage in this case
}, [page])

const handlePullToUpdate = async () => {
  cache.clear() // we could probably do something like this (NOT IMPLEMENTED)
  setPage(1)
}

return (
  <ScrollListener
    onLoadMore={() => setPage(page + 1)}
    onPullToUpdate={handlePullToUpdate}
  >
    {todos.map(todo => <div key={todo.id}>{todo.title}</div>}
    {loading && <Loading />}
    {error && <ManuallyReloadButton onClick={get} />}
  </ScrollListener>
)

alex-cory avatar Mar 06 '20 23:03 alex-cory

This is not implemented this way currently. When you make a request, if the age of the cache < cacheLife we immediately delete the cached response and retry the request. Do you think this should be the default behavior? If not, do you have any ideas for how syntax should look to make this work? I need to think about this some more...

I think there could be a different policy with this behavior. See this image. My implementation is based on it. The image is from the apipeline library

APIPeline cache policy

const handlePullToUpdate = async () => {
  cache.clear() // we could probably do something like this (NOT IMPLEMENTED)
  setPage(1)
}

If the cache were cleared and this error occurred in the new request, nothing would be returned! Calling get wouldn't work either, as it would use the cache. That's why I have a specific function for this.

I liked the idea of loading and ManuallyReloadButton at the end of the list. If the nth page fails, a button would appear at the bottom of the list where the user could manually request a reload. And get would make the request for the last page.

SrMouraSilva avatar Mar 07 '20 00:03 SrMouraSilva

I see what you're saying. I like the idea of another cache policy. I will take a closer look at this as soon as I'm done with Suspense support.

I really appreciate this feedback!

In the meantime, feel free to poke around the library and submit a PR if you get to it before me!

Side note, I think I might still add the cache to the response because I think it might be useful for debugging purposes. What do you think?

alex-cory avatar Mar 07 '20 01:03 alex-cory

I'm done with Suspense support. Suspense support is cool

In the meantime, feel free to poke around the library and submit a PR if you get to it before me!

I would like to, but I still need to mature some concepts before. Some things to avoid unnecessary rerendering, further deepening with reduce and states, understand why this useRef is for.

Side note, I think I might still add the cache to the response because I think it might be useful for debugging purposes. What do you think?

In the flow of my application, when information from the internet is requested:

  1. Try to make the request, loading = true
  2. If the data isn't stale, the cache is returned
  3. If it is stale and it was possible to obtain the data from the network, the data is displayed on the screen
  4. If there was a problem with the request, the cache is displayed and a message appears (by Toast) stating that it was not possible to update the data and that the user should check if there is a connection.
  5. If the user wants, he can try to request the update of the data manually (like pull to update).
  6. Nothing is displayed on the screen (empty state) if there is no cache (or the cache is disabled) and the network fails

In addition, if the user clicks on this information, the day and time of the information is displayed.

In this sense, it would be important for me to know if the data comes from the cache or the network, if there were requisition problems and also to know the information time. Some REST services return the time in the response header, so if you save the entire response, not just the body, it would work.

So, cache as boolean or an object with some data as time to become obsolete, reason why the cache is returned would be useful in addition to debugging

SrMouraSilva avatar Mar 08 '20 01:03 SrMouraSilva

The useRefs are used to reduce rerendering. We only have 1 state update, that's to loading.

I suggest you take a deeper look into what the cache-policies that aren't implemented yet to see if any of them fit your needs. Otherwise yeah, we can just implement a new one.

alex-cory avatar Mar 09 '20 22:03 alex-cory

The cache-and-network policy would fit pretty well with this use case.

cacheLife could specify the TTL during which no additional network request needs to be made:

Cache strategy No cached data Fresh cache Expired cache (via cacheLife)
apollo cache-and-network loading + request cached + request (not supported)
use-http cache-first loading + request cached loading(?) + request
use-http cache-and-network (proposed) loading + request cached cached + request

Edit: Might be better to always return cached data via cachedData or similar, even if it is stale/expired. The behaviour above goes against the guarantee that cache-and-network always triggers a request:

However, regardless of whether or not the full data is in your cache this fetchPolicy will always execute query with the network interface...

mogelbrod avatar Mar 10 '20 15:03 mogelbrod