feat(plugin): two-lane retry for never-suppress failures — fast lease retries for blips, cadence backoff for persistent ones; v0.24.0

packages/plugin/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@harperfast/prerender",
-	"version": "0.23.0",
+	"version": "0.24.0",
 	"type": "module",
 	"description": "Configurable Harper plugin for prerendering pages for bots and crawlers",
 	"license": "Apache-2.0",

packages/plugin/src/config.js

@@ -273,7 +273,7 @@ const defaultConfig = () => ({
 		//     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.
+		//     the render retries under `failureRetry` below.
 		suppression: {
 			recheckInterval: 7 * DAY,
 			maxStrikes: 4,
@@ -283,6 +283,22 @@ const defaultConfig = () => ({
 			},
 		},
 
+		// Retry shape for the HTTP failures that never suppress (401/403 auth-shaped,
+		// 408/429/5xx transient). The first `fastRetries` consecutive failures leave the
+		// job's claim lease in place, so the retry comes on lease expiry
+		// (`queue.jobLeaseTime`, minutes) — an origin blip recovers fast, and the cached
+		// page's stale-while-revalidate window covers bots throughout. From the next strike
+		// on, the retry drops to the target's normal cadence: a persistently failing page
+		// must not hot-loop 100+ renders a day. Past `page.swrTtl` the kept page stops
+		// serving and bots fall through to the origin on purpose — its answer (a live page
+		// for auth-shaped failures, an honest 5xx for transient ones) is the truth, and
+		// serving arbitrarily old snapshots while users get errors would break bot/user
+		// parity. Strikes are the target's one shared counter; any successful render
+		// clears it.
+		failureRetry: {
+			fastRetries: 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

packages/plugin/src/resources/RenderQueue.js

@@ -6,7 +6,7 @@ import { CacheKey } from '../util/cacheKey.js';
 import { canonicalizeUrl } from '../util/url.js';
 import { classifyPath, queryAllowlistFor, resolveRenderInterval, PRERENDER } from '../util/routeClass.js';
 import { recordUnroutedPath } from '../util/unrouted.js';
-import { Target } from './Target.js';
+import { Target, countedStrikes } from './Target.js';
 import { getDesiredPause, setDesiredPause } from '../util/queueControl.js';
 
 const protocol = server.hostname === 'localhost' ? 'http' : 'https';
@@ -306,25 +306,22 @@ export class RenderQueue extends Resource {
 			//
 			// 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.
+			// origin auth outage. Striking toward deletion 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, retry via retryAfterFailure.
 			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);
+				await this.retryAfterFailure(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);
+				// Suppressing would delete the last good cached page and park the URL for the
+				// recheck interval over what may be one bad minute at the origin — keep both
+				// and retry via retryAfterFailure (fast first, then the target's cadence).
+				logger.warn(`Prerender got transient ${result.statusCode} for ${cacheKey} — keeping target and cached page`);
+				await this.retryAfterFailure(cacheKey);
 			} else {
 				logger.warn(`Suppressing prerendered url: ${cacheKey}${result.reason ? ` (${result.reason})` : ''}`);
 				await Target.suppress(CacheKey.extractUrl(cacheKey), {
@@ -378,9 +375,9 @@ export class RenderQueue extends Resource {
 		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`
+					`${authShaped ? 'auth-shaped' : 'transient'}, keeping the target`
 			);
-			await this.rescheduleAtTargetCadence(result.id);
+			await this.retryAfterFailure(result.id);
 			return;
 		}
 
@@ -489,7 +486,7 @@ export class RenderQueue extends Resource {
 			await RenderSchedule.delete(cacheKey);
 			return;
 		}
-		const strikes = (Number.isFinite(renderTarget.strikes) ? Number(renderTarget.strikes) : 0) + 1;
+		const strikes = countedStrikes(renderTarget.strikes) + 1;
 		const maxStrikes = config.render.redirects.maxStrikes;
 		if (Number.isFinite(maxStrikes) && maxStrikes > 0 && strikes >= maxStrikes) {
 			logger.warn(
@@ -503,6 +500,48 @@ export class RenderQueue extends Resource {
 		await this.rescheduleAtTargetCadence(cacheKey, renderTarget);
 	}
 
+	/**
+	 * Retry shape for auth-shaped (401/403) and transient (408/429/5xx) failures — the ones
+	 * that never suppress. Two lanes, split by the target's strike count
+	 * (`render.failureRetry.fastRetries`):
+	 *
+	 *   FAST — the schedule is left holding its claim lease, so the retry comes on lease
+	 *   expiry (`queue.jobLeaseTime`, minutes). An origin blip recovers fast, and the cached
+	 *   page's swrTtl window keeps serving bots across a lease-sized wait.
+	 *
+	 *   SLOW — after `fastRetries` consecutive failures this is not a blip: drop to the
+	 *   target's normal cadence so a persistently failing page can't hot-loop renders all
+	 *   day. The kept page's expiry is deliberately NOT extended: `swrTtl` is the product
+	 *   bound on how stale we serve as if fresh, and past it bots fall through to the
+	 *   origin — whose answer (a live page for auth-shaped failures, an honest 5xx for
+	 *   transient ones) is the truth. Serving arbitrarily old snapshots while users get
+	 *   errors would break bot/user parity.
+	 *
+	 * Strikes are the target's one shared counter (suppression and redirect strikes use it
+	 * too); any successful render clears it. A targetless key (render-now one-off) has its
+	 * schedule dropped, as everywhere else.
+	 */
+	static async retryAfterFailure(cacheKey) {
+		const sourceUrl = CacheKey.extractUrl(cacheKey);
+		const renderTarget = await Target.get({ id: sourceUrl, select: ['strikes', 'renderInterval', 'sitemapUrl'] });
+		if (!renderTarget) {
+			await RenderSchedule.delete(cacheKey);
+			return;
+		}
+		const strikes = countedStrikes(renderTarget.strikes) + 1;
+		await Target.patch(sourceUrl, { strikes });
+
+		if (strikes <= config.render.failureRetry.fastRetries) {
+			logger.warn(`Retrying ${cacheKey} on its claim lease (failure strike ${strikes})`);
+			return; // schedule untouched — the lease written at claim drives the retry
+		}
+
+		const interval = resolveRenderInterval(sourceUrl, renderTarget.renderInterval);
+		const nextRenderTime = currentMinuteMs() + interval;
+		logger.warn(`Retrying ${cacheKey} at its normal cadence (failure strike ${strikes})`);
+		await RenderSchedule.put(cacheKey, { nextRenderTime, fromSitemap: !!renderTarget.sitemapUrl });
+	}
+
 	/**
 	 * 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

packages/plugin/src/resources/Target.js

@@ -15,6 +15,17 @@ const {
 // jittered schedules — which is exactly what writing the suppression row must not do.
 const TargetTable = databases.render_service.Target;
 
+/**
+ * A target's stored strike count, read defensively: coerce BEFORE the finite check, because
+ * Harper numeric columns can surface as BigInt (which `Number.isFinite` rejects outright —
+ * the same trap `resolveRenderInterval` guards for renderInterval), and `Number(null)` is 0,
+ * so an absent count correctly reads as zero rather than NaN.
+ */
+export const countedStrikes = (value) => {
+	const count = Number(value);
+	return Number.isFinite(count) && count > 0 ? count : 0;
+};
+
 /**
  * The URL registry — ONE row per URL (see schema.graphql). Device variants are not stored:
  * the devices a URL renders for are `config.deviceTypes.default` at write time, and `put`
@@ -146,7 +157,7 @@ export class Target extends TargetTable {
 			id: url,
 			select: ['strikes', 'renderInterval', 'sitemapUrl', 'schedulerNode'],
 		});
-		const strikes = (Number.isFinite(existing?.strikes) ? Number(existing.strikes) : 0) + 1;
+		const strikes = countedStrikes(existing?.strikes) + 1;
 
 		const maxStrikes = knobs.maxStrikes;
 		if (existing && Number.isFinite(maxStrikes) && maxStrikes > 0 && strikes >= maxStrikes) {

packages/plugin/test/suppressionStatus.test.js

@@ -207,20 +207,19 @@ test('a 404 verdict does NOT delete at gone.maxStrikes when the strikes came und
 
 // ---- auth-shaped (401/403) ----
 
-test('403 keeps the target, keeps the cached pages, reschedules at normal cadence', async () => {
+test('403 keeps the target and cached pages; first failure retries on the claim lease', 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');
+	assert.equal(target.strikes, 1, 'the failure is counted');
 
-	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.renderSchedule.get(key(A)).nextRenderTime,
+		1,
+		'schedule untouched — the claim lease drives the fast retry'
 	);
 	assert.equal(stores.prerenderedPage.get(key(A)).content, 'old html', 'last good page keeps serving');
 	assert.ok(
@@ -245,21 +244,45 @@ test('403 on a targetless one-off drops the schedule instead of retrying forever
 // ---- 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 () => {
+	test(`${statusCode} keeps the target and cached pages; first failure retries on the claim lease`, 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(target.strikes, 1, 'the failure is counted');
 		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');
+		assert.equal(stores.renderSchedule.get(key(A)).nextRenderTime, 1, 'schedule untouched — lease-driven retry');
 	});
 }
 
+test('past fastRetries, a failure drops to the target cadence — page kept but its expiry untouched', async () => {
+	const fast = config.render.failureRetry.fastRetries;
+	seedSource({ renderInterval: 3_600_000 });
+	stores.target.get(A).strikes = fast; // this failure is strike fast+1 — first slow-lane one
+	stores.prerenderedPage.get(key(A)).expiresAt = 777; // sentinel: must not be rewritten
+
+	await postResult(nonIndexable(503));
+
+	const target = stores.target.get(A);
+	assert.equal(target.strikes, fast + 1);
+	assert.notEqual(target.state, 'suppressed');
+
+	const schedule = stores.renderSchedule.get(key(A));
+	assert.ok(schedule.nextRenderTime > Date.now(), 'slow lane: rescheduled at the target cadence');
+	assert.ok(schedule.nextRenderTime < Date.now() + 2 * 3_600_000, 'cadence, not a suppression recheck');
+
+	const page = stores.prerenderedPage.get(key(A));
+	assert.equal(page.content, 'old html', 'page survives');
+	assert.equal(
+		page.expiresAt,
+		777,
+		'expiry deliberately NOT extended — swrTtl bounds staleness; past it, origin is the truth'
+	);
+});
+
 // ---- content verdicts stay on the default knobs ----
 
 test('noindex (status 200) still suppresses under the default knobs with its own reason', async () => {
@@ -299,7 +322,7 @@ test('a client-side redirect landing on a 403 page keeps the source and seeds no
 	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(source.strikes, 1, 'the failure is counted toward the retry lanes');
 	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');
 });
@@ -335,3 +358,14 @@ test('a client-side redirect landing on a 404 page retires the source and seeds
 	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');
 });
+
+test('a BigInt strikes value (Harper numeric surfacing) still counts toward the slow lane', async () => {
+	const fast = config.render.failureRetry.fastRetries;
+	seedSource({ renderInterval: 3_600_000 });
+	stores.target.get(A).strikes = BigInt(fast); // Number.isFinite(BigInt) is false — must coerce first
+
+	await postResult(nonIndexable(503));
+
+	assert.equal(stores.target.get(A).strikes, fast + 1, 'BigInt count read correctly, not reset to 1');
+	assert.ok(stores.renderSchedule.get(key(A)).nextRenderTime > Date.now(), 'transitioned to the slow lane');
+});