Skip to content

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

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.

Sign up for GitHub

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

Closed
rahul05ranjan wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 21 additions & 44 deletions site/check-links.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
import { maintenanceFromRepo } from "./checks.mjs";
import { parseFrontmatter } from "./lib/frontmatter.mjs";
import { listEntryFiles } from "./lib/entry-files.mjs";
import { classify, repoOf, reportUrl } from "./lib/link-check.mjs";
import { checkMaintenance, classify, parseStaleDays, reportUrl } from "./lib/link-check.mjs";

const __dirname = path.dirname(fileURLToPath(import.meta.url));
const ROOT = path.resolve(__dirname, "..");
Expand Down Expand Up @@ -82,48 +81,22 @@ async function run(entries) {
}

/**
* GitHub sources whose repository state and `maintenance` field disagree.
* One API call per github.com repository, one at a time: a few dozen calls,
* well inside the limit with a token. An API failure is listed, not guessed.
* The GitHub maintenance check now lives in ./lib/link-check.mjs
* (`checkMaintenance`) so its failure classification is tested: it returns
* `{ differs, skipped }`, and API failures are never counted as problems.
*/
async function checkMaintenance(entries) {
const headers = { "user-agent": UA, accept: "application/vnd.github+json" };
if (process.env.GITHUB_TOKEN) headers.authorization = `Bearer ${process.env.GITHUB_TOKEN}`;
const out = [];
for (const e of entries) {
let host;
try {
host = new URL(e.url).hostname;
} catch {
continue;
}
if (host !== "github.com") continue;
const repo = repoOf(e.url);
if (!repo.includes("/")) continue;
try {
const res = await fetch(`https://api.github.com/repos/${repo}`, { headers, signal: AbortSignal.timeout(TIMEOUT_MS) });
if (!res.ok) {
out.push({ ...e, detail: `GitHub API HTTP ${res.status} for ${repo}` });
if (res.status === 403 || res.status === 429) break; // rate limited: the rest would fail too
continue;
}
const data = await res.json();
const want = maintenanceFromRepo(data);
if (want !== e.maintenance) {
const state = data.archived ? "archived" : `last push ${String(data.pushed_at).slice(0, 10)}`;
out.push({ ...e, detail: `${repo} is ${state}; maintenance is ${e.maintenance || "unset"}, expected ${want || "unset"}` });
}
} catch (err) {
out.push({ ...e, detail: `GitHub API ${err.name || "error"} for ${repo}` });
}
}
return out;
}

const staleDays = Number(arg("--stale-days", "365"));
let staleDays;
try {
staleDays = parseStaleDays(arg("--stale-days", "365"));
} catch (err) {
console.error(err.message);
console.error("usage: node site/check-links.mjs [--report <file.md>] [--stale-days 365]");
process.exit(2);
}
const entries = loadEntries().filter((e) => e.status !== "deprecated");
const results = await run(entries);
const maintenance = await checkMaintenance(entries);
const { differs: maintenanceDiffers, skipped: maintenanceSkipped } = await checkMaintenance(entries, { userAgent: UA, timeoutMs: TIMEOUT_MS });
const cutoff = new Date(Date.now() - staleDays * 86400000).toISOString().slice(0, 10);
const rows = entries.map((e, i) => ({ ...e, ...results[i] }));
const pick = (kind) => rows.filter((r) => r.kind === kind);
Expand All @@ -133,7 +106,7 @@ const line = (r) => `- [ ] [\`${r.id}\`](${REPO}/blob/main/${r.rel}): ${reportUr
const dead = pick("dead");
const moved = pick("moved");
const blocked = pick("blocked");
const problems = dead.length + moved.length + stale.length + maintenance.length;
const problems = dead.length + moved.length + stale.length + maintenanceDiffers.length;
const date = new Date().toISOString().slice(0, 10);
const report = [
`Link check of ${entries.length} active and needs-review entries, run ${date}.`,
Expand All @@ -152,9 +125,13 @@ const report = [
"",
...(stale.length ? stale.map((e) => `- [ ] [\`${e.id}\`](${REPO}/blob/main/${e.rel}): verified ${e.verified}`) : ["None."]),
"",
`### GitHub repository state differs from \`maintenance\` (${maintenance.length})`,
`### GitHub repository state differs from \`maintenance\` (${maintenanceDiffers.length})`,
"",
...(maintenanceDiffers.length ? maintenanceDiffers.map(line) : ["None."]),
"",
`### Could not check (GitHub API failures, ${maintenanceSkipped.length})`,
"",
...(maintenance.length ? maintenance.map(line) : ["None."]),
...(maintenanceSkipped.length ? maintenanceSkipped.map(line) : ["None."]),
"",
`<details><summary>Refused the checker (${blocked.length}): usually bot blocking, check by hand</summary>`,
"",
Expand All @@ -168,4 +145,4 @@ const reportPath = arg("--report", null);
if (reportPath) fs.writeFileSync(reportPath, report);
else process.stdout.write(report);
if (process.env.GITHUB_OUTPUT) fs.appendFileSync(process.env.GITHUB_OUTPUT, `problems=${problems}\n`);
console.error(`${entries.length} checked: ${dead.length} dead, ${moved.length} moved, ${stale.length} stale, ${maintenance.length} maintenance, ${blocked.length} blocked`);
console.error(`${entries.length} checked: ${dead.length} dead, ${moved.length} moved, ${stale.length} stale, ${maintenanceDiffers.length} maintenance, ${maintenanceSkipped.length} API-unchecked, ${blocked.length} blocked`);
62 changes: 62 additions & 0 deletions site/lib/lib.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,68 @@ const neText = newEntryText("id: example\ncategory: 3d\nverified: 2026-07-19 #
eq("new-entry text", neText, "id: my-pack\ncategory: 2d\nverified: 2026-10-04\nstatus: needs-review\n");
eq("template copy starts needs-review", newEntryText(nodeFs.readFileSync(new URL("../../catalog/TEMPLATE.md", import.meta.url), "utf8"), { id: "a", category: "2d", today: "2026-10-04" }).split(/\r?\n/).includes("status: needs-review"), true);

/* check-links helpers (#82) ------------------------------------------------ */
import { checkMaintenance, parseStaleDays } from "./link-check.mjs";

eq("stale-days: whole number", parseStaleDays("365"), 365);
eq("stale-days: zero allowed", parseStaleDays("0"), 0);
let staleThrew = "";
try { parseStaleDays("abc"); } catch (e) { staleThrew = e.message; }
has("stale-days: non-number rejected", staleThrew, "whole number");
staleThrew = "";
try { parseStaleDays(undefined); } catch (e) { staleThrew = e.message; }
has("stale-days: missing value rejected", staleThrew, "undefined");
staleThrew = "";
try { parseStaleDays("-5"); } catch (e) { staleThrew = e.message; }
has("stale-days: negative rejected", staleThrew, "whole number");
staleThrew = "";
try { parseStaleDays("3.5"); } catch (e) { staleThrew = e.message; }
has("stale-days: fraction rejected", staleThrew, "whole number");

const ghEntry = (id, maintenance) => ({ id, url: `https://github.com/example/${id}`, maintenance, rel: `catalog/2d/${id}.md` });
const ghRes = (status, data) => ({ ok: status >= 200 && status < 300, status, json: async () => data });

{
const { differs, skipped } = await checkMaintenance([ghEntry("a", null)], { fetchImpl: async () => ghRes(500, {}) });
eq("maintenance: 5xx not a problem", differs.length, 0);
eq("maintenance: 5xx skipped", skipped.length, 1);
has("maintenance: 5xx detail", skipped[0].detail, "HTTP 500");
}
{
const { differs, skipped } = await checkMaintenance([ghEntry("a", null)], {
fetchImpl: async () => ghRes(200, { archived: true, pushed_at: "2026-01-01T00:00:00Z" }),
});
eq("maintenance: mismatch found", differs.length, 1);
eq("maintenance: mismatch only", skipped.length, 0);
has("maintenance: mismatch detail", differs[0].detail, "archived");
}
{
let calls = 0;
const { differs, skipped } = await checkMaintenance([ghEntry("a", null), ghEntry("b", null), ghEntry("c", null)], {
fetchImpl: async () => { calls += 1; return ghRes(403, {}); },
});
eq("maintenance: rate limit stops the loop", calls, 1);
eq("maintenance: rest reported skipped", skipped.length, 3);
has("maintenance: skipped reason", skipped[2].detail, "not checked");
eq("maintenance: rate limit not a problem", differs.length, 0);
}
{
const { differs, skipped } = await checkMaintenance([ghEntry("a", null)], {
fetchImpl: async () => { throw new Error("timeout"); },
});
eq("maintenance: fetch error skipped", skipped.length, 1);
has("maintenance: fetch error detail", skipped[0].detail, "GitHub API");
eq("maintenance: fetch error not a problem", differs.length, 0);
}
{
let calls = 0;
const { differs, skipped } = await checkMaintenance([{ id: "x", url: "https://example.com/a", maintenance: null, rel: "x" }], {
fetchImpl: async () => { calls += 1; return ghRes(200, {}); },
});
eq("maintenance: non-github ignored", calls, 0);
eq("maintenance: non-github no rows", differs.length + skipped.length, 0);
}

/* report (keep last) ------------------------------------------------------ */
if (failures.length) {
console.error(`lib.test failed (${failures.length}):`);
Expand Down
89 changes: 87 additions & 2 deletions site/lib/link-check.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
/**
* The pure parts of site/check-links.mjs: how a response is classified and
* how a URL is written into the report issue. Kept here so they are tested.
* The pure parts of site/check-links.mjs: how a response is classified,
* how a URL is written into the report issue, how the run is configured and
* how the GitHub maintenance check separates findings from outages.
* Kept here so they are tested.
*/

import { maintenanceFromRepo } from "../checks.mjs";

/**
* Two-label public suffixes the catalog's sources sit under. A plain "last two
* labels" rule would read every *.co.uk host as the same site.
Expand Down Expand Up @@ -44,3 +48,84 @@ export function classify(entryUrl, status, finalUrl) {
export function reportUrl(url) {
return `\`${String(url).replaceAll("`", "%60")}\``;
}

/**
* `--stale-days` for site/check-links.mjs: a whole number of days, >= 0.
* 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 (#82).
*/
export function parseStaleDays(raw) {
const n = Number(raw);
if (!Number.isInteger(n) || n < 0) {
throw new Error(`--stale-days wants a whole number of days >= 0, got ${JSON.stringify(String(raw))}`);
}
return n;
}

/**
* GitHub sources whose repository state and `maintenance` field disagree,
* plus the ones that could not be checked at all.
*
* An API failure — a 5xx, a timeout or a rate limit — is a *skipped* row,
* never a catalog problem: guessing from a transient error used to open or
* update the correction issue over an outage. After a 403/429 the remaining
* repositories are marked skipped with the reason instead of silently
* disappearing from the report (#82).
*
* `fetchImpl` is injectable so the failure classification is testable
* without the network.
*/
export async function checkMaintenance(entries, opts = {}) {
const fetchImpl = opts.fetchImpl ?? globalThis.fetch;
const timeoutMs = opts.timeoutMs ?? 20000;
const headers = {
accept: "application/vnd.github+json",
"user-agent": opts.userAgent ?? "Free-Game-Dev-Assets link check",
};
const token = opts.token ?? process.env.GITHUB_TOKEN;
if (token) headers.authorization = `Bearer ${token}`;

const pending = [];
for (const e of entries) {
let host;
try {
host = new URL(e.url).hostname;
} catch {
continue;
}
if (host !== "github.com") continue;
if (!repoOf(e.url).includes("/")) continue;
pending.push(e);
}

const differs = [];
const skipped = [];
for (let i = 0; i < pending.length; i++) {
const e = pending[i];
const repo = repoOf(e.url);
try {
const res = await fetchImpl(`https://api.github.com/repos/${repo}`, { headers, signal: AbortSignal.timeout(timeoutMs) });
if (!res.ok) {
skipped.push({ ...e, detail: `GitHub API HTTP ${res.status} for ${repo}` });
if (res.status === 403 || res.status === 429) {
// Rate limited: the rest would fail the same way — say so instead of dropping them.
for (const rest of pending.slice(i + 1)) {
skipped.push({ ...rest, detail: `not checked: stopped after GitHub API HTTP ${res.status}` });
}
break;
}
continue;
}
const data = await res.json();
const want = maintenanceFromRepo(data);
if (want !== e.maintenance) {
const state = data.archived ? "archived" : `last push ${String(data.pushed_at).slice(0, 10)}`;
differs.push({ ...e, detail: `${repo} is ${state}; maintenance is ${e.maintenance || "unset"}, expected ${want || "unset"}` });
}
} catch (err) {
skipped.push({ ...e, detail: `GitHub API ${err.name || "error"} for ${repo}` });
}
}
return { differs, skipped };
}