feat(rule): add maxRetryTime option
#136
Conversation
| // exponential retry | ||
| // 0ms -> 100ms -> 200ms -> 400ms -> 800ms ... | ||
| await waitTimeMs((currentRetryCount ** 2) * 100); | ||
| const retrySeconds = res.headers.get('Retry-After'); |
There was a problem hiding this comment.
I think that we need to prefer Retry-After if the response header includes Retry-After.
Edit: This concern come from default value of maxRetryTime https://github.com/textlint-rule/textlint-rule-no-dead-link/pull/136/files#r739803949
So, we not need to change the logics if we change the default value.
probably, next code. It may be subtle.
const retrySeconds = res.headers.get('Retry-After');
// If the response has `Retry-After` header, prefer it
// else exponential retry: 0ms -> 100ms -> 200ms -> 400ms -> 800ms ...
const retryWaitTimeMs = retrySeconds !== null ? retrySeconds * 1000 : currentRetryCount ** 2 * 100;
const maxRetryTimeMs = ruleOptions.maxRetryTime * 1000;
if (retryWaitTime <= maxRetryTimeMs) {
await waitTimeMs(currentRetryCount ** 2 * 100);
}
return isAliveURI(uri, 'GET', maxRetryCount, currentRetryCount + 1);There was a problem hiding this comment.
@azu
Thank you for your comment.
I rewrite the code as you said.
src/no-dead-link.js
Outdated
| @@ -20,7 +20,8 @@ const DEFAULT_OPTIONS = { | |||
| interval: 500, // The length of time in milliseconds before the interval count resets. Must be finite. [Experimental] | |||
| intervalCap: 8, // The max number of runs in the given interval of time. [Experimental] | |||
| keepAlive: false, // {boolean} if it is true, use keepAlive for checking request [Experimental] | |||
| userAgent: 'textlint-rule-no-dead-link/1.0' // {String} a UserAgent | |||
| userAgent: 'textlint-rule-no-dead-link/1.0', // {String} a UserAgent, | |||
| maxRetryTime: 0, // (number) The max of waiting seconds for retry, if response returns `After-Retry` header. | |||
There was a problem hiding this comment.
This default value means that we will ignore Retry-After by default.
I think that we should consider Retry-After header if the server response it.
Increase maxRetryTime to 10 or 30?
There was a problem hiding this comment.
I've set it to 10.
I've set 0 to not impact those who are already using it.
But I understand that After-Retry value should be used.
chick-p
changed the title
maxRetryTime optionmaxRetryTime option
|
Thanks! |
Fix #135
I've not added this test case.
Because I expect this test case occurs timeout.
Ref. textlint/textlint#781