Repository navigation
fix(http)!: advertise the lifetimes ocache actually enforces - #86
Conversation
Three findings, all in the one `serialize` block.
10.2 -- `serialize` read the *static* `opts.maxAge` even though `cache.ts` had
already resolved `entry.maxAge`/`entry.staleMaxAge` onto the very entry it was
handed. ocache expired its own entry on the dynamic lifetime while telling
every cache downstream the static one, which is most of a dynamic TTL's purpose
gone. It now advertises `entry.maxAge ?? opts.maxAge` (and the same for
`staleMaxAge`) -- the per-field precedence `cache.ts` already applies to the
freshness check, the storage TTL and `expireCache`'s rewrite. `http/index.ts`
always installs its `getMaxAge` wrapper, so both fields are always resolved
here: `undefined` means "no override", never "not asked".
10.3 -- `s-maxage` is shared-cache-only, so a private cache had no freshness
lifetime at all and fell back to heuristic freshness over `Date -
Last-Modified` (RFC 9111 4.2.2). `last-modified` is stamped at fill time, so
that is about zero and browsers revalidated on every navigation while the
server held the entry for `maxAge`. `max-age` is now emitted alongside
`s-maxage`, carrying the same number -- the one ocache itself enforces; smaller
would be unenforced fiction, larger an overclaim. `s-maxage` overrides
`max-age` in a shared cache (5.2.2.10) so CDNs are unaffected, and it stays
rather than being folded away because it is what authorizes a shared cache to
store the response to an `Authorization`-carrying request at all (3.5,
reachable under `allowAuthorization`).
10.4 -- the bare `stale-while-revalidate` emitted for the ISR shape (`swr`
with no `staleMaxAge`) is invalid syntax: RFC 5861 3 requires the
delta-seconds, so conforming caches dropped the directive wholesale and the
intended window evaporated downstream while reading as if it hadn't. Nothing
replaces it. That shape's stale window is genuinely unbounded -- the entry is
retained until the backend evicts it by capacity -- so there is no honest
number, and any invented one would advertise a window ocache cannot promise,
which is the finding-10 failure mode in the permissive direction. Silence
under-promises: downstream revalidates when `max-age` runs out and ocache
answers that from its own stale entry, so ISR still happens one layer in,
where it is actually enforced.
Directive strings, before -> after:
{ maxAge: 60 } max-age=60 (unchanged)
{ maxAge: 60, swr } s-maxage=60, swr -> max-age=60, s-maxage=60
{ maxAge: 60, swr, staleMaxAge:120 } s-maxage=60, swr=120 -> max-age=60, s-maxage=60, swr=120
{ maxAge: 3600, getMaxAge: ()=>2 } max-age=3600 -> max-age=2
Also fixes the docs claim that an empty body is never cached: `validate`
accepts `""` and rejects only `undefined`, so a zero-byte 200 caches normally.
BREAKING CHANGE: the synthesized `Cache-Control` changes shape. Private caches
now get a real `max-age` where they previously revalidated every time, and the
ISR shape no longer emits a `stale-while-revalidate` token at all.
Closes findings 10.2, 10.3 and 10.4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 19 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
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. Comment |
Closes findings 10.2, 10.3 and 10.4 — all three live in the same
serializeblock, so they're one sitting.10.2 —
getMaxAgenever reached the headerserializeread the staticopts.maxAgeeven thoughcache.tshad already resolvedentry.maxAge/entry.staleMaxAgeonto the very entry it was handed. ocache expired its own entry on the dynamic lifetime while telling every cache downstream the static one — most of a dynamic TTL's purpose, gone.It now advertises
entry.maxAge ?? opts.maxAge(same forstaleMaxAge) — the per-field precedencecache.tsalready applies to the freshness check, the storage TTL andexpireCache's rewrite. This is sound becausehttp/index.tsalways installs itsgetMaxAgewrapper, so both fields are always resolved by the timeserializesees the entry:undefinedthere means "no override", never "not asked".The wrapper's own
must-revalidate→staleMaxAge: 0can never leak into the advertisement: it only exists when the handler set aCache-Control, which is exactly when thehas("cache-control")check has already suppressed synthesis. There's a test asserting that.10.3 —
max-agealongsides-maxage, carrying the same numbers-maxageis shared-cache-only, so a private cache had no freshness lifetime at all and fell back to heuristic freshness overDate − Last-Modified(RFC 9111 §4.2.2).last-modifiedis stamped at fill time, so that's ≈0 — browsers revalidated on every navigation while the server happily held the entry formaxAge.Same number as
s-maxage, because that is the one ocache actually enforces for the entry: smaller would be unenforced fiction, larger an overclaim.s-maxageoverridesmax-agein a shared cache (§5.2.2.10), so CDN behavior is unchanged; the gain is entirely for private caches.s-maxageis kept rather than folded away into a lonemax-age(which would already govern shared caches): §3.5 makes it separately what authorizes a shared cache to store the response to anAuthorization-carrying request — reachable underallowAuthorization.10.4 — the bare
stale-while-revalidateis gone, and nothing replaces itRFC 5861 §3 requires the delta-seconds, so the bare token emitted for the ISR shape was unparseable and conforming caches had to drop the directive wholesale (RFC 9111 §5.2.3) — the intended window evaporated downstream while reading as though it hadn't.
Nothing replaces it. That shape's stale window is genuinely unbounded — the entry is retained until the backend evicts it by capacity — so there is no honest number, and any invented one (
maxAge, a year, a constant) would advertise a stale window ocache cannot promise. That is precisely the finding-10 failure mode, in the permissive direction.Silence under-promises, which is the safe direction: a downstream cache revalidates when
max-ageruns out, ocache answers that from its own retained stale copy (or a 304) and refreshes in the background. ISR still happens — one layer in, where it is actually enforced.Directive strings, before → after
{ maxAge: 60 }max-age=60{ maxAge: 60, swr: true }(ISR)s-maxage=60, stale-while-revalidatemax-age=60, s-maxage=60{ maxAge: 60, swr: true, staleMaxAge: 120 }s-maxage=60, stale-while-revalidate=120max-age=60, s-maxage=60, stale-while-revalidate=120{ maxAge: 0 }max-age=0{ maxAge: 0, swr: true }s-maxage=0, stale-while-revalidatemax-age=0, s-maxage=0(still refused){ maxAge: 3600, getMaxAge: () => 2 }max-age=3600max-age=2{ maxAge: 3600, staleMaxAge: 7200, swr: true, getMaxAge: () => ({maxAge: 2, staleMaxAge: 30}) }s-maxage=3600, stale-while-revalidate=7200max-age=2, s-maxage=2, stale-while-revalidate=30cache-controlsendCacheControl: falseAlso
Fixes the tracked docs bug in
docs/1.guide/8.cache-control.md: "Empty body — nothing to replay" sat under "These checks always apply and cannot be turned off" and is false —validateaccepts""and rejects onlyundefined(measured:new Response("", {status:200})→ HIT). Also removed twoAGENTS.mdclaims that the synthesized header carriespublic, which it never has.Breaking
The synthesized
Cache-Controlchanges shape: private caches now get a realmax-agewhere they previously revalidated every time, and the ISR shape emits nostale-while-revalidatetoken at all.Flagged for a possible follow-up
{ swr: true, maxAge: 0, staleMaxAge: 600 }still shipsstale-while-revalidate=600next to a zero lifetime — ocache stores nothing, so downstream is told it may serve stale for 600 s that ocache never keeps. Pre-existing and unchanged by this diff, and it is the literal reading of what the caller configured; suppressing it would be a further narrowing that didn't belong in this PR silently.Verification
pnpm vitest run test/— 357 passed (346 on main; 11 new in adescribe("synthesized Cache-Control lifetimes")block, 4 pre-existing assertions updated to the new strings)pnpm typecheck,pnpm lint— cleanpnpm bundle— cheaper on two of three axes, no budget touched: raw 49,632 → 49,449 (−183), min 20,161 → 20,132 (−29), minGzip 8,057 → 8,071 (+14). The removedelsebranch paid for the new one.🤖 Generated with Claude Code