diff --git a/package-lock.json b/package-lock.json index 1cf72984..5f6f2762 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9007,7 +9007,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.22.0", + "version": "0.21.0", "license": "Apache-2.0", "dependencies": { "fast-xml-parser": "^5.0.9", diff --git a/packages/plugin/package.json b/packages/plugin/package.json index c7758277..0695bf9d 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.22.0", + "version": "0.21.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0", diff --git a/packages/plugin/src/config.js b/packages/plugin/src/config.js index d458a8a1..a6b1e189 100644 --- a/packages/plugin/src/config.js +++ b/packages/plugin/src/config.js @@ -258,14 +258,41 @@ const defaultConfig = () => ({ defaultInterval: DAY, // What happens when a render proves a URL non-indexable (noindex, canonical - // mismatch, redirect loop). The target is not deleted — it is marked - // `state: suppressed` and rescheduled at `recheckInterval`, so the verdict + // mismatch, redirect loop, HTTP error page). The target is not deleted — it is + // marked `state: suppressed` and rescheduled at `recheckInterval`, so the verdict // re-proves (or heals) itself on cadence, and discovery stops re-creating it. // `maxStrikes` consecutive non-indexable verdicts delete the target outright; // crawler re-discovery restarts the cycle at bounded cost. + // + // Verdicts are not all equally permanent, so the knobs split by HTTP status: + // - 404/410 (`gone`): the origin's strongest statement that the page no longer + // exists. Rechecking it on the default cadence is almost pure waste, so it gets + // fewer, further-apart rechecks before deletion. + // - 401/403 never suppress at all (see RenderQueue): an auth-shaped error is far + // more likely a broken renderer credential or an origin rule change than a page + // verdict, and striking on it would mass-delete healthy targets during an outage. + // - 408/429/5xx never suppress either (see RenderQueue): the origin failed to serve + // the page, it didn't disavow it — the target and its cached page both survive and + // the render just retries at its normal cadence. suppression: { recheckInterval: 7 * DAY, maxStrikes: 4, + gone: { + recheckInterval: 14 * DAY, + maxStrikes: 2, + }, + }, + + // A redirect that proves nothing permanent (302/303/307, a client-side redirect's + // 200, or any redirect onto a route class we don't serve) keeps the source target on + // the theory the page is coming back. A source that answers that way EVERY interval + // is de facto permanent, so each such result counts a strike (the same shared + // counter suppression uses; any successful render clears it) and `maxStrikes` + // consecutive ones retire the source outright. Retiring is safe, not destructive: + // bot traffic for the URL is proxied to the origin — which serves the redirect + // itself — and on-demand discovery re-creates whatever the origin actually serves. + redirects: { + maxStrikes: 4, }, // Periodic repair of targets whose RenderSchedule row is missing. A target and its diff --git a/packages/plugin/src/resources/RenderQueue.js b/packages/plugin/src/resources/RenderQueue.js index c285c640..917594e6 100644 --- a/packages/plugin/src/resources/RenderQueue.js +++ b/packages/plugin/src/resources/RenderQueue.js @@ -239,7 +239,10 @@ export class RenderQueue extends Resource { if (outcome === 'rendered') { const url = CacheKey.extractUrl(cacheKey); - const renderTarget = await Target.get({ id: url, select: ['renderInterval', 'sitemapUrl', 'state'] }); + const renderTarget = await Target.get({ + id: url, + select: ['renderInterval', 'sitemapUrl', 'state', 'strikes'], + }); const renderInterval = renderTarget?.renderInterval; // Schedule the next render relative to when THIS one completed (now), not a @@ -281,6 +284,12 @@ export class RenderQueue extends Resource { if (renderTarget.state === 'suppressed' && result.isIndexable === true) { logger.warn(`Prerendered url ${url} is indexable again — lifting its suppression`); await Target.reactivate(url); + } else if (renderTarget.state !== 'suppressed' && renderTarget.strikes > 0) { + // Strikes are CONSECUTIVE failures by definition: a successful render resets the + // count, so redirect blips months apart never accumulate toward retirement. + // Guarded by strikes > 0 — the hot path (healthy target, no strikes) pays no + // extra write. + await Target.patch(url, { strikes: 0 }); } } else { // No target owns this schedule: it's a one-off (render-now) or an orphaned @@ -292,9 +301,37 @@ export class RenderQueue extends Resource { // `reason` (browser ≥ v1.16.0) says WHY: 'noindex', 'canonical-mismatch', 'http-error', // or 'redirect-loop' — the difference between "the site asked us not to" and "the // render is broken", which read identically without it. The verdict SUPPRESSES the - // target (state + recheck schedule) rather than deleting it — see Target.suppress. - logger.warn(`Suppressing prerendered url: ${cacheKey}${result.reason ? ` (${result.reason})` : ''}`); - await Target.suppress(CacheKey.extractUrl(cacheKey), { reason: result.reason }); + // target (state + recheck schedule) rather than deleting it — see Target.suppress, + // which also grades http-error verdicts by status (404/410 recheck less, die sooner). + // + // EXCEPT 401/403: an auth-shaped error is almost never a statement about the page — + // it's a broken renderer credential, an origin bot-mitigation rule change, or an + // origin auth outage. Striking on it would suppress (and after maxStrikes DELETE) + // swathes of healthy targets exactly when such a failure hits everything at once. + // Keep the target, keep its cached page, reschedule at normal cadence, log loudly. + if (result.statusCode === 401 || result.statusCode === 403) { + logger.error( + `Prerender got ${result.statusCode} for ${cacheKey} — auth-shaped, NOT suppressing. ` + + `If these are widespread, check the renderer's origin-bypass credential and the CDN/origin access rules.` + ); + await this.rescheduleAtTargetCadence(cacheKey); + } else if (result.statusCode === 408 || result.statusCode === 429 || result.statusCode >= 500) { + // Transient-shaped: the origin failed to serve the page, it didn't disavow it. + // Suppressing would delete the last good cached page and count a strike toward + // deletion over what may be one bad minute at the origin — keep both, retry at + // the target's normal cadence, and let a persistent failure surface as noise + // here rather than as silently vanished targets. + logger.warn( + `Prerender got transient ${result.statusCode} for ${cacheKey} — keeping target and cached page, retrying at normal cadence` + ); + await this.rescheduleAtTargetCadence(cacheKey); + } else { + logger.warn(`Suppressing prerendered url: ${cacheKey}${result.reason ? ` (${result.reason})` : ''}`); + await Target.suppress(CacheKey.extractUrl(cacheKey), { + reason: result.reason, + statusCode: result.statusCode, + }); + } } else { // The browser posts `reason` and the failed attempt's error (name/message/phase) since // v1.16.0; without them this branch can only say "unknown". `phase: 'navigation'` @@ -328,17 +365,38 @@ export class RenderQueue extends Resource { server.recordAnalytics(result.renderTime, 'render_time', result.statusCode, 'redirect'); } + // Same status rules as processJobResult, applied BEFORE anything retires or strikes + // the source. Only a rendered-through client-side redirect can carry these statuses + // (a bail-at-nav result posts the first hop's 3xx), so `statusCode` here is the LANDED + // document's: an auth-shaped or transient-shaped landing is a credential/origin + // problem, not a verdict on either URL. Without this, a page whose client-side + // redirect lands on a 401/403 would delete its source target on the FIRST such result + // (via the inspectedNonIndexable branch below) — the exact mass-deletion the + // processJobResult guard exists to prevent. + const authShaped = result.statusCode === 401 || result.statusCode === 403; + const transientShaped = result.statusCode === 408 || result.statusCode === 429 || result.statusCode >= 500; + if (authShaped || transientShaped) { + logger[authShaped ? 'error' : 'warn']( + `Prerendered url ${result.id} redirected to ${result.redirectedTo}, which returned ${result.statusCode} — ` + + `${authShaped ? 'auth-shaped' : 'transient'}, keeping the target and retrying at its normal cadence` + ); + await this.rescheduleAtTargetCadence(result.id); + return; + } + if (landedOn !== PRERENDER) { // The destination is a class we never serve from cache — adopting it would file - // renders where no read looks, and deleting the source would end its rendering for - // good on evidence as weak as an incomplete route list. Keep the source and retry at - // its normal cadence; the retry now costs a navigation, not a full settle. + // renders where no read looks, and deleting the source on ONE such result would end + // its rendering on evidence as weak as an incomplete route list. Keep the source and + // retry at its normal cadence (the retry costs a navigation, not a full settle) — + // but count the strike: a source that answers this way every interval is de facto + // permanently redirected, and recordRedirectStrike retires it after maxStrikes. logger.warn( `Prerendered url ${result.id} redirected (${result.statusCode}) to ${result.redirectedTo}, which is ` + `${landedOn} — keeping the target (no key to schedule the destination under)` ); recordUnroutedPath(landedOn, redirectPath, 'redirect'); - await this.rescheduleRedirectSource(result.id); + await this.recordRedirectStrike(result.id, `to unserved ${landedOn} destination`); return; } @@ -356,8 +414,10 @@ export class RenderQueue extends Resource { await Target.delete(CacheKey.extractUrl(result.id)); const destinationUrl = CacheKey.extractUrl(redirectKey); const domain = URL.parse(destinationUrl)?.hostname; + // Auth-shaped and transient statuses never reach here (guarded above), so this + // suppression is a genuine content/gone verdict about the destination. if (!config.domains.length || config.domains.includes(domain)) { - await Target.suppress(destinationUrl, { reason: result.reason }); + await Target.suppress(destinationUrl, { reason: result.reason, statusCode: result.statusCode }); } return; } @@ -365,12 +425,15 @@ export class RenderQueue extends Resource { if (result.statusCode !== 301 && result.statusCode !== 308) { // No proof of permanence (302/303/307 — failover, geo bounce, outage page — or a // client-side redirect's 200). The source is expected to come back — keep its target - // AND its cached page, and look again next interval. + // AND its cached page, and look again next interval. But a source that answers with + // a temp redirect EVERY interval is a permanent redirect wearing a temporary status: + // each result costs a strike and recordRedirectStrike retires the source after + // maxStrikes rather than paying a navigation every interval forever. logger.warn( `Prerendered url ${result.id} temporarily redirected (${result.statusCode}) to ${result.redirectedTo} — ` + `keeping the target and retrying at its normal cadence` ); - await this.rescheduleRedirectSource(result.id); + await this.recordRedirectStrike(result.id, `temporary ${result.statusCode} to ${result.redirectedTo}`); return; } @@ -409,18 +472,54 @@ export class RenderQueue extends Resource { await Target.put(destinationUrl, target); } + /** + * A redirect result that keeps its source in rotation still costs a strike: one temp + * redirect is failover noise, but `render.redirects.maxStrikes` consecutive ones mean the + * "temporary" status is a lie (or the route list will never serve the destination) and the + * source is retired outright. Retiring is safe, not destructive — bot traffic for the URL + * proxies to the origin, which serves its own redirect, and on-demand discovery re-creates + * whatever the origin actually serves. The strike counter is the target's one shared + * `strikes` field (suppression uses it too); any successful render clears it. + */ + static async recordRedirectStrike(cacheKey, why) { + const sourceUrl = CacheKey.extractUrl(cacheKey); + // One read serves both the strike decision and the reschedule below. + const renderTarget = await Target.get({ id: sourceUrl, select: ['strikes', 'renderInterval', 'sitemapUrl'] }); + if (!renderTarget) { + await RenderSchedule.delete(cacheKey); + return; + } + const strikes = (Number.isFinite(renderTarget.strikes) ? Number(renderTarget.strikes) : 0) + 1; + const maxStrikes = config.render.redirects.maxStrikes; + if (Number.isFinite(maxStrikes) && maxStrikes > 0 && strikes >= maxStrikes) { + logger.warn( + `Prerendered url ${sourceUrl} kept redirecting ${strikes} consecutive times (${why}) — retiring it; ` + + `bots get the origin's own redirect and discovery re-creates what it actually serves` + ); + await Target.delete(sourceUrl); // drops schedules + pages too + return; + } + await Target.patch(sourceUrl, { strikes }); + await this.rescheduleAtTargetCadence(cacheKey, renderTarget); + } + /** * Keep a redirecting source in its rotation. Mirrors the post-render scheduling in * processJobResult: a target-backed key comes due one interval from completion (so cadence * self-paces instead of realigning into a herd); a targetless key (render-now one-off, * orphaned row) has its schedule dropped so the lease doesn't re-claim it forever. + * + * `preloaded` (a row already read with at least renderInterval + sitemapUrl, e.g. by + * recordRedirectStrike) skips the point read. */ - static async rescheduleRedirectSource(cacheKey) { + static async rescheduleAtTargetCadence(cacheKey, preloaded) { const sourceUrl = CacheKey.extractUrl(cacheKey); - const renderTarget = await Target.get({ - id: sourceUrl, - select: ['renderInterval', 'sitemapUrl'], - }); + const renderTarget = + preloaded ?? + (await Target.get({ + id: sourceUrl, + select: ['renderInterval', 'sitemapUrl'], + })); if (!renderTarget) { await RenderSchedule.delete(cacheKey); return; diff --git a/packages/plugin/src/resources/Target.js b/packages/plugin/src/resources/Target.js index 7ae2fb73..69f1e86b 100644 --- a/packages/plugin/src/resources/Target.js +++ b/packages/plugin/src/resources/Target.js @@ -124,21 +124,34 @@ export class Target extends TargetTable { * cadence — the schedule IS the TTL. `maxStrikes` consecutive verdicts delete the target * outright; crawler re-discovery restarts the cycle at bounded cost. * + * `statusCode` (when the verdict came from an HTTP error page) picks the knob set: + * 404/410 use `render.suppression.gone` — fewer, further-apart rechecks, because "gone" + * is the origin's most permanent verdict — and are stored as `http-gone` so the registry + * distinguishes "page vanished" from "page errored". Callers must NOT route 401/403 here + * (see RenderQueue.processJobResult); auth-shaped errors are a renderer/origin problem, + * not a page verdict. + * * Creates the row when absent (a render-now one-off or a redirect destination can be * proven non-indexable before anything targeted it). Deletes the cached pages either * way — stale content of a page that said "don't index me" must not keep serving. */ - static async suppress(url, { reason } = {}) { + static async suppress(url, { reason, statusCode } = {}) { + // Only an http-error verdict classifies by status: a noindex/canonical verdict came + // from a document that rendered, so its status is not the statement being made. + const gone = reason === 'http-error' && (statusCode === 404 || statusCode === 410); + const knobs = gone ? config.render.suppression.gone : config.render.suppression; + const storedReason = gone ? 'http-gone' : (reason ?? null); + const existing = await Target.get({ id: url, select: ['strikes', 'renderInterval', 'sitemapUrl', 'schedulerNode'], }); const strikes = (Number.isFinite(existing?.strikes) ? Number(existing.strikes) : 0) + 1; - const maxStrikes = config.render.suppression.maxStrikes; + const maxStrikes = knobs.maxStrikes; if (existing && Number.isFinite(maxStrikes) && maxStrikes > 0 && strikes >= maxStrikes) { logger.warn( - `Prerender target ${url} non-indexable ${strikes} consecutive times (${reason ?? 'no reason'}) — deleting it` + `Prerender target ${url} non-indexable ${strikes} consecutive times (${storedReason ?? 'no reason'}) — deleting it` ); await Target.delete(url); // drops schedules + pages too return { deleted: true, strikes }; @@ -150,12 +163,12 @@ export class Target extends TargetTable { schedulerNode: existing?.schedulerNode ?? getResidencyByUrl(url), renderInterval: existing?.renderInterval ?? null, state: 'suppressed', - suppressedReason: reason ?? null, + suppressedReason: storedReason, suppressedAt: Date.now(), strikes, }); - const recheckAt = currentMinuteMs() + config.render.suppression.recheckInterval; + const recheckAt = currentMinuteMs() + knobs.recheckInterval; await Promise.all( cacheKeysOf(url).flatMap((cacheKey) => [ RenderSchedule.put(cacheKey, { nextRenderTime: recheckAt, fromSitemap: !!existing?.sitemapUrl }), diff --git a/packages/plugin/test/renderQueueRedirect.test.js b/packages/plugin/test/renderQueueRedirect.test.js index 01a20b63..6d482b2b 100644 --- a/packages/plugin/test/renderQueueRedirect.test.js +++ b/packages/plugin/test/renderQueueRedirect.test.js @@ -472,3 +472,60 @@ test('a legacy non-indexable result (no outcome field) still suppresses', async assert.equal(stores.target.get(A)?.state, 'suppressed'); }); + +// ---- redirect strikes (a source that keeps redirecting is retired) ---- + +test('a temp redirect strikes the source but keeps it (and its cached page) below maxStrikes', async () => { + seedSource(); + await postResult({ id: key(A), url: A, statusCode: 302, outcome: 'redirected', redirectedTo: B }); + + const target = stores.target.get(A); + assert.ok(target, 'source survives a first temp redirect'); + assert.equal(target.strikes, 1, 'but the strike is recorded'); + assert.equal(stores.prerenderedPage.get(key(A)).content, 'old html', 'cached page keeps serving'); + assert.ok(stores.renderSchedule.get(key(A)).nextRenderTime > Date.now(), 'retry scheduled at cadence'); +}); + +test('maxStrikes consecutive temp redirects retire the source outright', async () => { + const max = config.render.redirects.maxStrikes; + seedSource(); + stores.target.get(A).strikes = max - 1; + + await postResult({ id: key(A), url: A, statusCode: 302, outcome: 'redirected', redirectedTo: B }); + + assert.equal(stores.target.has(A), false, 'source retired — the temporary status was a lie'); + for (const device of DEVICES) { + assert.equal(stores.renderSchedule.has(key(A, device)), false, `${device} schedule dropped`); + assert.equal( + stores.prerenderedPage.has(key(A, device)), + false, + `${device} page dropped (origin serves the redirect)` + ); + } + assert.equal(stores.target.has(B), false, 'no adoption from a temp redirect'); +}); + +test('a redirect onto an unserved route class strikes the source too', async () => { + config.ingress.mode = 'forwarded'; // only forwarded mode classifies unmatched paths as unserved + config.ingress.routes = [{ match: 'prefix', path: '/product/', mode: 'prerender' }]; + const C = 'https://site.example.com/checkout/cart'; + seedSource(); + await postResult({ id: key(A), url: A, statusCode: 301, outcome: 'redirected', redirectedTo: C }); + + const target = stores.target.get(A); + assert.ok(target, 'source kept — route list may just be incomplete'); + assert.equal(target.strikes, 1, 'but the strike is recorded'); +}); + +test('a successful render clears redirect strikes — only CONSECUTIVE redirects retire', async () => { + seedSource(); + stores.target.get(A).strikes = 2; + + await postResult( + { id: key(A), url: A, statusCode: 200, outcome: 'rendered', isIndexable: true, headers: {}, renderTime: 10 }, + 'fresh' + ); + + assert.equal(stores.target.get(A).strikes, 0, 'strikes reset on success'); + assert.notEqual(stores.target.get(A).state, 'suppressed'); +}); diff --git a/packages/plugin/test/suppressionStatus.test.js b/packages/plugin/test/suppressionStatus.test.js new file mode 100644 index 00000000..8b64b6c6 --- /dev/null +++ b/packages/plugin/test/suppressionStatus.test.js @@ -0,0 +1,337 @@ +import { test, before, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; + +/** + * Status-aware result handling: not every HTTP error is the same statement about a page. + * + * The properties pinned here: + * - 404/410 (`gone`) suppress under `render.suppression.gone` — stored as `http-gone`, + * rechecked at the longer gone cadence, and deleted after the smaller gone maxStrikes. + * The origin said the page no longer exists; re-proving that 4 times a month is waste. + * - 401/403 NEVER suppress: an auth-shaped error is a renderer-credential or origin-rule + * problem, not a page verdict. The target and its cached pages survive and the job just + * reschedules — otherwise a credential outage would strike-and-delete the registry. + * - 408/429/5xx NEVER suppress: the origin failed to serve the page, it didn't disavow + * it. Target and cached pages survive; retry at normal cadence, no strike. + * - Content verdicts (noindex etc., status 200) still use the default knobs unchanged. + */ + +const DESKTOP = 'desktop'; +const key = (url, device = DESKTOP) => `${url}|${device}`; +const DEVICES = ['desktop', 'mobile']; + +const A = 'https://site.example.com/product/old'; +const B = 'https://site.example.com/product/new'; + +const stores = { + target: new Map(), + renderSchedule: new Map(), + prerenderedPage: new Map(), +}; + +let warns = []; +let errors = []; + +// Minimal stand-in for a Harper table/resource — same load-bearing semantics as the fake in +// renderQueueRedirect.test.js: static get/put/delete dispatch through an instance, put +// REPLACES, patch merges, string select returns the bare scalar. +const makeResourceBase = (rows) => + class FakeResource { + constructor(id) { + this.__id = id; + } + getId() { + return this.__id; + } + async put(data) { + rows.set(this.__id, { ...data }); + } + async delete() { + return rows.delete(this.__id); + } + static async get(query) { + const id = typeof query === 'object' ? query.id : query; + const row = rows.get(id); + if (!row) return null; + const select = typeof query === 'object' ? query.select : undefined; + if (typeof select === 'string') return row[select]; + if (Array.isArray(select)) { + const picked = {}; + for (const name of select) picked[name] = row[name]; + return picked; + } + return { ...row }; + } + static async put(id, data) { + const resource = new this(id); + return resource.put({ ...data }); + } + static async patch(id, data) { + rows.set(id, { ...(rows.get(id) ?? {}), ...data }); + } + static async delete(id) { + const resource = new this(id); + return resource.delete(); + } + static async search() { + return []; + } + }; + +let RenderQueue, config; + +before(async () => { + globalThis.Resource = class {}; + globalThis.server = { + hostname: 'test-node', + nodes: [], + config: { http: { port: 9926 } }, + recordAnalytics: () => {}, + }; + globalThis.logger = { + info() {}, + warn: (msg) => warns.push(String(msg)), + error: (msg) => errors.push(String(msg)), + }; + globalThis.createBlob = (buf) => buf; + globalThis.databases = { + coordination: { + SharedBuffer: { + primaryStore: { + getUserSharedBuffer: (_key, buf) => buf, + tryLock: () => true, + unlock() {}, + }, + }, + }, + render_service: { + Target: makeResourceBase(stores.target), + QueueControl: makeResourceBase(new Map()), + }, + render_schedule: { RenderSchedule: makeResourceBase(stores.renderSchedule) }, + page_cache: { PrerenderedPage: makeResourceBase(stores.prerenderedPage) }, + }; + + ({ config } = await import('../src/config.js')); + ({ RenderQueue } = await import('../src/resources/RenderQueue.js')); +}); + +beforeEach(() => { + for (const rows of Object.values(stores)) rows.clear(); + warns = []; + errors = []; +}); + +const postResult = async (metadata, content) => { + const meta = Buffer.from(JSON.stringify(metadata), 'utf8'); + const body = content ? Buffer.concat([meta, Buffer.from(content)]) : meta; + const ctx = { headers: new Map([['x-metadata-size', String(meta.byteLength)]]) }; + await RenderQueue.processJobResult(body, ctx); +}; + +const seedSource = ({ url = A, renderInterval = 3_600_000, strikes, state } = {}) => { + stores.target.set(url, { + url, + renderInterval, + ...(state ? { state } : {}), + ...(Number.isFinite(strikes) ? { strikes } : {}), + }); + for (const device of DEVICES) { + stores.renderSchedule.set(key(url, device), { nextRenderTime: 1, fromSitemap: false }); + stores.prerenderedPage.set(key(url, device), { statusCode: 200, content: 'old html' }); + } +}; + +const nonIndexable = (statusCode, extra = {}) => ({ + id: key(A), + url: A, + statusCode, + outcome: 'non-indexable', + isIndexable: false, + reason: 'http-error', + ...extra, +}); + +// ---- gone (404/410) ---- + +test('404 suppresses as http-gone at the gone recheck cadence', async () => { + seedSource(); + await postResult(nonIndexable(404)); + + const target = stores.target.get(A); + assert.equal(target.state, 'suppressed'); + assert.equal(target.suppressedReason, 'http-gone', '404 is stored as http-gone, not generic http-error'); + assert.equal(target.strikes, 1); + + const goneRecheck = config.render.suppression.gone.recheckInterval; + const defaultRecheck = config.render.suppression.recheckInterval; + assert.ok(goneRecheck > defaultRecheck, 'precondition: gone rechecks are further apart than the default'); + for (const device of DEVICES) { + const schedule = stores.renderSchedule.get(key(A, device)); + assert.ok(schedule, `${device} recheck schedule must exist`); + assert.ok( + schedule.nextRenderTime > Date.now() + defaultRecheck, + 'recheck must use the gone cadence, not the default' + ); + assert.equal(stores.prerenderedPage.has(key(A, device)), false, 'cached error page must not keep serving'); + } +}); + +test('410 classifies as gone too', async () => { + seedSource(); + await postResult(nonIndexable(410)); + assert.equal(stores.target.get(A).suppressedReason, 'http-gone'); +}); + +test('gone deletes after gone.maxStrikes, sooner than the default maxStrikes', async () => { + const goneMax = config.render.suppression.gone.maxStrikes; + assert.ok(goneMax < config.render.suppression.maxStrikes, 'precondition: gone dies sooner'); + + seedSource({ strikes: goneMax - 1, state: 'suppressed' }); + await postResult(nonIndexable(404)); + + assert.equal(stores.target.has(A), false, 'target must be deleted at gone.maxStrikes'); + for (const device of DEVICES) { + assert.equal(stores.renderSchedule.has(key(A, device)), false, `${device} schedule must be gone`); + } +}); + +test('a 404 verdict does NOT delete at gone.maxStrikes when the strikes came under the default class', async () => { + // Classification is per-verdict: strikes carry over, so a target already at + // gone.maxStrikes strikes from OTHER verdicts is deleted by the next gone verdict. + // This pins the arithmetic rather than any per-class strike ledger. + seedSource({ strikes: 1, state: 'suppressed' }); + await postResult(nonIndexable(404)); + assert.equal(stores.target.has(A), false, 'strikes are one shared counter; 2nd strike as gone deletes'); +}); + +// ---- auth-shaped (401/403) ---- + +test('403 keeps the target, keeps the cached pages, reschedules at normal cadence', async () => { + seedSource({ renderInterval: 3_600_000 }); + await postResult(nonIndexable(403)); + + const target = stores.target.get(A); + assert.ok(target, 'target must survive an auth-shaped result'); + assert.notEqual(target.state, 'suppressed'); + assert.ok(!target.strikes, 'no strike for an auth-shaped result'); + + const schedule = stores.renderSchedule.get(key(A)); + assert.ok(schedule.nextRenderTime > Date.now(), 'rescheduled in the future'); + assert.ok( + schedule.nextRenderTime < Date.now() + 2 * 3_600_000, + 'rescheduled at the target cadence, not a suppression recheck' + ); + assert.equal(stores.prerenderedPage.get(key(A)).content, 'old html', 'last good page keeps serving'); + assert.ok( + errors.some((e) => e.includes('403')), + 'auth-shaped results log at error level' + ); +}); + +test('401 behaves like 403', async () => { + seedSource(); + await postResult(nonIndexable(401)); + assert.notEqual(stores.target.get(A)?.state, 'suppressed'); + assert.equal(stores.prerenderedPage.has(key(A)), true); +}); + +test('403 on a targetless one-off drops the schedule instead of retrying forever', async () => { + stores.renderSchedule.set(key(A), { nextRenderTime: 1 }); + await postResult(nonIndexable(403)); + assert.equal(stores.renderSchedule.has(key(A)), false); +}); + +// ---- transient (408/429/5xx) ---- + +for (const statusCode of [408, 429, 500, 503]) { + test(`${statusCode} keeps the target and cached pages and retries at normal cadence`, async () => { + seedSource(); + await postResult(nonIndexable(statusCode)); + + const target = stores.target.get(A); + assert.ok(target, 'target must survive a transient failure'); + assert.notEqual(target.state, 'suppressed'); + assert.ok(!target.strikes, 'no strike for a transient failure'); + assert.equal(stores.prerenderedPage.get(key(A)).content, 'old html', 'last good page keeps serving'); + + const schedule = stores.renderSchedule.get(key(A)); + assert.ok(schedule.nextRenderTime > Date.now(), 'rescheduled in the future'); + }); +} + +// ---- content verdicts stay on the default knobs ---- + +test('noindex (status 200) still suppresses under the default knobs with its own reason', async () => { + seedSource(); + await postResult(nonIndexable(200, { reason: 'noindex' })); + + const target = stores.target.get(A); + assert.equal(target.state, 'suppressed'); + assert.equal(target.suppressedReason, 'noindex'); + + const goneRecheck = config.render.suppression.gone.recheckInterval; + const schedule = stores.renderSchedule.get(key(A)); + assert.ok(schedule.nextRenderTime < Date.now() + goneRecheck, 'default recheck cadence, not the gone one'); +}); + +test('a noindex verdict that happens to carry a 404 status is still classified gone by status only for http-error', async () => { + // Classification keys on reason === http-error; a content verdict's status is not the + // statement being made. + seedSource(); + await postResult(nonIndexable(404, { reason: 'noindex' })); + assert.equal(stores.target.get(A).suppressedReason, 'noindex'); +}); + +// ---- redirect destination ---- + +test('a client-side redirect landing on a 403 page keeps the source and seeds nothing', async () => { + seedSource(); + await postResult({ + id: key(A), + url: A, + statusCode: 403, + outcome: 'redirected', + redirectedTo: B, + isIndexable: false, + }); + + const source = stores.target.get(A); + assert.ok(source, 'auth-shaped landing must NOT retire the source — a credential outage would mass-delete'); + assert.notEqual(source.state, 'suppressed'); + assert.ok(!source.strikes, 'no strike either'); + assert.equal(stores.prerenderedPage.get(key(A)).content, 'old html', 'last good page keeps serving'); + assert.equal(stores.target.has(B), false, 'auth-shaped destination must not become a suppressed row'); +}); + +test('a client-side redirect landing on a 503 page keeps the source and seeds nothing', async () => { + seedSource(); + await postResult({ + id: key(A), + url: A, + statusCode: 503, + outcome: 'redirected', + redirectedTo: B, + isIndexable: false, + }); + + assert.ok(stores.target.get(A), 'transient landing must not retire the source'); + assert.equal(stores.target.has(B), false, 'transient destination must not become a suppressed row'); +}); + +test('a client-side redirect landing on a 404 page retires the source and seeds an http-gone destination', async () => { + // The genuine-verdict path the guard must NOT swallow: gone destinations still classify. + seedSource(); + await postResult({ + id: key(A), + url: A, + statusCode: 404, + outcome: 'redirected', + redirectedTo: B, + isIndexable: false, + reason: 'http-error', + }); + + assert.equal(stores.target.has(A), false, 'source retired — it leads to a page that is gone'); + assert.equal(stores.target.get(B)?.suppressedReason, 'http-gone', 'destination suppressed under the gone class'); +});