Skip to content

fix(links): validate --stale-days up front and never count API failures as problems - #127

Closed
rahul05ranjan wants to merge 1 commit into
TMHSDigital:mainfrom
rahul05ranjan:fix/check-links-robustness
Closed

rahul05ranjan wants to merge 1 commit into
TMHSDigital:mainfrom
rahul05ranjan:fix/check-links-robustness

Conversation

@rahul05ranjan

Copy link
Copy Markdown

Fixes #82

What changed

--stale-days is validated before anything runs. A flag given as the last argument arrives as undefined and a non-number as NaN; either used to slip through and the run died later in new Date(NaN).toISOString() — after every fetch had already run. parseStaleDays (now in site/lib/link-check.mjs, next to the other pure parts of the script) rejects anything that is not a whole number of days ≥ 0, and the script prints the reason plus a usage line and exits 2 immediately:

$ node site/check-links.mjs --stale-days abc
--stale-days wants a whole number of days >= 0, got "abc"
usage: node site/check-links.mjs [--report <file.md>] [--stale-days 365]

API failures are never catalog problems. checkMaintenance moved to site/lib/link-check.mjs (with an injectable fetch so the classification is testable) and returns { differs, skipped }. A 5xx, a timeout or a rate limit produces a skipped row in a new ### Could not check (GitHub API failures, N) report section, excluded from problems, so a transient outage no longer opens or updates the correction issue. After a 403/429 the loop still stops — but the remaining repositories are each marked not checked: stopped after GitHub API HTTP 403 instead of silently disappearing. Non-github.com entries are never sent to the API (same as before). The summary line now also reports the skipped count, e.g. ... 2 maintenance, 3 API-unchecked, 5 blocked.

The report wording for genuine maintenance mismatches, the blocked section, the --report path and the problems= workflow output are unchanged, except that problems no longer includes API failures.

Verification (system Node v26.12.1, no installs)

  • node site/lib/lib.test.mjs: 425 ok (404 before; 21 new assertions) — parseStaleDays (365, 0, abc, undefined, -5, 3.5) and checkMaintenance with a fake fetch: 5xx → skipped not problem; a genuine archived-repo mismatch → differs; 403 stops the loop, 1 call, remaining repos reported with the reason; a thrown fetch error → skipped; non-github entries make zero API calls.
  • node site/checks.test.mjs: 134 ok (unchanged — no regressions).
  • node --check site/check-links.mjs / site/lib/link-check.mjs: OK.
  • Live CLI check: --stale-days abc and a trailing --stale-days both exit 2 with the message and usage before any network activity (no catalog fetches run; the validation precedes loadEntries).
  • Could not run a real link check (it fetches 414 external sites; not appropriate here).

AI-assisted implementation and self-review.

…es as problems

--stale-days given as the last argument or as a non-number became NaN
and the run died in new Date(NaN).toISOString() only after every fetch
had already run. A GitHub API 5xx/timeout/rate limit was written as a
row under 'repository state differs from maintenance' and counted in
problems, so a transient outage opened the correction issue; after a
403/429 the loop broke and the remaining repositories were silently
unchecked. The flag is now validated before anything runs, API
failances land in a 'Could not check (N skipped)' section excluded
from problems, and a rate-limited loop says which repositories were
not checked and why. The helpers moved to lib/link-check.mjs so the
argument parsing and the failure classification are tested.

Fixes TMHSDigital#82

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
@TMHSDigital

Copy link
Copy Markdown
Owner

Thank you for this, and for the tests. #124 merged this morning with the same fix for #82 (opened before this PR could be reviewed), so this branch now conflicts with main and the issue is closed. I compared the two line by line and main covers everything here, so I am closing this as superseded rather than asking for a rebase. Contributions are welcome on any of the remaining open issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

check-links: crashes on a bad --stale-days, and GitHub API failures are reported as catalog problems

2 participants