apollo-datasource-http icon indicating copy to clipboard operation
apollo-datasource-http copied to clipboard

`maxTtlIfError` behavior clarification.

Open JustinTRoss opened this issue 3 years ago • 5 comments

This line makes it appear maxTtlIfError is whatever value is provided in seconds for maxTtlIfError https://github.com/StarpTech/apollo-datasource-http/blob/5f0036f6510790e42c5ecf36900367bb6400514f/README.md?plain=1#L113

This line makes it appear as though maxTtlIfError actually ends up being maxTtl + maxTtlIfError. https://github.com/StarpTech/apollo-datasource-http/blob/5f0036f6510790e42c5ecf36900367bb6400514f/src/http-data-source.ts#L328

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

JustinTRoss avatar Jan 18 '22 23:01 JustinTRoss

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.

StarpTech avatar Feb 06 '22 12:02 StarpTech

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)
},

JustinTRoss avatar Feb 06 '22 17:02 JustinTRoss

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

StarpTech avatar Feb 06 '22 18:02 StarpTech

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! 🙏

JustinTRoss avatar Feb 07 '22 09:02 JustinTRoss

Thanks!

StarpTech avatar Feb 07 '22 10:02 StarpTech