textlint-rule / textlint-rule-no-dead-link

textlint rule to check if all links are alive.
30 stars 11 forks source link

Some servers return 50x error for HEAD request #96

Closed 0x6b closed 6 years ago

0x6b commented 6 years ago

What did you do? Please include the actual source code causing the issue.

Check [results for textlint](https://www.npmjs.com/search?q=textlint) with the rule.

What did you expect to happen?

Check done without failure as I know the link is working.

What actually happened? Please include the actual, raw output from textlint.

1:24  error  https://www.npmjs.com/search?q=textlint is dead. (503 first byte timeout)  no-dead-link

npmjs.com seems to return 503 for HEAD request. fetch with GET method works as expected. Since no exception occurred, fallback (https://github.com/textlint-rule/textlint-rule-no-dead-link/pull/86) does not work.

In the meantime, I can ignore the error with ignore option, but additional configuration such as specify method for each hostname could be valuable.

Created a repo with reproduce steps at https://github.com/0x6b/textlint-rule-no-dead-link-npmjs-issue for your convenience. Thank you.

nodaguti commented 6 years ago

Thank you for the bug report and the feature suggestion.

It might be better to have an option like preferGET, which is an array of hostnames (or possibly partial URI strings) and lets the rule connect to each host in the list using GET, not HEAD.

Unfortunately however, most of my time is currently being taken for writing a master thesis and I don't have enough time to write code including tests, publish the package, etc.

@azu Would it be possible to spare some time to work on this?

0x6b commented 6 years ago

@nodaguti, thank you for your comment while you are busy. Hope your thesis finished well 🙏 @azu, could you kindly review my preliminary implementation for preferGET at #102?

nodaguti commented 6 years ago

@0x6b Thank you for your understanding and kind concern 🙇

azu commented 6 years ago

@0x6b @nodaguti OK. I'll see it.

azu commented 6 years ago

npmjs.com seems to return 503 for HEAD request. fetch with GET method works as expected. Since no exception occurred, fallback (#86) does not work.

Should this fallback work too when the result of HEAD request is 5xx? (If response.ok is false, try to GET request)

In other context, preferGET seem to save timeout error.

0x6b commented 6 years ago

@azu, thank you very much for your review. Current fallback only works when fetch throws an exception. As you pointed out, additional fallback will solve the issue, and it seems to be more robust.

I'm thinking adding following before returning. Appreciate your comment.

    if (!res.ok && method === 'HEAD') {
      return isAliveURI(uri, 'GET')
    }
0x6b commented 6 years ago

Added above at https://github.com/textlint-rule/textlint-rule-no-dead-link/pull/102/files#diff-c8313408a0f3e1bd753169ec651e12edR89. Await review.