-
Notifications
You must be signed in to change notification settings - Fork 120
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
429 HTTP code retry is flacky #114
Comments
This is not an issue at the markdown-link-check level but at the link-check library level. |
This may interest you to know that link-check dependency fix is released : https://github.com/tcort/link-check/blob/master/CHANGELOG.md#version-452 See also #123 for the pending update of markdown-link-check. Could make some tests to confirm that it works now? (We have unit tests and I tested on my CI that was always failing and now it's fine, but still I'd like to make sure) You just have to update your package.json file to point to the master branch on this repos until we create a new release. Thanks! nodejs/community-committee#640 |
fixes related to handling of 429 HTTP errors fixes #114
… links When a pr is opened the linter will run over all md files and check if a link is not valid. # Notes: - Some md files were removed because I couldn't file a redirection to them: - deleted: coverpage.md - deleted: developer-guides/android-developer-guide.md - deleted: developer-guides/samples.md - deleted: ../start/coverpage.md - The removed link were removed from the doc, it doesn't makes sense have an invalid link. For example: - https://www.ekito.fr/people/sparkjava-and-koin/ - https://medium.com/mindorks/using-dependency-injection-with-koin-bee0b461714a - Config Files comments: - CHANGELOG.md is excluded from the linter. It contains a lot of github links, so when the linter checks it, GitHub returns a max requests reached error (429 HTTP error code). The [linter library is adding a feature to fix this case. However, they have to release a new version with this feature](tcort/markdown-link-check#114). - https://developer.android.com/ links are removed from the check because it requires authentication, so they fail due to an redirect loop.
The issue
#106 introduced a retry on HTTP 429 code meaning "retry later" as many links on Github experienced this issues.
But even if it mostly works, sometimes it's obviously not enough.
Example screenshot of a CI pipeline that needed to be retried twice before being all green on link checking. Without changing any thing between the runs of course.
The matching commit is Consensys/doc.goquorum@f3eb2ca
The possible causes
It may be because of too many links tested and Github increases the expected retry delay value then when retry is done on our side, it may be too soon as other requests made the value increase.
There's some research to do here.
Options
One idea it raises is that, as other open issues (#40 #111), having a broader computation that just links taken one by one could make this improve. Having a context where we know if we have other links waiting for retry and what the latest duration value is for a specific domain may help.
But first we have to clarify what the issue is exactly. It's not easy to test as it's not something we can reproduce locally when targeting Github links (429 only happens when ran from CI) but we can try and perhaps ask Github directly.
The text was updated successfully, but these errors were encountered: