diff --git a/scripts/check-edge-cache.ts b/scripts/check-edge-cache.ts index 244e47e8..239a7eae 100644 --- a/scripts/check-edge-cache.ts +++ b/scripts/check-edge-cache.ts @@ -16,6 +16,9 @@ async function main(): Promise { baseUrl: process.env.EDGE_CACHE_BASE_URL ?? 'https://skilld.dev', }) console.log(JSON.stringify(result, null, 2)) + // A GitHub annotation, so the deploy summary shows it without failing the job. + for (const warning of result.warnings) + console.log(`::warning::Cloudflare removed the ETag from the HTML of ${warning.path}. A zone feature that rewrites HTML, such as Email Obfuscation, strips it. A 304 still carries it.`) if (result._tag === 'failed') process.exitCode = 1 } diff --git a/scripts/lib/edge-cache-check.ts b/scripts/lib/edge-cache-check.ts index 194ee4d9..b7869f60 100644 --- a/scripts/lib/edge-cache-check.ts +++ b/scripts/lib/edge-cache-check.ts @@ -19,6 +19,9 @@ * 7. A stored page carries an ETag, and a request that sends it back gets a * 304. A 304 must never be stored as the page: after a conditional request * on a cold entry, the next plain request still gets the whole page. + * Cloudflare removes the ETag from an HTML 200 while a zone feature that + * rewrites HTML is on, such as Email Obfuscation. A 304 still carries it, + * so the check reads it there and warns instead of failing. */ import { varyKeysEdgeCache } from '../../shared/content-negotiation' @@ -65,9 +68,17 @@ export type EdgeCacheFailure | { _tag: 'not-revalidated', path: string, status: number } | { _tag: 'stored-not-modified', path: string, status: number } +/** True of production, and worth fixing, but no reason to fail a deploy. */ +export interface EdgeCacheWarning { _tag: 'etag-stripped', path: string } + export type EdgeCacheCheckResult - = | { _tag: 'passed', checks: Array<{ path: string, cacheStatuses: string[] }> } - | { _tag: 'failed', failures: EdgeCacheFailure[] } + = | { _tag: 'passed', checks: Array<{ path: string, cacheStatuses: string[] }>, warnings: EdgeCacheWarning[] } + | { _tag: 'failed', failures: EdgeCacheFailure[], warnings: EdgeCacheWarning[] } + +interface RevalidationOutcome { + failure: EdgeCacheFailure | null + warning: EdgeCacheWarning | null +} export interface EdgeCacheCheckDependencies { baseUrl: string @@ -240,17 +251,43 @@ async function checkSessionCookie(path: string, url: string, fetch: EdgeCacheFet * one variant per `Accept` value, so an `Accept` that names the nonce reaches * a cold entry that still renders HTML. */ -async function checkRevalidation(path: string, url: string, fetch: EdgeCacheFetch, nonce: string): Promise { - const networkError = (message: string): EdgeCacheFailure => ({ _tag: 'network-error', path, request: 'revalidation', message }) +async function checkRevalidation(path: string, url: string, fetch: EdgeCacheFetch, nonce: string): Promise { + const found = await findEtag(path, url, fetch) + if (found._tag === 'failed') + return { failure: found.failure, warning: null } + const warning: EdgeCacheWarning | null = found._tag === 'stripped' ? { _tag: 'etag-stripped', path } : null + return { failure: await checkConditionalRequests(path, url, fetch, nonce, found.etag), warning } +} + +/** + * The ETag of the stored page. When the page arrives without one, + * `If-None-Match: *` asks for a 304, and the 304 carries the stored ETag. + */ +async function findEtag(path: string, url: string, fetch: EdgeCacheFetch): Promise< + | { _tag: 'sent' | 'stripped', etag: string } + | { _tag: 'failed', failure: EdgeCacheFailure } +> { + const networkError = (message: string): { _tag: 'failed', failure: EdgeCacheFailure } => + ({ _tag: 'failed', failure: { _tag: 'network-error', path, request: 'revalidation', message } }) const stored = await request(fetch, url, BROWSER_HEADERS) if (stored._tag === 'error') return networkError(stored.message) if (stored.response.status !== 200) - return { _tag: 'status', path, request: 'revalidation', expected: 200, actual: stored.response.status } - const etag = stored.response.headers.get('etag') - if (!etag) - return { _tag: 'no-etag', path } + return { _tag: 'failed', failure: { _tag: 'status', path, request: 'revalidation', expected: 200, actual: stored.response.status } } + const sent = stored.response.headers.get('etag') + if (sent) + return { _tag: 'sent', etag: sent } + + const probe = await request(fetch, url, { ...BROWSER_HEADERS, 'if-none-match': '*' }) + if (probe._tag === 'error') + return networkError(probe.message) + const etag = probe.response.status === 304 ? probe.response.headers.get('etag') : null + return etag ? { _tag: 'stripped', etag } : { _tag: 'failed', failure: { _tag: 'no-etag', path } } +} + +async function checkConditionalRequests(path: string, url: string, fetch: EdgeCacheFetch, nonce: string, etag: string): Promise { + const networkError = (message: string): EdgeCacheFailure => ({ _tag: 'network-error', path, request: 'revalidation', message }) const conditional = await request(fetch, url, { ...BROWSER_HEADERS, 'if-none-match': etag }) if (conditional._tag === 'error') @@ -298,6 +335,7 @@ export async function checkEdgeCache(dependencies: EdgeCacheCheckDependencies): const nonce = dependencies.nonce ?? crypto.randomUUID() const failures: EdgeCacheFailure[] = [] + const warnings: EdgeCacheWarning[] = [] const checks: Array<{ path: string, cacheStatuses: string[] }> = [] const identities = new Map() for (const path of paths) { @@ -310,8 +348,10 @@ export async function checkEdgeCache(dependencies: EdgeCacheCheckDependencies): checks.push({ path, cacheStatuses: anonymous.cacheStatuses }) identities.set(anonymous.identity, [...identities.get(anonymous.identity) ?? [], path]) const revalidation = await checkRevalidation(path, url, fetch, nonce) - if (revalidation) - failures.push(revalidation) + if (revalidation.failure) + failures.push(revalidation.failure) + if (revalidation.warning) + warnings.push(revalidation.warning) } // After the anonymous loop, so the apex entry is warm. @@ -327,5 +367,5 @@ export async function checkEdgeCache(dependencies: EdgeCacheCheckDependencies): failures.push({ _tag: 'shared-cache-entry', paths: sharing, identity }) } - return failures.length ? { _tag: 'failed', failures } : { _tag: 'passed', checks } + return failures.length ? { _tag: 'failed', failures, warnings } : { _tag: 'passed', checks, warnings } } diff --git a/test/unit/edge-cache-check.test.ts b/test/unit/edge-cache-check.test.ts index 995458fe..8b445eb2 100644 --- a/test/unit/edge-cache-check.test.ts +++ b/test/unit/edge-cache-check.test.ts @@ -98,6 +98,7 @@ describe('checkEdgeCache', () => { { path: '/skills/trending', cacheStatuses: ['MISS', 'HIT'] }, { path: '/skills/trending?range=all', cacheStatuses: ['MISS', 'HIT'] }, ], + warnings: [], }) }) @@ -195,6 +196,7 @@ describe('checkEdgeCache', () => { expect(result).toEqual({ _tag: 'failed', failures: [{ _tag: 'never-served-from-cache', path: '/skills/trending', cacheStatuses: ['MISS', 'MISS', 'MISS', 'MISS'] }], + warnings: [], }) }) @@ -318,11 +320,23 @@ describe('checkEdgeCache', () => { expect(conditional).toEqual([STORED_ETAG, STORED_ETAG]) }) - it('fails when a stored page carries no ETag', async () => { + // Production on 2026-10-13: zone features that rewrite HTML were on, such as + // Email Obfuscation. The HTML 200 lost its ETag, and `If-None-Match: *` got a + // 304 that carried it. + it('warns, and still checks the 304, when Cloudflare strips the ETag from the page', async () => { const result = await run(edge({ anonymous: (_path, count) => ({ headers: { 'cf-cache-status': count === 1 ? 'MISS' : 'HIT', 'etag': '' } }), })) + expect(result).toMatchObject({ _tag: 'passed', warnings: [{ _tag: 'etag-stripped', path: '/skills/trending' }] }) + }) + + it('fails when neither the page nor a 304 carries an ETag', async () => { + const result = await run(edge({ + anonymous: (_path, count) => ({ headers: { 'cf-cache-status': count === 1 ? 'MISS' : 'HIT', 'etag': '' } }), + conditional: () => ({ headers: { 'cf-cache-status': 'HIT', 'etag': '' } }), + })) + expect(result).toMatchObject({ _tag: 'failed', failures: [{ _tag: 'no-etag', path: '/skills/trending' }] }) })