diff --git a/apps/web/src/app/[slug]/page.jsx b/apps/web/src/app/[slug]/page.jsx index fa8c8e1..10861f2 100644 --- a/apps/web/src/app/[slug]/page.jsx +++ b/apps/web/src/app/[slug]/page.jsx @@ -424,8 +424,8 @@ export default async function FeedPage({ params, base }) { {feed.status === 'dead' && (

- This feed stopped responding, so we no longer crawl it. The archive below is what we - collected while it was live. + This feed stopped responding, so we only check it once a month now and it is back as + soon as it answers. The archive below is what we collected while it was live.

)} diff --git a/apps/web/src/app/api/feeds/[slug]/refresh/route.js b/apps/web/src/app/api/feeds/[slug]/refresh/route.js index 78d266a..b4b18f5 100644 --- a/apps/web/src/app/api/feeds/[slug]/refresh/route.js +++ b/apps/web/src/app/api/feeds/[slug]/refresh/route.js @@ -43,7 +43,7 @@ export async function POST(_req, { params }) { const client = db(); const feed = await q.feedBySlug(client, slug); if (!feed) return json({ error: 'not-found' }, 404); - if (feed.status === 'dead') return json({ ok: false, state: 'dead', error: 'This feed has been retired after repeated failures.' }, 409); + if (feed.status === 'dead') return json({ ok: false, state: 'dead', error: 'This feed has been retired after repeated failures; it is rechecked once a month.' }, 409); const last = Date.parse(String(feed.last_fetched_at ?? '')); if (Number.isFinite(last) && Date.now() - last < RECENT_MS) { diff --git a/apps/web/src/lib/reasons.js b/apps/web/src/lib/reasons.js index c2c6984..0810c19 100644 --- a/apps/web/src/lib/reasons.js +++ b/apps/web/src/lib/reasons.js @@ -20,6 +20,10 @@ export const REASONS = { 'no-feed-found': 'no feed on the site', timeout: 'site did not respond', 'fetch-failed': 'site could not be reached', + 'connection-failed': 'site refused or dropped the connection', + 'tls-failed': 'site has a broken HTTPS certificate', + 'dns-failed': 'site name does not resolve', + 'dns-retry': 'site name could not be looked up just now', 'blocked-host': 'not a public address', // Emitted by the checker but never given words until the log started showing // rejections one by one, where a bare code is the most common thing on screen. diff --git a/packages/db/migrations-pg/20261108165240_dead_feeds_recheck.sql b/packages/db/migrations-pg/20261108165240_dead_feeds_recheck.sql new file mode 100644 index 0000000..7b3f7ba --- /dev/null +++ b/packages/db/migrations-pg/20261108165240_dead_feeds_recheck.sql @@ -0,0 +1,27 @@ +-- 'dead' is no longer for ever (fleet crawler rule, 2026-11-08: flag a slow or +-- broken target and recheck it in a month). A retired feed comes back to the +-- crawler once its next_fetch_at passes, read by recheckFeeds() in queries.js +-- off this partial index: every other due index is `where status <> 'dead'`. +-- +-- Concurrently, as feeds_due_tier_idx was: migrate() runs each statement on +-- its own with no transaction around the file. +create index concurrently if not exists feeds_recheck_idx + on feeds (next_fetch_at) + where status = 'dead'; + +-- Feeds retired before this change carry a next_fetch_at from the old ladder +-- (a day at most after their last attempt), which would make all of them due +-- at once. Re-stamp each to thirty days after its last attempt, which is when +-- the new rule would have scheduled its recheck. Idempotent: rerunning it +-- computes the same value from the same column. recheckFeeds only offers a +-- small share of each tick, so the ones already past thirty days drain over a +-- day or two rather than in one burst. +update feeds + set next_fetch_at = to_char( + (last_fetched_at::timestamptz + interval '30 days') at time zone 'UTC', + 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"') + where status = 'dead' + and last_fetched_at is not null + and next_fetch_at < to_char( + (last_fetched_at::timestamptz + interval '30 days') at time zone 'UTC', + 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"'); diff --git a/packages/db/src/queries.js b/packages/db/src/queries.js index 3dd6123..48a3f83 100644 --- a/packages/db/src/queries.js +++ b/packages/db/src/queries.js @@ -2894,6 +2894,39 @@ export async function expressFeeds(db, limit = 25) { return rows; } +/** + * The share of a tick offered to retired feeds that are due their monthly + * recheck. Small and bounded, so the 31k feeds that were already dead when the + * recheck arrived drain over a day or two instead of landing in one tick; slots + * the live tiers cannot fill are handed to it as well. + */ +const RECHECK_SHARE = 0.05; + +/** + * Retired ('dead') feeds whose recheck is due, oldest first. + * + * 'dead' used to be for ever: every due query filters `status <> 'dead'`, so a + * blog that was down for a fortnight in August was never looked at again. It + * now comes back once its next_fetch_at passes -- thirty days after the attempt + * that retired it -- for exactly one recheck. Served by `feeds_recheck_idx`, the + * partial index on `status = 'dead'`. + * + * @param {Client} db + * @param {number} [limit] + * @returns {Promise} + */ +export async function recheckFeeds(db, limit = 25) { + if (limit <= 0) return []; + const { rows } = await db.execute({ + sql: `select ${DUE_COLUMNS} + from feeds + where status = 'dead' and next_fetch_at <= ? + order by next_fetch_at asc limit ?`, + args: [nowIso(), limit], + }); + return rows; +} + /** * A feed's crawl tier, from its interval alone: 0 hot (read at least hourly), * 1 active (daily), 2 slow, 3 dormant (gone quiet, read weekly), 4 archived @@ -2928,13 +2961,18 @@ export async function dueFeeds(db, limit = 25) { // Bounded rather than unbounded even though the express table is tiny: the // point of a reserved share is that it is reserved in both directions. const express = await expressFeeds(db, Math.floor(limit * EXPRESS_SHARE)); - const room = limit - express.length; - if (room <= 0) return express; + if (limit - express.length <= 0) return express; + + // Retired feeds due their monthly recheck: a small reserved share first, and + // whatever the live tiers leave over at the end. + const rechecks = await recheckFeeds(db, Math.max(1, Math.floor((limit - express.length) * RECHECK_SHARE))); + const room = limit - express.length - rechecks.length; const now = nowIso(); - const out = [...express]; + const out = [...express, ...rechecks]; const exhausted = new Set(); - const seen = new Set(express.map((r) => String(r.id))); + const seen = new Set(out.map((r) => String(r.id))); + if (room <= 0) return out; const take = async (tier, want) => { // The express rows are excluded rather than deduplicated afterwards: they @@ -2969,6 +3007,15 @@ export async function dueFeeds(db, limit = 25) { if (carry <= 0) break; if (!exhausted.has(tier)) carry -= await take(tier, carry); } + if (carry > 0) { + for (const row of await recheckFeeds(db, carry + rechecks.length)) { + if (carry <= 0) break; + if (seen.has(String(row.id))) continue; + seen.add(String(row.id)); + out.push(row); + carry -= 1; + } + } return out; } @@ -3505,18 +3552,25 @@ export async function markUnchanged(db, id, minutes, signals = {}) { /** * Record a failed crawl and push the next attempt out. * + * Ten consecutive failures retire a feed (status 'dead': hidden from listings, + * page kept), but not for ever: `recheckFeeds` hands a dead feed back to the + * crawler once its `next_fetch_at` passes, which the crawler sets thirty days + * out (see failureMinutes in @rssamplifier/ingest). + * * @param {Client} db * @param {string} id * @param {string} error * @param {number} errorCount consecutive failures including this one - * @param {number} minutes backoff + * @param {number} minutes until the next attempt + * @param {{ keepInterval?: boolean }} [opts] keepInterval: leave + * fetch_interval_minutes alone (a month-long flag is not the feed's cadence) */ -export async function markCrawlFailure(db, id, error, errorCount, minutes) { +export async function markCrawlFailure(db, id, error, errorCount, minutes, opts = {}) { const now = nowIso(); await db.execute({ sql: `update feeds set status = ?, last_fetched_at = ?, last_error = ?, error_count = ?, - fetch_interval_minutes = ?, next_fetch_at = ?, updated_at = ? + fetch_interval_minutes = coalesce(?, fetch_interval_minutes), next_fetch_at = ?, updated_at = ? where id = ?`, args: [ // Ten consecutive failures is a dead feed: stop crawling, keep the page. @@ -3524,7 +3578,7 @@ export async function markCrawlFailure(db, id, error, errorCount, minutes) { now, error, errorCount, - minutes, + opts.keepInterval ? null : minutes, nowIso(minutes * 60_000), now, id, diff --git a/packages/db/test/db.test.js b/packages/db/test/db.test.js index b9620fe..a0977f1 100644 --- a/packages/db/test/db.test.js +++ b/packages/db/test/db.test.js @@ -557,17 +557,51 @@ test('crawl failure backs off and eventually marks the feed dead', async () => { row = await q.feedBySlug(db, 'test-blog'); assert.equal(row.status, 'dead', 'ten consecutive failures retires the feed'); - // A dead feed must drop out of the crawl queue and the public listing. - await db.execute({ - sql: 'update feeds set next_fetch_at = ? where id = ?', - args: [nowIso(-60_000), id], - }); - assert.equal((await q.dueFeeds(db, 10)).length, 0, 'dead feeds are not re-crawled'); + // A dead feed drops out of the public listing at once, and out of the crawl + // queue until its recheck comes due. assert.equal( (await q.listFeeds(db)).some((f) => f.slug === 'test-blog'), false, 'dead feeds are hidden from the index', ); + await db.execute({ + sql: 'update feeds set next_fetch_at = ? where id = ?', + args: [nowIso(29 * 86_400_000), id], + }); + assert.equal((await q.dueFeeds(db, 10)).length, 0, 'not before its recheck'); + assert.equal((await q.recheckFeeds(db, 10)).length, 0); + + // But not dead for ever: once next_fetch_at passes it is offered for one recheck. + await db.execute({ + sql: 'update feeds set next_fetch_at = ? where id = ?', + args: [nowIso(-60_000), id], + }); + assert.deepEqual( + (await q.recheckFeeds(db, 10)).map((f) => String(f.id)), + [id], + 'a dead feed comes back for its monthly recheck', + ); + assert.deepEqual( + (await q.dueFeeds(db, 10)).map((f) => String(f.id)), + [id], + 'and the crawler is handed it', + ); + + // A failed recheck parks it another month, still dead, interval untouched. + await q.markCrawlFailure(db, id, 'timeout', 11, 43_200, { keepInterval: true }); + row = await q.feedBySlug(db, 'test-blog'); + assert.equal(row.status, 'dead'); + assert.equal(Number(row.fetch_interval_minutes), 1440, 'a flag is not a cadence'); + const days = (Date.parse(row.next_fetch_at) - Date.now()) / 86_400_000; + assert.ok(days > 29.9 && days <= 30, `next check in 30 days -> ${days.toFixed(2)}`); + assert.equal((await q.dueFeeds(db, 10)).length, 0); + + // A successful recheck restores it. + await q.markUnchanged(db, id, 60); + row = await q.feedBySlug(db, 'test-blog'); + assert.equal(row.status, 'active', 'success clears the flag'); + assert.equal(Number(row.error_count), 0); + assert.equal((await q.listFeeds(db)).some((f) => f.slug === 'test-blog'), true); }); test('submission audit and rate-limit window', async () => { diff --git a/packages/feed/index.js b/packages/feed/index.js index 365f51f..a9d2a11 100644 --- a/packages/feed/index.js +++ b/packages/feed/index.js @@ -29,7 +29,17 @@ export { export { extractKeywords, feedTopics, tokenize, topicSlug, singularize } from './src/keywords.js'; export { sanitizeHtml, textLength } from './src/sanitize.js'; export { normalizeUrl, findFeedLinks, guessFeedUrls, looksLikeFeed } from './src/discover.js'; -export { safeFetch, safeFetchBytes, resolveFeed, isBlockedAddress, isPublicHost, fetchPage } from './src/fetch.js'; +export { + safeFetch, + safeFetchBytes, + resolveFeed, + isBlockedAddress, + isPublicHost, + hostVerdict, + fetchErrorCode, + fetchPage, + CRAWL_FETCH_TIMEOUT_MS, +} from './src/fetch.js'; export { imageSize } from './src/imagesize.js'; export { findFeedCard, diff --git a/packages/feed/src/fetch.js b/packages/feed/src/fetch.js index 39b2b49..731b6b4 100644 --- a/packages/feed/src/fetch.js +++ b/packages/feed/src/fetch.js @@ -16,7 +16,15 @@ import { findPlaylistLinks } from './playlist.js'; const USER_AGENT = 'RSSAmplifierBot/1.0 (+https://rssamp.com/about; feed directory indexer)'; const MAX_BYTES = 5 * 1024 * 1024; // 5 MB — generous for a feed, bounded for us -const TIMEOUT_MS = 15_000; + +/** + * The whole budget for one plain fetch of somebody else's URL: DNS, connect, + * headers and body together (fleet crawler rule, 2026-11-08: "crawlers should + * timeout after 10 seconds for fetch ... we don't want broken slow shit"). + * A server that cannot hand over a feed document in ten seconds is broken for + * our purposes, and the crawler flags it for a month rather than waiting on it. + */ +export const CRAWL_FETCH_TIMEOUT_MS = 10_000; /** * Private, loopback, link-local and carrier-grade-NAT ranges. @@ -65,20 +73,104 @@ export function isBlockedAddress(ip) { * @returns {Promise} true when safe to fetch */ export async function isPublicHost(hostname) { - if (net.isIP(hostname)) return !isBlockedAddress(hostname); + return (await hostVerdict(hostname)) === 'ok'; +} + +/** + * The same check, saying *why* a host was refused. + * + * `blocked` is a private or unparseable address and is our refusal; a name + * that does not resolve is a fact about the publisher (`dns-failed`), and the + * crawler treats the two very differently -- one is a policy, the other a broken + * site to flag and leave alone for a month. A resolver that answers "try again" + * (EAI_AGAIN) is usually *our* resolver having a bad minute, so it is kept apart + * as `dns-retry` and does not condemn the feed. + * + * Bounded by `signal` when given, so the lookup counts against the fetch's + * budget instead of adding the system resolver's own retries on top of it. + * + * @param {string} hostname + * @param {AbortSignal} [signal] + * @returns {Promise<'ok'|'blocked'|'dns-failed'|'dns-retry'|'timeout'>} + */ +export async function hostVerdict(hostname, signal) { + if (net.isIP(hostname)) return isBlockedAddress(hostname) ? 'blocked' : 'ok'; const lower = hostname.toLowerCase(); if (lower === 'localhost' || lower.endsWith('.localhost') || lower.endsWith('.internal')) { - return false; + return 'blocked'; } try { - const addrs = await dns.lookup(hostname, { all: true }); - if (addrs.length === 0) return false; - return addrs.every((a) => !isBlockedAddress(a.address)); - } catch { - return false; + const lookup = dns.lookup(hostname, { all: true }); + const addrs = signal ? await raceAbort(lookup, signal) : await lookup; + if (addrs.length === 0) return 'dns-failed'; + return addrs.every((a) => !isBlockedAddress(a.address)) ? 'ok' : 'blocked'; + } catch (err) { + const code = fetchErrorCode(err); + if (code === 'timeout' || code === 'dns-retry') return code; + return 'dns-failed'; + } +} + +/** + * @template T + * @param {Promise} promise + * @param {AbortSignal} signal + * @returns {Promise} + */ +function raceAbort(promise, signal) { + if (signal.aborted) return Promise.reject(abortError()); + return new Promise((resolve, reject) => { + const onAbort = () => reject(abortError()); + signal.addEventListener('abort', onAbort, { once: true }); + promise.then( + (v) => { + signal.removeEventListener('abort', onAbort); + resolve(v); + }, + (e) => { + signal.removeEventListener('abort', onAbort); + reject(e); + }, + ); + }); +} + +function abortError() { + const err = new Error('aborted'); + err.name = 'AbortError'; + return err; +} + +/** + * A thrown fetch error, reduced to the short code stored in `feeds.last_error`. + * + * Node (undici) hides the reason in `err.cause.code`; Bun puts it on the error + * itself, with its own spellings (`ConnectionRefused`, `FailedToOpenSocket`). + * Everything here except `dns-retry` is what the fleet rule calls a slow or + * broken target: the crawler flags it and does not ask again for thirty days. + * + * @param {unknown} err + * @returns {'timeout'|'dns-failed'|'dns-retry'|'tls-failed'|'connection-failed'|'fetch-failed'} + */ +export function fetchErrorCode(err) { + const e = /** @type {any} */ (err); + if (e?.name === 'AbortError' || e?.name === 'TimeoutError') return 'timeout'; + const code = String(e?.cause?.code ?? e?.code ?? e?.cause?.name ?? ''); + const text = `${code} ${String(e?.cause?.message ?? '')} ${String(e?.message ?? '')}`; + if (/EAI_AGAIN|DNSTryAgain/i.test(text)) return 'dns-retry'; + if (/ENOTFOUND|EAI_NONAME|EAI_NODATA|ENODATA|DNS_?ENOTFOUND|getaddrinfo/i.test(text)) return 'dns-failed'; + if (/TIMEOUT|ETIMEDOUT|Timed ?out/i.test(code)) return 'timeout'; + if (/CERT|TLS|SSL|SELF_SIGNED|UNABLE_TO_(GET|VERIFY)|ALTNAME|HANDSHAKE/i.test(text)) return 'tls-failed'; + if ( + /ECONNREFUSED|ECONNRESET|EHOSTUNREACH|ENETUNREACH|EPIPE|ECONNABORTED|UND_ERR_SOCKET|UND_ERR_CONNECT|ConnectionRefused|ConnectionClosed|FailedToOpenSocket|socket hang up|other side closed/i.test( + text, + ) + ) { + return 'connection-failed'; } + return 'fetch-failed'; } /** @@ -113,20 +205,27 @@ export async function safeFetch(url, conditional = {}) { } const target = new URL(normalized); - if (!(await isPublicHost(target.hostname))) { + + // One deadline for the whole exchange, started before the name is even + // resolved: DNS, connect, headers and body all spend the same ten seconds. + // A ref'd setTimeout rather than AbortSignal.timeout(), whose unref'd timer + // lets Node 22's test runner exit under a stubbed fetch. + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), CRAWL_FETCH_TIMEOUT_MS); + + const verdict = await hostVerdict(target.hostname, controller.signal); + if (verdict !== 'ok') { + clearTimeout(timer); return { ok: false, status: 0, contentType: '', body: '', url: normalized, - error: 'blocked-host', + error: verdict === 'blocked' ? 'blocked-host' : verdict, }; } - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), TIMEOUT_MS); - try { /** @type {Record} */ const headers = { 'user-agent': USER_AGENT, accept: '*/*' }; @@ -153,7 +252,7 @@ export async function safeFetch(url, conditional = {}) { // A redirect can land somewhere internal even when the origin was public. const finalHost = new URL(res.url).hostname; - if (finalHost !== target.hostname && !(await isPublicHost(finalHost))) { + if (finalHost !== target.hostname && (await hostVerdict(finalHost, controller.signal)) !== 'ok') { return { ok: false, status: res.status, @@ -207,7 +306,7 @@ export async function safeFetch(url, conditional = {}) { contentType: '', body: '', url: normalized, - error: err?.name === 'AbortError' ? 'timeout' : 'fetch-failed', + error: fetchErrorCode(err), }; } finally { clearTimeout(timer); @@ -239,20 +338,23 @@ export async function safeFetchBytes(url, maxBytes = 64 * 1024) { } const target = new URL(normalized); - if (!(await isPublicHost(target.hostname))) { + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), CRAWL_FETCH_TIMEOUT_MS); + + const verdict = await hostVerdict(target.hostname, controller.signal); + if (verdict !== 'ok') { + clearTimeout(timer); return { ok: false, status: 0, contentType: '', bytes: empty, url: normalized, - error: 'blocked-host', + error: verdict === 'blocked' ? 'blocked-host' : verdict, }; } - const controller = new AbortController(); - const timer = setTimeout(() => controller.abort(), TIMEOUT_MS); - try { const res = await paidFetch(normalized, { headers: { @@ -265,7 +367,7 @@ export async function safeFetchBytes(url, maxBytes = 64 * 1024) { }); const finalHost = new URL(res.url).hostname; - if (finalHost !== target.hostname && !(await isPublicHost(finalHost))) { + if (finalHost !== target.hostname && (await hostVerdict(finalHost, controller.signal)) !== 'ok') { return { ok: false, status: res.status, @@ -295,7 +397,7 @@ export async function safeFetchBytes(url, maxBytes = 64 * 1024) { contentType: '', bytes: empty, url: normalized, - error: err?.name === 'AbortError' ? 'timeout' : 'fetch-failed', + error: fetchErrorCode(err), }; } finally { clearTimeout(timer); @@ -502,7 +604,7 @@ export async function resolveFeed(input, conditional = {}) { * * The fetch behind the OpenProfile claim check and the OpenWebring member * check. Both read a stranger's front page to see whether it links back at - * us, so neither wants the crawler's fifteen seconds and five megabytes, and + * us, so neither wants the crawler's ten seconds and five megabytes, and * both name themselves so a publisher reading their logs can see what asked * and why. Every failure is a null: to the caller, a site that is down and a * site that answered without the link are the same fact, "no link found". diff --git a/packages/feed/test/crawl-timeout.test.js b/packages/feed/test/crawl-timeout.test.js new file mode 100644 index 0000000..e027915 --- /dev/null +++ b/packages/feed/test/crawl-timeout.test.js @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { CRAWL_FETCH_TIMEOUT_MS, fetchErrorCode, hostVerdict } from '../src/fetch.js'; + +test('a plain crawl fetch gets ten seconds, total', () => { + assert.equal(CRAWL_FETCH_TIMEOUT_MS, 10_000); +}); + +test('thrown fetch errors reduce to codes the crawler can act on', () => { + const abort = Object.assign(new Error('aborted'), { name: 'AbortError' }); + const cause = (code) => Object.assign(new TypeError('fetch failed'), { cause: Object.assign(new Error(code), { code }) }); + + assert.equal(fetchErrorCode(abort), 'timeout'); + assert.equal(fetchErrorCode(Object.assign(new Error('t'), { name: 'TimeoutError' })), 'timeout'); + assert.equal(fetchErrorCode(cause('UND_ERR_CONNECT_TIMEOUT')), 'timeout'); + assert.equal(fetchErrorCode(cause('ENOTFOUND')), 'dns-failed'); + assert.equal(fetchErrorCode(cause('EAI_AGAIN')), 'dns-retry'); + assert.equal(fetchErrorCode(cause('CERT_HAS_EXPIRED')), 'tls-failed'); + assert.equal(fetchErrorCode(cause('DEPTH_ZERO_SELF_SIGNED_CERT')), 'tls-failed'); + assert.equal(fetchErrorCode(cause('ECONNREFUSED')), 'connection-failed'); + assert.equal(fetchErrorCode(cause('ECONNRESET')), 'connection-failed'); + // Bun's own spellings, on the error itself. + assert.equal(fetchErrorCode(Object.assign(new Error('x'), { code: 'ConnectionRefused' })), 'connection-failed'); + assert.equal(fetchErrorCode(Object.assign(new Error('x'), { code: 'FailedToOpenSocket' })), 'connection-failed'); + assert.equal(fetchErrorCode(new Error('something else')), 'fetch-failed'); +}); + +test('the host check refuses private names without asking DNS', async () => { + assert.equal(await hostVerdict('localhost'), 'blocked'); + assert.equal(await hostVerdict('10.0.0.1'), 'blocked'); + assert.equal(await hostVerdict('93.184.216.34'), 'ok'); +}); + +test('the host check spends the fetch budget, not its own', async () => { + const controller = new AbortController(); + controller.abort(); + assert.equal(await hostVerdict('example.com', controller.signal), 'timeout'); +}); diff --git a/packages/ingest/src/crawl.js b/packages/ingest/src/crawl.js index 576797d..11591e4 100644 --- a/packages/ingest/src/crawl.js +++ b/packages/ingest/src/crawl.js @@ -46,6 +46,61 @@ export function backoffMinutes(errorCount) { return Math.min(BACKOFF[Math.min(errorCount - 1, BACKOFF.length - 1)], MAX_INTERVAL); } +/** + * How long a slow or broken target is left alone: thirty days (fleet crawler + * rule, 2026-11-08: "flag to recheck later in a month"). + */ +export const RECHECK_MINUTES = 30 * 24 * 60; + +/** Consecutive failures that retire a feed (status 'dead'; see markCrawlFailure). */ +export const DEAD_AFTER = 10; + +/** + * Failures that say the target is slow or broken rather than that it said no: + * the ten-second fetch budget ran out, the name does not resolve, TLS or the + * connection failed, or the server answered 5xx. Asking again in an hour only + * spends another ten seconds of a worker on the same broken server, so these + * are flagged for a month instead of walking the short ladder. + * + * Not here, on purpose: `dns-retry` (our resolver having a bad minute -- one + * hiccup must not park the whole directory for a month), `blocked-host` (our + * policy), 4xx and `no-feed-found` (the publisher answered; they keep the + * ladder), and anything thrown by a social provider, which is a vendor and is + * turned into a throttle long before it gets here. + * + * @param {string|null|undefined} error the code resolveFeed / scrapeFeed returned + * @returns {boolean} + */ +export function isSlowOrBroken(error) { + const e = String(error ?? ''); + return ( + e === 'timeout' || + e === 'fetch-failed' || + e === 'connection-failed' || + e === 'tls-failed' || + e === 'dns-failed' || + /^http-5\d\d$/.test(e) + ); +} + +/** + * When to try a feed again after a failed crawl, in minutes. + * + * Thirty days for a slow or broken target, and thirty days for a feed this + * failure retires: 'dead' is not forever any more. A retired feed comes due once + * a month for exactly one recheck, a success restores it (`storeCrawl` and + * `markUnchanged` set it active and zero the count), and another failure parks + * it for another month. Everything else walks the short ladder as before. + * + * @param {string|null|undefined} error + * @param {number} errorCount consecutive failures, including this one + * @returns {number} + */ +export function failureMinutes(error, errorCount) { + if (isSlowOrBroken(error) || errorCount >= DEAD_AFTER) return RECHECK_MINUTES; + return backoffMinutes(errorCount); +} + /** How long to wait after a throttle that named no interval of its own. */ const THROTTLE_DEFAULT = 30; @@ -299,8 +354,18 @@ export async function crawlFeed(db, feed, opts = {}) { if (!resolved.ok) { const errorCount = Number(feed.error_count ?? 0) + 1; - await q.markCrawlFailure(db, id, resolved.error, errorCount, backoffMinutes(errorCount)); - return { ok: false, newItems: 0, error: resolved.error }; + // A social source's failures come from a vendor (Arctic Shift, RSSHub) and + // only "no such source" reaches here, so the month-long flag is for feeds + // and scraped pages we fetched ourselves. + const minutes = social ? backoffMinutes(errorCount) : failureMinutes(resolved.error, errorCount); + // A month-long wait is a flag, not a cadence: the feed's own interval is + // kept so that its tier and its rhythm survive to the recheck, and a + // recovered feed is not mistaken for one that publishes monthly. + const flagged = minutes >= RECHECK_MINUTES; + await q.markCrawlFailure(db, id, resolved.error, errorCount, minutes, { + keepInterval: flagged, + }); + return { ok: false, newItems: 0, error: resolved.error, ...(flagged ? { recheckDays: 30 } : {}) }; } // The ordinary path writes the whole crawl in one transaction. Production diff --git a/packages/ingest/test/recheck.test.js b/packages/ingest/test/recheck.test.js new file mode 100644 index 0000000..b449caa --- /dev/null +++ b/packages/ingest/test/recheck.test.js @@ -0,0 +1,102 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +import { + crawlFeed, + failureMinutes, + isSlowOrBroken, + backoffMinutes, + RECHECK_MINUTES, + DEAD_AFTER, +} from '../src/crawl.js'; + +/** + * Fleet crawler rule (2026-11-08): a slow or broken target -- timeout, DNS, TLS, + * connection error, 5xx -- is flagged and left alone for thirty days; nothing + * is dead for ever; 4xx keeps its short ladder. + */ + +function recorder() { + const calls = []; + return { + calls, + execute: async (statement) => { + calls.push(statement); + return { rows: [], rowsAffected: 0 }; + }, + batch: async (statements) => { + calls.push(...statements); + return statements.map(() => ({ rows: [], rowsAffected: 0 })); + }, + }; +} + +const feed = { id: 'feed-1', feed_url: 'https://slow.example/feed', error_count: 0, fetch_interval_minutes: 60 }; + +test('thirty days, in minutes', () => { + assert.equal(RECHECK_MINUTES, 43_200); + assert.equal(DEAD_AFTER, 10); +}); + +test('slow or broken targets are told apart from refusals', () => { + for (const e of ['timeout', 'fetch-failed', 'connection-failed', 'tls-failed', 'dns-failed', 'http-500', 'http-503']) { + assert.equal(isSlowOrBroken(e), true, e); + } + for (const e of ['http-404', 'http-410', 'http-403', 'no-feed-found', 'blocked-host', 'dns-retry', 'invalid-url', null, undefined]) { + assert.equal(isSlowOrBroken(e), false, String(e)); + } +}); + +test('a slow or broken target waits thirty days from the first failure', () => { + assert.equal(failureMinutes('timeout', 1), RECHECK_MINUTES); + assert.equal(failureMinutes('http-502', 1), RECHECK_MINUTES); + assert.equal(failureMinutes('dns-failed', 3), RECHECK_MINUTES); +}); + +test('4xx keeps the short ladder until it retires the feed, then monthly rechecks', () => { + assert.equal(failureMinutes('http-404', 1), backoffMinutes(1)); + assert.equal(failureMinutes('http-410', 5), backoffMinutes(5)); + assert.equal(failureMinutes('http-404', 9), 1440); + assert.equal(failureMinutes('http-404', DEAD_AFTER), RECHECK_MINUTES, 'dead is a month, not for ever'); + assert.equal(failureMinutes('no-feed-found', 25), RECHECK_MINUTES); +}); + +test('a crawl that times out records the error and a next check thirty days out', async () => { + const db = recorder(); + const res = await crawlFeed(db, feed, { resolve: async () => ({ ok: false, error: 'timeout' }) }); + assert.equal(res.ok, false); + assert.equal(res.recheckDays, 30); + + const call = db.calls.find((c) => /last_error/.test(c.sql)); + assert.ok(call, 'markCrawlFailure ran'); + const [status, , error, count, interval, next] = call.args; + assert.equal(status, 'error'); + assert.equal(error, 'timeout'); + assert.equal(count, 1); + assert.equal(interval, null, 'the feed keeps its own interval'); + const days = (Date.parse(next) - Date.now()) / 86_400_000; + assert.ok(days > 29.99 && days <= 30, `30 days -> ${days}`); +}); + +test('a 404 still walks the ladder and stores it as the interval', async () => { + const db = recorder(); + await crawlFeed(db, feed, { resolve: async () => ({ ok: false, error: 'http-404' }) }); + const call = db.calls.find((c) => /last_error/.test(c.sql)); + const [, , , , interval, next] = call.args; + assert.equal(interval, 60); + const minutes = (Date.parse(next) - Date.now()) / 60_000; + assert.ok(minutes > 59 && minutes <= 60); +}); + +test('a 429 is still a throttle, capped at a day, never a flag', async () => { + const db = recorder(); + const res = await crawlFeed(db, feed, { + resolve: async () => ({ ok: false, error: 'http-429', throttled: true, retryAfter: 90 * 86_400 }), + }); + assert.equal(res.throttled, true); + const sql = db.calls.map((c) => c.sql).join('\n'); + assert.ok(!/last_error/.test(sql)); + const next = db.calls[0].args[0]; + const hours = (Date.parse(next) - Date.now()) / 3_600_000; + assert.ok(hours > 23.9 && hours <= 24, `capped at a day -> ${hours}`); +}); diff --git a/packages/ingest/test/recrawl.test.js b/packages/ingest/test/recrawl.test.js index b7c1852..1f29869 100644 --- a/packages/ingest/test/recrawl.test.js +++ b/packages/ingest/test/recrawl.test.js @@ -347,7 +347,7 @@ test('the stored item count tracks what is really there', async () => { test('a failing feed still backs off on its own ladder', async () => { const feed = await seed(); - const res = await crawlFeed(db, feed, { resolve: async () => ({ ok: false, error: 'timeout' }) }); + const res = await crawlFeed(db, feed, { resolve: async () => ({ ok: false, error: 'http-404' }) }); assert.equal(res.ok, false); assert.equal(res.newItems, 0); @@ -357,6 +357,35 @@ test('a failing feed still backs off on its own ladder', async () => { assert.equal(Number(row.fetch_interval_minutes), 60, 'first failure waits an hour'); }); +test('a timed-out feed is flagged for a month, and a success clears the flag', async () => { + const feed = await seed(); + await crawlFeed(db, feed, { resolve }); + const before = await q.feedBySlug(db, 'quiet'); + + const res = await crawlFeed(db, before, { resolve: async () => ({ ok: false, error: 'timeout' }) }); + assert.equal(res.ok, false); + assert.equal(res.recheckDays, 30); + + const row = await q.feedBySlug(db, 'quiet'); + assert.equal(row.status, 'error'); + assert.equal(row.last_error, 'timeout'); + assert.equal(Number(row.error_count), 1); + const days = (Date.parse(row.next_fetch_at) - Date.now()) / 86_400_000; + assert.ok(days > 29.9 && days <= 30, `rechecked in 30 days -> ${days.toFixed(2)}`); + assert.equal( + Number(row.fetch_interval_minutes), + Number(before.fetch_interval_minutes), + 'the cadence survives the flag', + ); + + // The recheck succeeds: back to active, count cleared, on its own rhythm. + await crawlFeed(db, row, { resolve }); + const healed = await q.feedBySlug(db, 'quiet'); + assert.equal(healed.status, 'active'); + assert.equal(Number(healed.error_count), 0); + assert.ok((Date.parse(healed.next_fetch_at) - Date.now()) / 86_400_000 < 8); +}); + /* --------------------------------------------------------- conditional GET */ test('validators are stored on a crawl and sent back on the next one', async () => { diff --git a/packages/ingest/test/submit-queues.test.js b/packages/ingest/test/submit-queues.test.js index 8105b21..fde4490 100644 --- a/packages/ingest/test/submit-queues.test.js +++ b/packages/ingest/test/submit-queues.test.js @@ -127,7 +127,8 @@ test('a single URL is still resolved inline, because it has a blog to land on', // It is proved by the attempt failing rather than by a stubbed response. // `safeFetch` resolves the hostname itself before it fetches anything — the // SSRF guard — and `.example` has no DNS, so the resolve stops at - // `blocked-host` without `fetch` ever being reached. A stub cannot observe + // `dns-failed` without `fetch` ever being reached (`dns-retry` where the + // resolver itself is unreachable, as in a sandbox). A stub cannot observe // this; a rejection can, and a rejection is only possible if the entry went // to the head rather than the tail. const res = await submitCatalogue(db, [{ url: 'https://solo.example/feed.xml' }], { @@ -136,7 +137,7 @@ test('a single URL is still resolved inline, because it has a blog to land on', assert.equal(res.queued, 0, 'one URL is not queued, it is resolved'); assert.equal(res.accepted.length, 0); - assert.deepEqual(res.rejected, [ - { url: 'https://solo.example/feed.xml', error: 'blocked-host' }, - ]); + assert.equal(res.rejected.length, 1); + assert.equal(res.rejected[0].url, 'https://solo.example/feed.xml'); + assert.ok(['dns-failed', 'dns-retry'].includes(res.rejected[0].error), res.rejected[0].error); });