Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions scripts/check-edge-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ async function main(): Promise<void> {
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
}
Expand Down
62 changes: 51 additions & 11 deletions scripts/lib/edge-cache-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<EdgeCacheFailure | null> {
const networkError = (message: string): EdgeCacheFailure => ({ _tag: 'network-error', path, request: 'revalidation', message })
async function checkRevalidation(path: string, url: string, fetch: EdgeCacheFetch, nonce: string): Promise<RevalidationOutcome> {
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<EdgeCacheFailure | null> {
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')
Expand Down Expand Up @@ -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<string | null, string[]>()
for (const path of paths) {
Expand All @@ -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.
Expand All @@ -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 }
}
16 changes: 15 additions & 1 deletion test/unit/edge-cache-check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ describe('checkEdgeCache', () => {
{ path: '/skills/trending', cacheStatuses: ['MISS', 'HIT'] },
{ path: '/skills/trending?range=all', cacheStatuses: ['MISS', 'HIT'] },
],
warnings: [],
})
})

Expand Down Expand Up @@ -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: [],
})
})

Expand Down Expand Up @@ -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' }] })
})

Expand Down
Loading