Skip to content

fix(cache): add response delay to corrected age value - #5659

Open
RaphaelFakhri wants to merge 2 commits into
nodejs:mainfrom
RaphaelFakhri:fix/rfc9111-corrected-age-value
Open

fix(cache): add response delay to corrected age value#5659
RaphaelFakhri wants to merge 2 commits into
nodejs:mainfrom
RaphaelFakhri:fix/rfc9111-corrected-age-value

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

RFC 9111 section 4.2.3 defines a stored response's initial age as:

apparent_age           = max(0, response_time - date_value)
response_delay         = response_time - request_time
corrected_age_value    = age_value + response_delay
corrected_initial_age  = max(apparent_age, corrected_age_value)

CacheHandler captured a single timestamp at response-header time and computed
max(apparentAge, resAge). There is no request_time, so response_delay was always
zero and the corrected_age_value term reduced to the bare Age the origin sent.

The delay matters because the Age an origin reports is already out of date by the time the
response finishes arriving. Dropping it stores a slow response as younger than it is, and a
response near its freshness boundary is then served after it has actually expired.

Reproduction

Origin sends Cache-Control: public, max-age=600, Age: 100, and delays 3 seconds:

response_delay measured   : 3030 ms
upstream Age              : 100 s
RFC corrected_initial_age : >= 103 s
Age served from cache     : 100 s     <- understated by the full response delay

The understatement scales with upstream latency, so it is largest exactly where caching
matters most.

Change

onRequestStart records the request time, and corrected_age_value adds the resulting
delay. The field is reset on every onRequestStart, so a retried or redirected request
measures its own delay rather than the first attempt's.

Tests

test/interceptors/cache-corrected-age.js, three cases:

  • the served age includes the response delay
  • a response without an Age header is aged by at most the response delay itself, since
    section 4.2.3 treats a missing Age as age_value = 0
  • a response whose corrected age already exceeds max-age is not reused

Cases 1 and 3 fail on main and pass with this change. Case 2 passes on both and is there to
pin the bound on responses that carry no Age.

npm run test:cache-interceptor passes 76/76, and lint is clean.

RFC 9111 section 4.2.3 computes a stored response's initial age from

  response_delay        = response_time - request_time
  corrected_age_value   = age_value + response_delay
  corrected_initial_age = max(apparent_age, corrected_age_value)

CacheHandler captured a single timestamp at response-header time and computed
max(apparentAge, resAge), so response_delay was always zero and corrected_age_value
reduced to the bare Age the origin sent.

The delay matters because the Age an origin reports is already out of date by the
time the response finishes arriving. Dropping it stores a slow response as younger
than it is, and a response near its freshness boundary is then served after it has
actually expired. Against an origin reporting Age: 100 that takes 3 seconds to
respond, the cache served an age of 100 where the RFC requires at least 103.

onRequestStart now records the request time, and the correction adds the resulting
delay. The field resets on every onRequestStart so a retried or redirected request
measures its own delay. A missing Age is treated as age_value = 0 per the same
section, so a slow response without one is still aged by the delay.
@metcoder95
metcoder95 requested a review from mcollina August 7, 2026 09:15

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you avoid using setTimeout in tests but use fake-timers instead? timers are flaky.

Real setTimeout in the origin handler made the response delay a wall-clock timer,
which is flaky under load. Switch to @sinonjs/fake-timers with toFake: ['Date'],
the pattern already used in test/interceptors/cache.js, and inject the response
delay by advancing the clock inside the handler: it runs after the request was sent
and before the response is received, so Date.now() moves by exactly the delay. The
assertions become exact ages rather than lower bounds, and nothing depends on a real
timer.
@RaphaelFakhri

Copy link
Copy Markdown
Author

Done, switched to @sinonjs/fake-timers with toFake: ['Date'], same pattern as test/interceptors/cache.js. The response delay is now injected via clock.tick() inside the origin handler, so the assertions are exact values instead of lower bounds and nothing depends on real timers. The http2-request-never-settles timeout on Node 25 looks unrelated to this change (http2 test, pre-existing flake).

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