Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maxStale alone never send back cache #39

Closed
wants to merge 1 commit into from
Closed

maxStale alone never send back cache #39

wants to merge 1 commit into from

Conversation

matthieurosset
Copy link

Hello,

Thanks a lot for your work, your package is really helpful.

I found an issue in DioCacheInterceptor.onError. If I define a Duration for maxStale but I don't define any exception for hitCacheOnErrorExcept, interceptor never send me back my cache when an error occurs (for example : an error 500). In my case, I don't want to define any exception.

Hope this helps !

@llfbandit
Copy link
Owner

Thanks but you only need to provide empty hitCacheOnErrorExcept list to make this work.
Am I wrong ?

@matthieurosset
Copy link
Author

No, you're right, it can be that way. But in this case, add a required to this parameter or define an empty array by default. I had to debug your package to understand why my cache was not returned.

@llfbandit
Copy link
Owner

null value => No cache hit on error.
value => Cache hit on error, excepted given status codes.

So, as we need both, we must keep hitCacheOnErrorExcept optional.

@matthieurosset
Copy link
Author

If you don't define maxStale, what do you expect ? Return a cache without time limit ?

What I expected was "no cache on error".

@llfbandit
Copy link
Owner

maxStale is not related to hitCacheOnErrorExcept.
With maxStale, the cache store won't provide the cached value if maxStale is in the past.
This is mostly useful if the origin server has no cache set up.

Otherwise, the cached value expires according standard HTTP expiration headers.

@matthieurosset
Copy link
Author

matthieurosset commented Oct 5, 2021

Okay, I understand your point for maxStale.

For me, it's still weird to give an empty list to an optional exception list to hit my cache in case of errors. If I don't have any exceptions, I expect to get all of my items back and not any of them. Looks like an hidden feature. An additionnal bool parameter "hitCacheOnError" would be more clear.

@llfbandit llfbandit closed this Dec 27, 2021
@matthieurosset matthieurosset deleted the hotfix-interceptor-maxStale-on-error branch May 20, 2022 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants