Skip to content
This repository has been archived by the owner on Feb 28, 2023. It is now read-only.

Get stale cache on network error #20

Closed
FMCorz opened this issue Nov 8, 2017 · 4 comments · Fixed by #21
Closed

Get stale cache on network error #20

FMCorz opened this issue Nov 8, 2017 · 4 comments · Fixed by #21

Comments

@FMCorz
Copy link
Contributor

FMCorz commented Nov 8, 2017

Hi,

I would like to be able to return the cache when Axios returns a network error. From what I can see it is not possible as it is because the cache store throws an error when the cache is stale. And also because there is no wrapper try/catch around the call to the default adapter. Is that right?

Would you be interested in supporting such feature? If yes, how would you recommend it to be coded?

Thanks!
Fred

@FMCorz
Copy link
Contributor Author

FMCorz commented Nov 8, 2017

I've put something together, let me know what you think:

https://gist.github.com/FMCorz/540bdd3e740d69dbf84fa0dc17cea940

@ghost
Copy link

ghost commented Nov 8, 2017

Hi @FMCorz !

Awesome idea 👍

There's several things to consider when developing such a fallback.

First you're right there is no try/catch around the default adapter call because I wanted to leave that burden in the user land.

Your implementation looks fine! 😃

We should also consider if this action should trigger on any kind of network error (4xx, 5xx).
Maybe there should be a second maxAge for stale cache being served when a network error occurs so that at some point the error is actually detected and fixed?

I guess we could have an option

// Simple implementation for any kind of error
const cache = setupCache({
  readOnError: true
})

// Only for 5xx errors
const cache = setupCache({
  readOnError: {
    // Some kind of regex against which we can compare the response.status
    status: /5../,
    // Only read cache 5 times for a same error (this is maybe not a good idea)
    limit: 5,
    // Another maxAge for stale content?
    maxAge: 15 * 60 * 1000
  }
})

Any of these would automatically set the clearOnStale option to false.

I'm probably overthinking it. Maybe we can implement the simple version first and see from there.

The first cache read (before using the default adapter) should still throw a cache-stale error so that we actually hit the network.
Add a try/catch around the network call.
When network error is caught re-call the cache read but preventing the cache-stale error from throwing so that data is returned (probably by passing a acceptStale param or something to the cache reader).

Do you want to have a go at this? Else I'll give it a try soon.

Cheers 🍻

@ghost ghost added this to the 2.1.0 milestone Nov 8, 2017
@ghost
Copy link

ghost commented Nov 8, 2017

Ok this got me intrigued so I setup a branch trying to add this feature check out the linked PR 😃

@FMCorz
Copy link
Contributor Author

FMCorz commented Nov 9, 2017

Heh, I didn't have time to give it a go ;-). I agree with your comments. You will see in the comments I left on the PR what I think about handling the statuses, etc... It might be better to have functions that do these things, rather than trying to cater for all needs at once. Example:

const isNetworkError = function(e, req) {
  return e.request;
}

const isResponse5xx = function(e, req) {
  return e.response && e.response.status >= 500 && e.response.status < 600;
}

const combineHandlers = function(...handlers) {
  return function(e, req) {
    return !!handlers.find(handler => handler(e, req));
  }
}

const cache = setupCache({
  staleOnError: true // Not recommended as hides side effects.
})

const cache = setupCache({
  staleOnError: isNetworkError
})

const cache = setupCache({
  staleOnError: isResponse5xx
})

const cache = setupCache({
  staleOnError: combineHandlers(isNetworkError, isResponse5xx)
})

Now, in these examples you cannot substitute the response, you can only dynamically tell whether the stale cache should be used. But another issue could be adding onError, which default implementation would be to apply staleOnError.

What do you think?

@ghost ghost modified the milestones: 2.2.0, 2.3.0 Oct 11, 2018
@ghost ghost modified the milestones: 2.3.0, 2.2.0 Jan 16, 2019
@ghost ghost closed this as completed in #21 Jan 17, 2019
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant