Skip to content

Conversation

howard-e
Copy link
Contributor

@howard-e howard-e commented May 21, 2025

From #3264 (comment):

A string of failures on this makes me think this solution is flaky in other areas, particularly with GitHub links. So something to investigate separately as well. 6 failures after my last commit for the following links:

expand for list of failing builds
  1. https://github.com/w3c/aria-practices/actions/runs/15139538651/job/42559608538?pr=3264
  2. https://github.com/w3c/aria-practices/actions/runs/15139538651/job/42559779924?pr=3264
  3. https://github.com/w3c/aria-practices/actions/runs/15139538651/job/42560052280?pr=3264
  4. https://github.com/w3c/aria-practices/actions/runs/15139538651/job/42560300319?pr=3264
  5. https://github.com/w3c/aria-practices/actions/runs/15139538651/job/42560518354?pr=3264
  6. https://github.com/w3c/aria-practices/actions/runs/15139538651/job/42560823761?pr=3264

Originally posted by @howard-e in #3264 (comment)

Looking again at the responses, the response codes were 429 and an outlier 508 (which was still most likely related to rate limiting).

About this PR:

Since calls to external links are coming from an unauthenticated source, unable to parse out any reliable retry-related headers from the responses and even if, the links in this project are variable enough that, that may prove difficult to normalise. So defaulting to 15 seconds.

Note: Harder to confirm this on my local setup (but possible). Leaving as a draft while I confirm using this PR.


WAI Preview Link (Last built on Wed, 21 May 2025 22:13:40 GMT).

@howard-e howard-e added the Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation label May 21, 2025
@howard-e
Copy link
Contributor Author

For reviewers: this has now been re-ran 5 times without any unexpected failures to the link-checker build

@howard-e howard-e marked this pull request as ready for review May 21, 2025 14:56
// Found the retry-after unit returned from response headers too
// variable to use here, but ~15 seconds seems like a safe
// default
const retryAfter = baseDelay * 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the formula for the delay in this case need to be different than the formula for the delay in case of an exception? If not, then we could reduce duplication by simply throwing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, de-duped and also removed left-over retry logic in e08b32c

@jugglinmike
Copy link
Contributor

Looking good, @howard-e! I'd love to merge this for you, but GitHub feels differently:

Merging is blocked

You're not authorized to push to this branch. Visit https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches for more information.

@howard-e howard-e merged commit 00d33b1 into main May 22, 2025
7 checks passed
@howard-e howard-e deleted the link-checker-rate-limit branch May 22, 2025 11:49
@howard-e
Copy link
Contributor Author

Looking good, @howard-e! I'd love to merge this for you, but GitHub feels differently:

Merging is blocked
You're not authorized to push to this branch. Visit https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches for more information.

Thanks! (and thinking we should look into your permissions here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants