Skip to content

feat(runtime): conditional GET (ETag/304) for cacheable HTML pages - #1239

Draft
guitavano wants to merge 1 commit into
mainfrom
guitavano/conditional-get-etag
Draft

guitavano wants to merge 1 commit into
mainfrom
guitavano/conditional-get-etag

Conversation

@guitavano

@guitavano guitavano commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Draft — this is the mechanism (foundational half). Read "What still blocks the win" before merging. It is safe to ship as-is (no-op until the companion fix lands), but on its own it will rarely emit a 304 on real farmrio pages.

Problem

Cacheable pages carry Cache-Control: public, max-age=180, stale-while-revalidate=3600 but no validator. So every CDN revalidation — once per max-age, per cache variant, per POP — re-fetches the full body from the origin even when the render is byte-for-byte the content the CDN already holds. On a high-traffic catalog with a 180s TTL this revalidation churn is the dominant slice of the #1 infra cost (Egress→Cloudflare ≈ 69%).

There is conditional-GET/ETag handling for static assets (runtime/htmx/serveStatic.ts) and schema meta, but not for the HTML page path.

What this does

Emits a weak ETag on cacheable 200 HTML and answers 304 Not Modified when the CDN revalidates with a matching If-None-Match, collapsing an unchanged revalidation to headers only.

The ETag hashes the rendered HTML with only per-request cosmetic tokens masked out — the CSP nonce (unique per response by design) and renderSalt. Everything semantic — prices, stock, product order, copy — stays in the hash, so any real change yields a fresh 200. The ETag never governs freshness (max-age/SWR + the loader cache still do); it only avoids re-sending a body the CDN already has. A token that slips the mask just varies the ETag → no 304, no harm.

  • runtime/conditionalGet.ts — normalizeForEtag, weakEtagFor, ifNoneMatchSatisfied (RFC 9110 §13.1.2 weak comparison).
  • runtime/middleware.ts — gated to cacheable 200 HTML (a no-store response is never revalidated). Body is buffered to hash it; streaming is preserved for every other response, and cacheable renders only happen on MISS/revalidation (users are served by the CDN), so buffering is off the hot path.

What still blocks the win (measured)

I fetched the same farmrio PLP twice from origin (identical product set: 47/47 SKUs). After masking nonce + renderSalt, the only remaining per-request difference was ~30 lines: one deco section-instance id (nmP0315333356 vs nmP0315788765). That id is ${idPrefix}-${renderSalt} where renderSalt derives from a per-render salt / preact useId (components/section.tsx:199-202) that varies every render.

Consequence: the New Menu (in the header → every page) carries such an id, so this ETag will not 304 on farmrio pages until section-instance ids are made deterministic per page render. That companion change (deterministic renderSalt/useId seeding) touches the section/partial render path and Fast Preview, so it is intentionally not in this PR — but it is the required second half. This PR is the safe, no-op-until-effective mechanism.

Tests

runtime/conditionalGet.test.ts — masks nonce/renderSalt; ETag stable across differing nonce+salt; ETag changes when the price changes; weak If-None-Match match (weak/strong/list/*) and non-match. 5/5 green (deno test, fmt/lint clean; full deno check blocked by this env's dependency-date lock, not the code).

🤖 Generated with Claude Code


Summary by cubic

Adds conditional GET (ETag/304) for cacheable HTML pages so CDN revalidations fetch headers only when the render is unchanged. Cacheable pages carried Cache-Control freshness but no validator, so every revalidation re-fetched the full body from origin.

Emits a weak ETag on cacheable 200 HTML and answers 304 on matching If-None-Match, with the CSP nonce and renderSalt masked before hashing so only per-request cosmetics vary the hash. Gated to cacheable 200 HTML and buffers the body only for hashing; streaming is preserved elsewhere.

What still blocks the win

On farmrio pages, section-instance ids still vary per render (they derive from a per-render salt), so the ETag won't match between renders. This PR is a safe no-op there until a companion change makes render salts deterministic per page render.

Written for commit aea6aa7. Summary will update on new commits.

Review in cubic

Cacheable pages carry `Cache-Control: max-age, stale-while-revalidate`
but no validator, so every CDN revalidation (once per max-age, per cache
variant, per POP) re-fetches the FULL body from the origin even when the
render is the content the CDN already holds. On a high-traffic catalog
with a short max-age this revalidation churn is the dominant origin
egress.

This emits a weak ETag on cacheable 200 HTML and answers 304 when the
CDN revalidates with a matching If-None-Match — collapsing an unchanged
revalidation to headers only.

The ETag hashes the rendered HTML with only per-request COSMETIC tokens
masked out (CSP `nonce`, `renderSalt`). Everything semantic — prices,
stock, product order, copy — stays in the hash, so any real change yields
a fresh 200. The ETag never governs freshness (max-age/SWR + loader cache
still do); it only avoids re-sending a body the CDN already has, and a
token that slips the mask just varies the ETag (no 304, no harm).

Gated to cacheable 200 HTML (a no-store response is never revalidated).
The body is buffered to hash it; streaming is preserved for every other
response, and cacheable renders only happen on MISS/revalidation (the CDN
serves users), so the buffering is off the hot path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 1.211.3 update
  • 🎉 for Minor 1.212.0 update
  • 🚀 for Major 2.0.0 update

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant