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

maxTtlIfError behavior clarification. #27

Open
JustinTRoss opened this issue Jan 18, 2022 · 5 comments
Open

maxTtlIfError behavior clarification. #27

JustinTRoss opened this issue Jan 18, 2022 · 5 comments

Comments

@JustinTRoss
Copy link
Contributor

This line makes it appear maxTtlIfError is whatever value is provided in seconds for maxTtlIfError

maxTtlIfError: 30 * 60, // 30min, will respond with the cached response in case of an error (for further 20min)

This line makes it appear as though maxTtlIfError actually ends up being maxTtl + maxTtlIfError.

ttl: request.requestCache.maxTtl + request.requestCache.maxTtlIfError,

I'm happy to PR clarification once I have it. Just need to know what that is 🙃 .

@StarpTech
Copy link
Owner

StarpTech commented Feb 6, 2022

Hi @JustinTRoss, you're right this is confusing. The TTL of the error cache must always be longer than the default TTL cache.
maxTtlIfError defines the exact duration in seconds of the error cache. We should update docs with this, for example:

maxTtl: 60 // cache requests for 1min
maxTtlIfError: 60 // cache for another 1min when the revalidation requests result in an error.

@JustinTRoss
Copy link
Contributor Author

Got it. To confirm:

requestCache: {
        maxTtl: 10 * 60, // 10min, will respond for 10min with the result in cache (revalidated every 10min)
        maxTtlIfError: 30 * 60, // 30min, will respond with the result in cache for a further 30 minutes when revalidation attempts result in error (40 minutes total)
},

@StarpTech
Copy link
Owner

StarpTech commented Feb 6, 2022

yes, (40 minutes total) is confusing. I'd remove it.

@JustinTRoss
Copy link
Contributor Author

Agreed. It could be better worded. It sounds like the sentiment is generally true though, that the cached value can be returned for up to 40 minutes. This seems necessary for folks to understand. I'll work on conveying it clearly and send a PR with updated docs.

Thanks again! 🙏

@StarpTech
Copy link
Owner

Thanks!

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

No branches or pull requests

2 participants