Fix transient server errors reported as dead links

site/check-links.mjs

@@ -3,7 +3,9 @@
  * Fetches every entry's `url` and reports the ones that need a person:
  *   node site/check-links.mjs [--report <file.md>] [--stale-days 365]
  *
- * - dead: 404/410, 5xx, DNS failure or timeout
+ * - dead: 404/410 and other HTTP errors, DNS failure or timeout
+ * - unavailable: 502/503/504, a temporary server failure; listed under
+ *   "Could not check", never counted as a problem
  * - moved: the final URL is on another domain, or another GitHub repository
  * - stale: `verified` is older than --stale-days (365 by default)
  * - blocked: 401/403/429, which is usually a site refusing robots rather
@@ -84,7 +86,7 @@ async function run(entries) {
         let r = await check(entries[i]);
         // One retry for anything that did not answer cleanly, except a full
         // 20 s timeout: a second one would cost a worker 40 s per dead host.
-        if (r.kind === "dead" && !r.timedOut) r = await check(entries[i]);
+        if ((r.kind === "dead" || r.kind === "unavailable") && !r.timedOut) r = await check(entries[i]);
         // A refusal is not retried, and the checker never poses as a browser:
         // a host that refuses scripts goes on the manual list (config.json).
         results[i] = r;
@@ -118,11 +120,13 @@ if (staleArg.error) {
 const staleDays = staleArg.days;
 const entries = loadEntries().filter(isListed);
 const results = await run(entries);
-// Licence pages quoted in Evidence or linked in Notes, each fetched once (#89, #289). Only a page that
-// is gone is reported; a refusal says nothing about the licence.
+// Licence pages quoted in Evidence or linked in Notes, each fetched once (#89, #289).
+// Only a page that is gone counts as a problem; a refusal says nothing about the licence.
+// Temporary server errors are listed under "Could not check".
 const pageJobs = [...new Map(entries.flatMap((e) => e.licencePages.map((url) => [url, { id: e.id, rel: e.rel, url }]))).values()];
 const pageResults = await run(pageJobs);
-const deadPages = pageJobs.map((j, i) => ({ ...j, ...pageResults[i] })).filter((r) => r.kind === "dead");
+const pageRows = pageJobs.map((j, i) => ({ ...j, ...pageResults[i] }));
+const deadPages = pageRows.filter((r) => r.kind === "dead");
 const { mismatches: maintenance, skipped } = await checkMaintenance(entries);
 const cutoff = new Date(Date.now() - staleDays * 86400000).toISOString().slice(0, 10);
 const rows = entries.map((e, i) => ({ ...e, ...results[i] }));
@@ -132,6 +136,8 @@ const line = (r) => `- [ ] [\`${r.id}\`](${REPO}/blob/main/${r.rel}): ${reportUr
 
 const dead = pick("dead");
 const moved = pick("moved");
+const unavailable = [...pick("unavailable"), ...pageRows.filter((r) => r.kind === "unavailable")];
+const unchecked = [...unavailable, ...skipped];
 const refused = pick("blocked");
 const blocked = refused.filter((r) => !manualHostReason(r.url, MANUAL_HOSTS));
 const byHand = refused.filter((r) => manualHostReason(r.url, MANUAL_HOSTS));
@@ -162,10 +168,12 @@ const report = [
   "",
   ...(maintenance.length ? maintenance.map(line) : ["None."]),
   "",
-  // API trouble is the checker's problem, not the catalog's: listed, not counted.
-  `### Could not check (${skipped.length} skipped)`,
+  // Temporary server and API trouble: listed, not counted as catalog problems.
+  `### Could not check (${unchecked.length} skipped)`,
   "",
-  ...(skipped.length ? skipped.map(line) : ["None."]),
+  "Temporary server errors and GitHub API failures are not counted as problems.",
+  "",
+  ...(unchecked.length ? unchecked.map(line) : ["None."]),
   "",
   `<details><summary>Refused the checker (${blocked.length}): usually bot blocking, check by hand</summary>`,
   "",
@@ -185,4 +193,4 @@ const reportPath = flags.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, ${deadPages.length} dead licence pages, ${blocked.length} blocked, ${byHand.length} checked by hand, ${skipped.length} API checks skipped`);
+console.error(`${entries.length} checked: ${dead.length} dead, ${moved.length} moved, ${stale.length} stale, ${maintenance.length} maintenance, ${deadPages.length} dead licence pages, ${unavailable.length} temporarily unavailable, ${blocked.length} blocked, ${byHand.length} checked by hand, ${skipped.length} API checks skipped`);

site/lib/lib.test.mjs

@@ -892,6 +892,105 @@ eq("403 is blocked", classify("https://a.test/", 403, "https://a.test/").kind, "
 eq("404 is dead", classify("https://a.test/", 404, "https://a.test/").kind, "dead");
 eq("report url is an inert code span", reportUrl("https://a.test/@x](y)`z"), "`https://a.test/@x](y)%60z`");
 
+/* transient link-check failures (#275) ------------------------------------ */
+await section("link-check transient classification (#275)", async () => {
+  const url = "https://assets.test/pack";
+  for (const status of [502, 503, 504]) {
+    const result = classify(url, status, url);
+    eq(`HTTP ${status} is unavailable`, result.kind, "unavailable");
+    eq(`HTTP ${status} keeps its status`, result.detail, `HTTP ${status}`);
+    eq(`HTTP ${status} after a redirect is still unavailable`, classify(url, status, "https://other.test/").kind, "unavailable");
+  }
+  for (const status of [400, 404, 405, 410, 422, 500, 501]) {
+    eq(`HTTP ${status} stays dead`, classify(url, status, url).kind, "dead");
+  }
+  for (const status of [401, 403, 429]) {
+    eq(`HTTP ${status} stays blocked`, classify(url, status, url).kind, "blocked");
+  }
+  eq("HTTP 200 stays ok", classify(url, 200, url).kind, "ok");
+});
+
+await section("link-check transient report (#275)", async () => {
+  const { execFileSync } = await import("node:child_process");
+  const { pathToFileURL } = await import("node:url");
+  const root = nodeFs.mkdtempSync(nodePath.join(nodeOs.tmpdir(), "fgda-links-"));
+  try {
+    // Run the real CLI against a tiny catalog, with all network I/O stubbed
+    // in a child process so neither the real catalog nor other tests change.
+    nodeFs.cpSync(new URL("../", import.meta.url), nodePath.join(root, "site"), {
+      recursive: true,
+      filter: (src) => nodeFs.statSync(src).isDirectory()
+        ? !["public", "dist"].includes(nodePath.basename(src))
+        : src.endsWith(".mjs") || nodePath.basename(src) === "config.json",
+    });
+    const catalog = nodePath.join(root, "catalog");
+    nodeFs.mkdirSync(catalog);
+    const put = (id, url, evidence = "", heading = "Evidence") => nodeFs.writeFileSync(nodePath.join(catalog, `${id}.md`),
+      `---\nid: ${id}\nurl: ${url}\nstatus: active\n---\n\n## ${heading}\n\n${evidence}\n`);
+    for (const status of [502, 503, 504]) {
+      put(`source-${status}`, `https://assets.test/source-${status}`, `https://assets.test/licence-${status}`, status === 504 ? "Notes" : "Evidence");
+    }
+    put("recovered", "https://assets.test/recovered-503");
+    const preload = nodePath.join(root, "network.mjs");
+    nodeFs.writeFileSync(preload, `
+      import dns from "node:dns/promises";
+      import { syncBuiltinESMExports } from "node:module";
+      dns.lookup = async () => [{ address: "8.8.8.8", family: 4 }];
+      syncBuiltinESMExports();
+      const calls = new Map();
+      globalThis.fetch = async (url) => {
+        const u = new URL(url);
+        if (u.hostname === "api.github.com") return new Response(null, { status: 503 });
+        if (u.hostname !== "assets.test") throw new Error("unexpected test host");
+        const n = (calls.get(url) || 0) + 1;
+        calls.set(url, n);
+        const status = u.pathname.includes("recovered") && n > 1 ? 200 : Number(u.pathname.match(/(\\d+)$/)[1]);
+        return new Response(null, { status });
+      };
+    `);
+    const output = nodePath.join(root, "github-output");
+    const reportPath = nodePath.join(root, "report.md");
+    const run = () => {
+      nodeFs.writeFileSync(output, "");
+      execFileSync(process.execPath, ["--import", pathToFileURL(preload).href,nodePath.join(root, "site/check-links.mjs"), "--report", reportPath], {
+        env: { ...process.env, GITHUB_OUTPUT: output, GITHUB_TOKEN: "" },
+        stdio: ["ignore", "pipe", "pipe"], timeout: 10000,
+      });
+      return nodeFs.readFileSync(reportPath, "utf8");
+    };
+    const transient = run();
+    has("transient report: no dead sources", transient, "### Dead (0)");
+    has("transient report: no dead licence pages", transient, "### Pages quoted in Evidence or linked in Notes that no longer load (0)");
+    has("transient report: sources and licence pages could not be checked", transient, "### Could not check (6 skipped)");
+    const skipped = transient.split("### Could not check")[1] || "";
+    for (const status of [502, 503, 504]) {
+      has(`transient report: source HTTP ${status}`, skipped, `\`https://assets.test/source-${status}\` (HTTP ${status})`);
+      has(`transient report: licence HTTP ${status}`, skipped, `\`https://assets.test/licence-${status}\` (HTTP ${status})`);
+    }
+    lacks("transient report: a successful retry is not reported", transient, "https://assets.test/recovered-503");
+    eq("transient report: no sources need a look", nodeFs.readFileSync(output, "utf8"), "problems=0\n");
+
+    for (const status of [404, 410]) {
+      put(`dead-${status}`, `https://assets.test/source-${status}`, `https://assets.test/licence-${status}`);
+    }
+    put("api-failure", "https://github.com/o/r");
+    // GitHub's page is healthy, but its API is temporarily unavailable.
+    nodeFs.appendFileSync(preload, `
+      const stubFetch = globalThis.fetch;
+      globalThis.fetch = async (url) => new URL(url).hostname === "github.com"
+        ? new Response(null, { status: 200 }) : stubFetch(url);
+    `);
+    const mixed = run();
+    has("mixed report: 404/410 sources stay dead", mixed, "### Dead (2)");
+    has("mixed report: 404/410 licence pages stay dead", mixed, "### Pages quoted in Evidence or linked in Notes that no longer load (2)");
+    has("mixed report: transient answers and API failures stay visible", mixed, "### Could not check (7 skipped)");
+    has("mixed report: API failure keeps its reason", mixed, "GitHub API HTTP 503 for o/r");
+    eq("mixed report: only dead sources and licence pages need a look", nodeFs.readFileSync(output, "utf8"), "problems=4\n");
+  } finally {
+    nodeFs.rmSync(root, { recursive: true, force: true });
+  }
+});
+
 // #64: one entry walker; hidden and scratch folders are not entries.
 const tmp = nodeFs.mkdtempSync(nodePath.join(nodeOs.tmpdir(), "entries-"));
 for (const rel of ["2d/a.md", "2d/README.md", "2d/TEMPLATE.md", "2d/.hidden/b.md", "_scratch/c.md", "3d/sub/d.md", "3d/e.txt"]) {

site/lib/link-check.mjs

@@ -60,6 +60,7 @@ export const repoOf = (url) => new URL(url).pathname.split("/").slice(1, 3).join
 /** What a finished request says about an entry. */
 export function classify(entryUrl, status, finalUrl) {
   if ([401, 403, 429].includes(status)) return { kind: "blocked", detail: `HTTP ${status}` };
+  if ([502, 503, 504].includes(status)) return { kind: "unavailable", detail: `HTTP ${status}` };
   if (status >= 400) return { kind: "dead", detail: `HTTP ${status}` };
   const from = siteOf(new URL(entryUrl).hostname);
   const to = siteOf(new URL(finalUrl).hostname);