Skip to content

fix(cache): only apply 1-year deleteAt for immutable responses#4913

Merged
mcollina merged 2 commits intonodejs:mainfrom
metalix2:fix/determine-delete-at-immutable
Mar 19, 2026
Merged

fix(cache): only apply 1-year deleteAt for immutable responses#4913
mcollina merged 2 commits intonodejs:mainfrom
metalix2:fix/determine-delete-at-immutable

Conversation

@metalix2
Copy link
Contributor

@metalix2 metalix2 commented Mar 19, 2026

This relates to...

determineDeleteAt in lib/handler/cache-handler.js unconditionally applying a 1-year deleteAt to all responses without stale-while-revalidate or stale-if-error directives.

Rationale

A Cache-Control: max-age=60 response was getting deleteAt set to now + 1 year instead of a value proportional to its freshness lifetime. This causes cache stores (especially persistent stores like Redis/SQLite) to retain entries far longer than intended, wasting storage.

The variable was named immutable but the condition never checked for the actual immutable cache-control directive.

Changes

Features

N/A

Bug Fixes

  • Guard the 1-year deleteAt fallback behind an actual cacheControlDirectives.immutable check
  • For non-immutable responses without stale directives, set deleteAt to staleAt + freshnessLifetime (effectively 2× max-age), giving entries enough time past staleAt for revalidation without lingering indefinitely

Before: Cache-Control: max-age=60deleteAt ≈ 1 year out
After: Cache-Control: max-age=60deleteAt ≈ 120s out (60s fresh + 60s revalidation buffer)

Note: when immutable IS set, determineStaleAt already returns 1 year for staleAt, so the 1-year deleteAt is a belt-and-suspenders safety net for the deleteAt side.

Expected Changes

Cache entries without immutable, stale-while-revalidate, or stale-if-error will now expire sooner from cache stores. This is the correct behavior per RFC 9111 but could cause more cache misses for deployments that were (incorrectly) relying on the 1-year retention.

Benchmark

                                                            
  ┌─────────┬───────────────────────┬─────────┬────────────────────┬─────────────┬─────────────────────────┐                                                                                                                                                  
  │ (index) │ Tests                 │ Samples │ Result             │ Tolerance   │ Difference with slowest │                                                                                                                                                  
  ├─────────┼───────────────────────┼─────────┼────────────────────┼─────────────┼─────────────────────────┤                                                                                                                                                  
  │ 0       │ 'http - keepalive'    │ 101     │ '2841.15 req/sec'  │ '± 27.36 %' │ '-'                     │
  │ 1       │ 'http - no keepalive' │ 101     │ '3438.36 req/sec'  │ '± 22.81 %' │ '+ 21.02 %'             │
  │ 2       │ 'got'                 │ 101     │ '9018.10 req/sec'  │ '± 5.39 %'  │ '+ 217.41 %'            │
  │ 3       │ 'axios'               │ 101     │ '9045.68 req/sec'  │ '± 4.56 %'  │ '+ 218.38 %'            │
  │ 4       │ 'node-fetch'          │ 101     │ '10455.97 req/sec' │ '± 5.00 %'  │ '+ 268.02 %'            │
  │ 5       │ 'superagent'          │ 101     │ '11030.21 req/sec' │ '± 4.95 %'  │ '+ 288.23 %'            │
  │ 6       │ 'request'             │ 70      │ '11238.50 req/sec' │ '± 2.97 %'  │ '+ 295.56 %'            │
  │ 7       │ 'undici - fetch'      │ 101     │ '12466.29 req/sec' │ '± 3.19 %'  │ '+ 338.78 %'            │
  │ 8       │ 'undici - pipeline'   │ 101     │ '12970.99 req/sec' │ '± 4.81 %'  │ '+ 356.54 %'            │
  │ 9       │ 'undici - request'    │ 101     │ '13271.45 req/sec' │ '± 5.61 %'  │ '+ 367.12 %'            │
  │ 10      │ 'undici - stream'     │ 101     │ '13296.19 req/sec' │ '± 5.33 %'  │ '+ 367.99 %'            │
  │ 11      │ 'undici - dispatch'   │ 85      │ '14256.85 req/sec' │ '± 2.90 %'  │ '+ 401.80 %'            │
  └─────────┴───────────────────────┴─────────┴────────────────────┴─────────────┴─────────────────────────┘


Status

metalix2 and others added 2 commits March 19, 2026 17:07
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify that:
- max-age responses get deleteAt proportional to freshness lifetime (not 1 year)
- immutable responses get deleteAt of ~1 year
- stale-while-revalidate correctly extends deleteAt beyond staleAt

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.90%. Comparing base (1c5dc1a) to head (8830832).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4913   +/-   ##
=======================================
  Coverage   92.90%   92.90%           
=======================================
  Files         112      112           
  Lines       35638    35646    +8     
=======================================
+ Hits        33108    33118   +10     
+ Misses       2530     2528    -2     

☔ View full report in Codecov by Sentry.
📢 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.

@mcollina mcollina merged commit 9077500 into nodejs:main Mar 19, 2026
36 of 37 checks passed
@github-actions github-actions bot mentioned this pull request Mar 19, 2026
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.

3 participants