Skip to content

Treat 502/503/504 as a transient answer, not a dead link #299

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
bfs2021 wants to merge 1 commit into from
Closed

Treat 502/503/504 as a transient answer, not a dead link #299

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
13 changes: 9 additions & 4 deletions site/check-links.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, other 4xx/5xx answers, DNS failure or timeout
* - unavailable: 502/503/504, a gateway or host that was down at that moment;
* listed under "Could not check", never counted as a problem (#275)
* - 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
Expand Down Expand Up @@ -132,6 +134,7 @@ const line = (r) => `- [ ] [\`${r.id}\`](${REPO}/blob/main/${r.rel}): ${reportUr

const dead = pick("dead");
const moved = pick("moved");
const unavailable = pick("unavailable");
const refused = pick("blocked");
const blocked = refused.filter((r) => !manualHostReason(r.url, MANUAL_HOSTS));
const byHand = refused.filter((r) => manualHostReason(r.url, MANUAL_HOSTS));
Expand Down Expand Up @@ -163,9 +166,11 @@ 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)`,
`### Could not check (${unavailable.length} transient answers, ${skipped.length} API checks skipped)`,
"",
...(skipped.length ? skipped.map(line) : ["None."]),
...(unavailable.length ? unavailable.map(line) : []),
...(skipped.length ? skipped.map(line) : []),
...(unavailable.length || skipped.length ? [] : ["None."]),
"",
`<details><summary>Refused the checker (${blocked.length}): usually bot blocking, check by hand</summary>`,
"",
Expand All @@ -185,4 +190,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, ${unavailable.length} unavailable, ${stale.length} stale, ${maintenance.length} maintenance, ${deadPages.length} dead licence pages, ${blocked.length} blocked, ${byHand.length} checked by hand, ${skipped.length} API checks skipped`);
5 changes: 5 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 @@ -890,6 +890,11 @@ eq("same .co.uk site is ok", classify("https://a.co.uk/x", 200, "https://www.a.c
eq("github repo transfer is moved", classify("https://github.com/a/b", 200, "https://github.com/c/b").kind, "moved");
eq("403 is blocked", classify("https://a.test/", 403, "https://a.test/").kind, "blocked");
eq("404 is dead", classify("https://a.test/", 404, "https://a.test/").kind, "dead");
eq("410 is dead", classify("https://a.test/", 410, "https://a.test/").kind, "dead");
for (const status of [502, 503, 504]) {
eq(`${status} is unavailable, not dead`, classify("https://a.test/", status, "https://a.test/").kind, "unavailable");
}
eq("a 502 detail names the status", classify("https://a.test/", 502, "https://a.test/").detail, "HTTP 502");
eq("report url is an inert code span", reportUrl("https://a.test/@x](y)`z"), "`https://a.test/@x](y)%60z`");

// #64: one entry walker; hidden and scratch folders are not entries.
Expand Down
8 changes: 8 additions & 0 deletions site/lib/link-check.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,17 @@ export function evidenceUrls(evidence) {
/** owner/repo of a github.com URL, lowercased. A transferred repository redirects within github.com. */
export const repoOf = (url) => new URL(url).pathname.split("/").slice(1, 3).join("/").toLowerCase();

/**
* Gateway answers that say the host was unavailable at that moment, not that
* the page is gone. A weekly run that hits an outage must not list every entry
* on that host as dead (#275).
*/
const TRANSIENT_STATUSES = new Set([502, 503, 504]);

/** 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 (TRANSIENT_STATUSES.has(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);
Expand Down
Loading