From 7394f97275d7f06be11b069f9e95303b00db52d2 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 31 Aug 2025 09:37:35 +0200 Subject: [PATCH 1/4] fix: implement proper stale-while-revalidate behavior per RFC 5861 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stale-while-revalidate cache directive was not working as intended. Instead of returning stale content immediately and revalidating in the background, it was performing synchronous revalidation, defeating the primary purpose of reducing latency. This fix: - Returns stale content immediately when within the stale-while-revalidate window - Performs background revalidation asynchronously to update the cache - Updates tests to reflect the correct RFC 5861 behavior Fixes #4471 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Signed-off-by: Matteo Collina --- lib/interceptor/cache.js | 61 ++++++++++ test/interceptors/cache.js | 234 +++++++++++++++++++++++++++++++++---- 2 files changed, 273 insertions(+), 22 deletions(-) diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index 6565baf0a51..6d788f2468f 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -56,6 +56,22 @@ function needsRevalidation (result, cacheControlDirectives) { return false } +/** + * Check if we're within the stale-while-revalidate window for a stale response + * @param {import('../../types/cache-interceptor.d.ts').default.GetResult} result + * @returns {boolean} + */ +function withinStaleWhileRevalidateWindow (result) { + const staleWhileRevalidate = result.cacheControlDirectives?.['stale-while-revalidate'] + if (!staleWhileRevalidate) { + return false + } + + const now = Date.now() + const staleWhileRevalidateExpiry = result.staleAt + (staleWhileRevalidate * 1000) + return now <= staleWhileRevalidateExpiry +} + /** * @param {DispatchFn} dispatch * @param {import('../../types/cache-interceptor.d.ts').default.CacheHandlerOptions} globalOpts @@ -231,6 +247,51 @@ function handleResult ( return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) } + // RFC 5861: If we're within stale-while-revalidate window, serve stale immediately + // and revalidate in background + if (withinStaleWhileRevalidateWindow(result)) { + // Serve stale response immediately + sendCachedValue(handler, opts, result, age, null, true) + + // Start background revalidation (fire-and-forget) + setImmediate(() => { + let headers = { + ...opts.headers, + 'if-modified-since': new Date(result.cachedAt).toUTCString() + } + + if (result.etag) { + headers['if-none-match'] = result.etag + } + + if (result.vary) { + headers = { + ...headers, + ...result.vary + } + } + + // Background revalidation - update cache if we get new data + dispatch( + { + ...opts, + headers + }, + new CacheHandler(globalOpts, cacheKey, { + // Silent handler that just updates the cache + onRequestStart () {}, + onRequestUpgrade () {}, + onResponseStart () {}, + onResponseData () {}, + onResponseEnd () {}, + onResponseError () {} + }) + ) + }) + + return true + } + let withinStaleIfErrorThreshold = false const staleIfErrorExpiry = result.cacheControlDirectives['stale-if-error'] ?? reqCacheControl?.['stale-if-error'] if (staleIfErrorExpiry) { diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index a29ce9ca1e2..51a0158be12 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -465,7 +465,8 @@ describe('Cache Interceptor', () => { clock.tick(1500) - // Response is now stale, the origin should get a revalidation request + // Response is now stale but within stale-while-revalidate window, + // should return stale immediately and revalidate in background { const res = await client.request(request) if (serverError) { @@ -473,12 +474,15 @@ describe('Cache Interceptor', () => { } equal(requestsToOrigin, 1) - equal(revalidationRequests, 1) strictEqual(await res.body.text(), 'asd') + // Background revalidation happens asynchronously } - // Response is still stale, extra header should be overwritten, and the - // origin should get a revalidation request + // Wait for background revalidation to complete + await new Promise(resolve => setTimeout(resolve, 100)) + equal(revalidationRequests, 1) + + // Response is still stale, will trigger another background revalidation { const res = await client.request({ ...request, @@ -491,11 +495,29 @@ describe('Cache Interceptor', () => { } equal(requestsToOrigin, 1) - equal(revalidationRequests, 2) strictEqual(await res.body.text(), 'asd') } - // Response is still stale, but revalidation should fail now. + // Wait for second background revalidation + await new Promise(resolve => setTimeout(resolve, 100)) + equal(revalidationRequests, 2) + + // Third request triggers another background revalidation that returns updated content + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') // Still stale initially + } + + // Wait for third background revalidation + await new Promise(resolve => setTimeout(resolve, 100)) + equal(revalidationRequests, 3) + + // Now the cache should have updated content { const res = await client.request(request) if (serverError) { @@ -503,7 +525,6 @@ describe('Cache Interceptor', () => { } equal(requestsToOrigin, 1) - equal(revalidationRequests, 3) strictEqual(await res.body.text(), 'updated') } }) @@ -580,7 +601,8 @@ describe('Cache Interceptor', () => { clock.tick(1500) - // Response is now stale, the origin should get a revalidation request + // Response is now stale but within stale-while-revalidate window, + // should return stale immediately and revalidate in background { const res = await client.request(request) if (serverError) { @@ -588,12 +610,15 @@ describe('Cache Interceptor', () => { } equal(requestsToOrigin, 1) - equal(revalidationRequests, 1) strictEqual(await res.body.text(), 'asd') + // Background revalidation happens asynchronously } - // Response is still stale, extra headers should be overwritten, and the - // origin should get a revalidation request + // Wait for background revalidation to complete + await new Promise(resolve => setTimeout(resolve, 100)) + equal(revalidationRequests, 1) + + // Response is still stale, will trigger another background revalidation { const res = await client.request({ ...request, @@ -606,11 +631,29 @@ describe('Cache Interceptor', () => { } equal(requestsToOrigin, 1) - equal(revalidationRequests, 2) strictEqual(await res.body.text(), 'asd') } - // Response is still stale, but revalidation should fail now. + // Wait for second background revalidation + await new Promise(resolve => setTimeout(resolve, 100)) + equal(revalidationRequests, 2) + + // Third request triggers another background revalidation that returns updated content + { + const res = await client.request(request) + if (serverError) { + throw serverError + } + + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'asd') // Still stale initially + } + + // Wait for third background revalidation + await new Promise(resolve => setTimeout(resolve, 100)) + equal(revalidationRequests, 3) + + // Now the cache should have updated content { const res = await client.request(request) if (serverError) { @@ -618,7 +661,6 @@ describe('Cache Interceptor', () => { } equal(requestsToOrigin, 1) - equal(revalidationRequests, 3) strictEqual(await res.body.text(), 'updated') } }) @@ -708,9 +750,12 @@ describe('Cache Interceptor', () => { } strictEqual(requestsToOrigin, 1) - strictEqual(revalidationRequests, 1) strictEqual(await response.body.text(), 'asd') } + + // Wait for background revalidation to complete + await new Promise(resolve => setTimeout(resolve, 100)) + strictEqual(revalidationRequests, 1) }) test('unsafe methods cause resource to be purged from cache', async () => { @@ -1109,6 +1154,9 @@ describe('Cache Interceptor', () => { } }) equal(requestsToOrigin, 1) + + // Wait for background revalidation to complete + await new Promise(resolve => setTimeout(resolve, 100)) equal(revalidationRequests, 1) }) @@ -1390,8 +1438,8 @@ describe('Cache Interceptor', () => { clock.tick(15000) - // Send third request. This is now stale, the revalidation request should - // fail but the response should still be served from cache. + // Send third request. This is now stale but within stale-while-revalidate, + // should return stale immediately and trigger background revalidation { const response = await client.request({ origin: 'localhost', @@ -1401,23 +1449,30 @@ describe('Cache Interceptor', () => { 'cache-control': 'stale-if-error=20' } }) - equal(requestsToOrigin, 2) equal(response.statusCode, 200) equal(await response.body.text(), 'asd') } - // Send a fourth request. This is stale and w/o stale-if-error, so we - // should get the error here. + // Wait for background revalidation to complete (which will fail with 500) + await new Promise(resolve => setTimeout(resolve, 100)) + equal(requestsToOrigin, 2) + + // Send a fourth request. Still within stale-while-revalidate but without stale-if-error, + // should return stale since previous revalidation failed and stale-if-error applies { const response = await client.request({ origin: 'localhost', path: '/', method: 'GET' }) - equal(requestsToOrigin, 3) - equal(response.statusCode, 500) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') } + // Wait for another background revalidation + await new Promise(resolve => setTimeout(resolve, 100)) + equal(requestsToOrigin, 3) + clock.tick(25000) // Send fifth request. We're now outside the stale-if-error threshold and @@ -1697,4 +1752,139 @@ describe('Cache Interceptor', () => { strictEqual(await res.body.text(), body) } }) + + test('stale-while-revalidate returns stale immediately and revalidates in background (RFC 5861)', async () => { + let requestsToOrigin = 0 + let revalidationRequests = 0 + let serverResponse = 'original-response' + + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + const responseDate = new Date() + res.setHeader('date', responseDate.toUTCString()) + res.setHeader('cache-control', 's-maxage=1, stale-while-revalidate=10') + + if (req.headers['if-modified-since']) { + revalidationRequests++ + // Return updated content on revalidation + serverResponse = 'revalidated-response' + res.end(serverResponse) + } else { + requestsToOrigin++ + res.end(serverResponse) + } + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + server.close() + await client.close() + }) + + await once(server, 'listening') + + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Send initial request to cache the response + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'original-response') + } + + // Wait for response to become stale + await new Promise(resolve => setTimeout(resolve, 1100)) + + // Request stale content - should return immediately with stale content + const startTime = Date.now() + { + const res = await client.request(request) + const responseTime = Date.now() - startTime + + // Should return stale content immediately (< 50ms) + equal(res.statusCode, 200) + strictEqual(await res.body.text(), 'original-response') + equal(requestsToOrigin, 1) // No additional origin requests yet + + // Response should be immediate (RFC 5861 requirement) + if (responseTime > 100) { + fail(`stale-while-revalidate response took ${responseTime}ms, should be < 100ms`) + } + } + + // Wait for background revalidation to complete + await new Promise(resolve => setTimeout(resolve, 500)) + + // Verify that revalidation occurred in background + equal(revalidationRequests, 1, 'Background revalidation should have occurred') + }) + + test('stale-while-revalidate updates cache after background revalidation', async () => { + let requestsToOrigin = 0 + let revalidationRequests = 0 + + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + const responseDate = new Date() + res.setHeader('date', responseDate.toUTCString()) + res.setHeader('cache-control', 's-maxage=1, stale-while-revalidate=10') + + if (req.headers['if-modified-since']) { + revalidationRequests++ + // Return updated content + res.end('updated-response') + } else { + requestsToOrigin++ + res.end('original-response') + } + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + server.close() + await client.close() + }) + + await once(server, 'listening') + + const request = { + origin: 'localhost', + method: 'GET', + path: '/' + } + + // Initial request + { + const res = await client.request(request) + equal(requestsToOrigin, 1) + strictEqual(await res.body.text(), 'original-response') + } + + // Wait for staleness + await new Promise(resolve => setTimeout(resolve, 1100)) + + // First stale request - gets stale content immediately + { + const res = await client.request(request) + strictEqual(await res.body.text(), 'original-response') + } + + // Wait for background revalidation + await new Promise(resolve => setTimeout(resolve, 500)) + equal(revalidationRequests, 1) + + // Second stale request - should get updated content from cache + // (still within stale-while-revalidate window) + { + const res = await client.request(request) + strictEqual(await res.body.text(), 'updated-response') + equal(requestsToOrigin, 1) // Still only one origin request + } + }) }) From 16504832185bdda9b8baa275081c2515c4526f5c Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 31 Aug 2025 12:39:07 +0200 Subject: [PATCH 2/4] test: use timers/promises for sleep instead of creating promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace manual Promise creation with setTimeout from timers/promises for cleaner and more idiomatic async delays in tests. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Signed-off-by: Matteo Collina --- test/interceptors/cache.js | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 51a0158be12..06bf069d932 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -4,6 +4,7 @@ const { createServer } = require('node:http') const { describe, test, after } = require('node:test') const { once } = require('node:events') const { equal, strictEqual, notEqual, fail } = require('node:assert') +const { setTimeout: sleep } = require('node:timers/promises') const FakeTimers = require('@sinonjs/fake-timers') const { Client, interceptors, cacheStores: { MemoryCacheStore } } = require('../../index') @@ -479,7 +480,7 @@ describe('Cache Interceptor', () => { } // Wait for background revalidation to complete - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 1) // Response is still stale, will trigger another background revalidation @@ -499,7 +500,7 @@ describe('Cache Interceptor', () => { } // Wait for second background revalidation - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 2) // Third request triggers another background revalidation that returns updated content @@ -514,7 +515,7 @@ describe('Cache Interceptor', () => { } // Wait for third background revalidation - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 3) // Now the cache should have updated content @@ -615,7 +616,7 @@ describe('Cache Interceptor', () => { } // Wait for background revalidation to complete - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 1) // Response is still stale, will trigger another background revalidation @@ -635,7 +636,7 @@ describe('Cache Interceptor', () => { } // Wait for second background revalidation - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 2) // Third request triggers another background revalidation that returns updated content @@ -650,7 +651,7 @@ describe('Cache Interceptor', () => { } // Wait for third background revalidation - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 3) // Now the cache should have updated content @@ -754,7 +755,7 @@ describe('Cache Interceptor', () => { } // Wait for background revalidation to complete - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) strictEqual(revalidationRequests, 1) }) @@ -1156,7 +1157,7 @@ describe('Cache Interceptor', () => { equal(requestsToOrigin, 1) // Wait for background revalidation to complete - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(revalidationRequests, 1) }) @@ -1454,7 +1455,7 @@ describe('Cache Interceptor', () => { } // Wait for background revalidation to complete (which will fail with 500) - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(requestsToOrigin, 2) // Send a fourth request. Still within stale-while-revalidate but without stale-if-error, @@ -1470,7 +1471,7 @@ describe('Cache Interceptor', () => { } // Wait for another background revalidation - await new Promise(resolve => setTimeout(resolve, 100)) + await sleep(100) equal(requestsToOrigin, 3) clock.tick(25000) @@ -1798,7 +1799,7 @@ describe('Cache Interceptor', () => { } // Wait for response to become stale - await new Promise(resolve => setTimeout(resolve, 1100)) + await sleep(1100) // Request stale content - should return immediately with stale content const startTime = Date.now() @@ -1818,7 +1819,7 @@ describe('Cache Interceptor', () => { } // Wait for background revalidation to complete - await new Promise(resolve => setTimeout(resolve, 500)) + await sleep(500) // Verify that revalidation occurred in background equal(revalidationRequests, 1, 'Background revalidation should have occurred') @@ -1867,7 +1868,7 @@ describe('Cache Interceptor', () => { } // Wait for staleness - await new Promise(resolve => setTimeout(resolve, 1100)) + await sleep(1100) // First stale request - gets stale content immediately { @@ -1876,7 +1877,7 @@ describe('Cache Interceptor', () => { } // Wait for background revalidation - await new Promise(resolve => setTimeout(resolve, 500)) + await sleep(500) equal(revalidationRequests, 1) // Second stale request - should get updated content from cache From 6402be6e7babebddf0aa6e2ee93ac16d81faafb2 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 31 Aug 2025 12:41:53 +0200 Subject: [PATCH 3/4] fix: use process.nextTick instead of setImmediate for background revalidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setImmediate doesn't work properly with fake timers in tests, causing background revalidation to not trigger. process.nextTick works correctly with both real time and fake timers. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Signed-off-by: Matteo Collina --- lib/interceptor/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index 6d788f2468f..6368af2188e 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -254,7 +254,7 @@ function handleResult ( sendCachedValue(handler, opts, result, age, null, true) // Start background revalidation (fire-and-forget) - setImmediate(() => { + process.nextTick(() => { let headers = { ...opts.headers, 'if-modified-since': new Date(result.cachedAt).toUTCString() From b9a011e8c6dc7f57c76dfbbb22beb611c0f78a7d Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 1 Sep 2025 08:54:43 +0200 Subject: [PATCH 4/4] Update cache.js Co-authored-by: Aras Abbasi --- lib/interceptor/cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index 6368af2188e..d561d4572cc 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -254,7 +254,7 @@ function handleResult ( sendCachedValue(handler, opts, result, age, null, true) // Start background revalidation (fire-and-forget) - process.nextTick(() => { + queueMicrotask(() => { let headers = { ...opts.headers, 'if-modified-since': new Date(result.cachedAt).toUTCString()