fix: update stored cache entry on synchronous 304 revalidation

[jeswr]

Fixes #5505

Problem

#4617 (fixing #4596) taught CacheHandler to merge 304 responses into the stored entry, but that code path was only reachable from the stale-while-revalidate background refresh. On the synchronous revalidation path, CacheRevalidationHandler short-circuited on statusCode === 304 and never forwarded anything to the wrapped CacheHandler, so:

1. Headers carried by the 304 (notably a new Cache-Control) were discarded. RFC 9111 §4.3.4 says the cache MUST update the stored response with the 304's header fields and recompute freshness. An origin extending freshness via 304 + Cache-Control: max-age=60 got zero benefit: undici revalidated again on the very next request, every time, forever.
2. Even a bare 304 left the entry stale, producing per-request conditional traffic instead of a fresh window.
3. The revalidated response was served with Warning: 110 - "response is stale" although it had just been successfully validated (TODO in lib/interceptor/cache.js; the Warning header is also obsolete per RFC 9111 §5.5).

Additionally, CacheHandler's cacheability pre-checks ran against the 304's own headers, so bare 304s bypassed the merge branch even on the background path, and the merged entry lost its stored Vary.

Repro

const { Agent, interceptors, cacheStores, request } = require('undici')
const http = require('node:http')
const { setTimeout: sleep } = require('node:timers/promises')

let hits = 0
const server = http.createServer((req, res) => {
  hits++
  if (req.headers['if-none-match'] === '"v1"') {
    res.writeHead(304, { 'cache-control': 'max-age=60', etag: '"v1"' })
    return res.end()
  }
  res.writeHead(200, { 'cache-control': 'max-age=1', etag: '"v1"' })
  res.end('hello')
}).listen(0, async () => {
  const origin = `http://localhost:${server.address().port}`
  const d = new Agent().compose(interceptors.cache({ store: new cacheStores.MemoryCacheStore() }))
  const get = () => request(origin, { dispatcher: d }).then(r => r.body.text())
  await get()            // hits=1 (stored, fresh 1s)
  await sleep(1400)
  await get()            // hits=2 (revalidate -> 304 max-age=60)
  await get()            // expected: served fresh from the updated entry
  await get()
  console.log(hits)
  server.close()
})

Before (main @ cb4c2f1): hits = 4 and keeps growing on every request; validated response served with warning: 110 - "response is stale" and the old cache-control: max-age=1.
After: hits = 2; validated response served with no warning, the merged cache-control: max-age=60, and age: 0; subsequent requests are cache hits.

Changes

• lib/handler/cache-handler.js: 304 handling moved in front of the cacheability gates into a #handle304 method. Freshness (staleAt/deleteAt/cachedAt) is recomputed from the merged (stored + 304) headers per §4.3.4, so bare 304s also refresh the entry; the stored Vary and ETag are preserved (an ETag on the 304 wins if usable); Content-Length from the 304 is not merged. New exported helper mergeValidationHeaders.
• lib/handler/cache-revalidation-handler.js: on a 304 the response events are forwarded to a dedicated cache-update CacheHandler (silent downstream), and the validation response's status/headers are passed to the callback.
• lib/interceptor/cache.js: on successful validation the cached body is served with the freshened headers, age: 0, and without the Warning: 110 header (still emitted for stale-if-error 5xx fallbacks and stale-while-revalidate serves, which really are stale).
• Fixed the 304 body-reuse path byte-iterating Buffer bodies returned by SqliteCacheStore (Buffer.prototype.values() yields single bytes, corrupting the re-written entry).

Tests

• New: test/interceptors/cache-revalidate-stale.js — "304 revalidation response updates the stored entry (RFC 9111 §4.3.4)" and "bare 304 revalidation response refreshes the stored entry". Both fail on main, pass with this change.
• Cache unit suites (test/interceptors/cache*.js, test/cache-interceptor/*.js): 123/123 pass (121 baseline + 2 new).
• mnot cache-tests conformance (node test/cache-interceptor/cache-tests.mjs): un-skipped four 304-etag-update-response-* tests ("We're not caching 304s currently"); they now pass in all 4 environments (shared/private × MemoryCacheStore/SqliteCacheStore). Totals: Passed 220 → 238, Failed 0 → 0, Failed-optional 58 → 44 (14 optional update304 checks flipped to yes), Setup-failed 5 → 5. A per-test diff against main shows every status change is within the update304 family; nothing else moved.
• 304-etag-update-response-Cache-Control stays skipped with an updated comment: its stored entry (max-age=1) is deleted at ~2x its freshness lifetime, before the test's 3s pause, so there is nothing left to revalidate — that needs the entry-retention floor discussed in fix: max-age=0 and no-cache must be considered "immediately stale" #4624 (fix/cache-store-etag-no-cache territory), not 304 merging.

Related

Self-contained against main; sibling PRs from the same review touch adjacent logic: jeswr:fix/cache-if-modified-since-value (revalidation request headers), jeswr:fix/cache-request-max-age-zero (request-directive revalidation path), jeswr:fix/cache-store-etag-no-cache (entry retention for revalidation-only responses).

This change is agent-assisted (Claude Fable 5) working with @jeswr.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 88.47584% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.43%. Comparing base (cb4c2f1) to head (24642c2).

Files with missing lines	Patch %	Lines
lib/handler/cache-handler.js	85.85%	28 Missing ⚠️
lib/handler/cache-revalidation-handler.js	92.85%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5516      +/-   ##
==========================================
- Coverage   93.44%   93.43%   -0.02%     
==========================================
  Files         110      110              
  Lines       37328    37486     +158     
==========================================
+ Hits        34881    35024     +143     
- Misses       2447     2462      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.