diff --git a/lib/cache/memory-cache-store.js b/lib/cache/memory-cache-store.js index 78c1c90439c..200d5813dbe 100644 --- a/lib/cache/memory-cache-store.js +++ b/lib/cache/memory-cache-store.js @@ -90,6 +90,7 @@ class MemoryCacheStore { headers: entry.headers, body: entry.body, etag: entry.etag, + cacheControlDirectives: entry.cacheControlDirectives, cachedAt: entry.cachedAt, staleAt: entry.staleAt, deleteAt: entry.deleteAt diff --git a/lib/cache/sqlite-cache-store.js b/lib/cache/sqlite-cache-store.js index c1ecbea265e..964b118ee3d 100644 --- a/lib/cache/sqlite-cache-store.js +++ b/lib/cache/sqlite-cache-store.js @@ -108,6 +108,7 @@ class SqliteCacheStore { statusCode INTEGER NOT NULL, statusMessage TEXT NOT NULL, headers TEXT NULL, + cacheControlDirectives TEXT NULL, etag TEXT NULL, vary TEXT NULL, cachedAt INTEGER NOT NULL, @@ -128,6 +129,7 @@ class SqliteCacheStore { statusMessage, headers, etag, + cacheControlDirectives, vary, cachedAt, staleAt @@ -147,6 +149,7 @@ class SqliteCacheStore { statusMessage = ?, headers = ?, etag = ?, + cacheControlDirectives = ?, cachedAt = ?, staleAt = ?, deleteAt = ? @@ -164,11 +167,12 @@ class SqliteCacheStore { statusMessage, headers, etag, + cacheControlDirectives, vary, cachedAt, staleAt, deleteAt - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `) this.#deleteByUrlQuery = this.#db.prepare( @@ -223,6 +227,9 @@ class SqliteCacheStore { statusMessage: value.statusMessage, headers: value.headers ? JSON.parse(value.headers) : undefined, etag: value.etag ? value.etag : undefined, + cacheControlDirectives: value.cacheControlDirectives + ? JSON.parse(value.cacheControlDirectives) + : undefined, cachedAt: value.cachedAt, staleAt: value.staleAt, deleteAt: value.deleteAt @@ -275,6 +282,7 @@ class SqliteCacheStore { value.statusMessage, value.headers ? JSON.stringify(value.headers) : null, value.etag ? value.etag : null, + value.cacheControlDirectives ? JSON.stringify(value.cacheControlDirectives) : null, value.cachedAt, value.staleAt, value.deleteAt, @@ -292,6 +300,7 @@ class SqliteCacheStore { value.statusMessage, value.headers ? JSON.stringify(value.headers) : null, value.etag ? value.etag : null, + value.cacheControlDirectives ? JSON.stringify(value.cacheControlDirectives) : null, value.vary ? JSON.stringify(value.vary) : null, value.cachedAt, value.staleAt, diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js index 7c3a7da0bb6..d0f00330143 100644 --- a/lib/handler/cache-handler.js +++ b/lib/handler/cache-handler.js @@ -113,6 +113,7 @@ class CacheHandler { statusMessage, headers: strippedHeaders, vary: varyDirectives, + cacheControlDirectives, cachedAt: now, staleAt, deleteAt @@ -170,7 +171,7 @@ class CacheHandler { * * @param {number} statusCode * @param {Record} headers - * @param {import('../util/cache.js').CacheControlDirectives} cacheControlDirectives + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives */ function canCacheResponse (statusCode, headers, cacheControlDirectives) { if (statusCode !== 200 && statusCode !== 307) { @@ -217,7 +218,7 @@ function canCacheResponse (statusCode, headers, cacheControlDirectives) { /** * @param {number} now * @param {Record} headers - * @param {import('../util/cache.js').CacheControlDirectives} cacheControlDirectives + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives * * @returns {number | undefined} time that the value is stale at or undefined if it shouldn't be cached */ @@ -253,21 +254,28 @@ function determineStaleAt (now, headers, cacheControlDirectives) { /** * @param {number} now - * @param {import('../util/cache.js').CacheControlDirectives} cacheControlDirectives + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives * @param {number} staleAt */ function determineDeleteAt (now, cacheControlDirectives, staleAt) { + let staleWhileRevalidate = -Infinity + let staleIfError = -Infinity + if (cacheControlDirectives['stale-while-revalidate']) { - return now + (cacheControlDirectives['stale-while-revalidate'] * 1000) + staleWhileRevalidate = now + (cacheControlDirectives['stale-while-revalidate'] * 1000) + } + + if (cacheControlDirectives['stale-if-error']) { + staleIfError = now + (cacheControlDirectives['stale-if-error'] * 1000) } - return staleAt + return Math.max(staleAt, staleWhileRevalidate, staleIfError) } /** * Strips headers required to be removed in cached responses * @param {Record} headers - * @param {import('../util/cache.js').CacheControlDirectives} cacheControlDirectives + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives * @returns {Record} */ function stripNecessaryHeaders (headers, cacheControlDirectives) { diff --git a/lib/handler/cache-revalidation-handler.js b/lib/handler/cache-revalidation-handler.js index 9e45268bfec..96a70683ea8 100644 --- a/lib/handler/cache-revalidation-handler.js +++ b/lib/handler/cache-revalidation-handler.js @@ -17,10 +17,12 @@ const assert = require('node:assert') */ class CacheRevalidationHandler { #successful = false + /** * @type {((boolean, any) => void) | null} */ #callback + /** * @type {(import('../../types/dispatcher.d.ts').default.DispatchHandler)} */ @@ -29,19 +31,26 @@ class CacheRevalidationHandler { #context /** - * @param {(boolean, any) => void} callback Function to call if the cached value is valid - * @param {import('../../types/dispatcher.d.ts').default.DispatchHandler} handler + * @type {boolean} + */ + #allowErrorStatusCodes + + /** + * @param {(boolean) => void} callback Function to call if the cached value is valid + * @param {import('../../types/dispatcher.d.ts').default.DispatchHandlers} handler + * @param {boolean} allowErrorStatusCodes */ - constructor (callback, handler) { + constructor (callback, handler, allowErrorStatusCodes) { if (typeof callback !== 'function') { throw new TypeError('callback must be a function') } this.#callback = callback this.#handler = handler + this.#allowErrorStatusCodes = allowErrorStatusCodes } - onRequestStart (controller, context) { + onRequestStart (_, context) { this.#successful = false this.#context = context } @@ -59,7 +68,9 @@ class CacheRevalidationHandler { assert(this.#callback != null) // https://www.rfc-editor.org/rfc/rfc9111.html#name-handling-a-validation-respo - this.#successful = statusCode === 304 + // https://datatracker.ietf.org/doc/html/rfc5861#section-4 + this.#successful = statusCode === 304 || + (this.#allowErrorStatusCodes && statusCode >= 500 && statusCode <= 504) this.#callback(this.#successful, this.#context) this.#callback = null diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index 8da8468b04b..733df30e17e 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -1,3 +1,4 @@ +// @ts-check 'use strict' const assert = require('node:assert') @@ -10,45 +11,15 @@ const { assertCacheStore, assertCacheMethods, makeCacheKey, parseCacheControlHea const { AbortError } = require('../core/errors.js') /** - * @param {import('../../types/dispatcher.d.ts').default.DispatchHandler} handler + * @typedef {(options: import('../../types/dispatcher.d.ts').default.DispatchOptions, handler: import('../../types/dispatcher.d.ts').default.DispatchHandler) => void} DispatchFn */ -function sendGatewayTimeout (handler) { - let aborted = false - try { - if (typeof handler.onConnect === 'function') { - handler.onConnect(() => { - aborted = true - }) - - if (aborted) { - return - } - } - - if (typeof handler.onHeaders === 'function') { - handler.onHeaders(504, [], () => {}, 'Gateway Timeout') - if (aborted) { - return - } - } - - if (typeof handler.onComplete === 'function') { - handler.onComplete([]) - } - } catch (err) { - if (typeof handler.onError === 'function') { - handler.onError(err) - } - } -} /** * @param {import('../../types/cache-interceptor.d.ts').default.GetResult} result - * @param {number} age - * @param {import('../util/cache.js').CacheControlDirectives | undefined} cacheControlDirectives + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives | undefined} cacheControlDirectives * @returns {boolean} */ -function needsRevalidation (result, age, cacheControlDirectives) { +function needsRevalidation (result, cacheControlDirectives) { if (cacheControlDirectives?.['no-cache']) { // Always revalidate requests with the no-cache directive return true @@ -81,6 +52,214 @@ function needsRevalidation (result, age, cacheControlDirectives) { return false } +/** + * @param {DispatchFn} dispatch + * @param {import('../../types/cache-interceptor.d.ts').default.CacheHandlerOptions} globalOpts + * @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} cacheKey + * @param {import('../../types/dispatcher.d.ts').default.DispatchHandler} handler + * @param {import('../../types/dispatcher.d.ts').default.RequestOptions} opts + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives | undefined} reqCacheControl + */ +function handleUncachedResponse ( + dispatch, + globalOpts, + cacheKey, + handler, + opts, + reqCacheControl +) { + if (reqCacheControl?.['only-if-cached']) { + let aborted = false + try { + if (typeof handler.onConnect === 'function') { + handler.onConnect(() => { + aborted = true + }) + + if (aborted) { + return + } + } + + if (typeof handler.onHeaders === 'function') { + handler.onHeaders(504, [], () => {}, 'Gateway Timeout') + if (aborted) { + return + } + } + + if (typeof handler.onComplete === 'function') { + handler.onComplete([]) + } + } catch (err) { + if (typeof handler.onError === 'function') { + handler.onError(err) + } + } + + return true + } + + return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) +} + +/** + * @param {import('../../types/cache-interceptor.d.ts').default.GetResult} result + * @param {number} age + */ +function sendCachedValue (handler, opts, result, age, context) { + // TODO (perf): Readable.from path can be optimized... + const stream = util.isStream(result.body) + ? result.body + : Readable.from(result.body ?? []) + + assert(!stream.destroyed, 'stream should not be destroyed') + assert(!stream.readableDidRead, 'stream should not be readableDidRead') + + const controller = { + resume () { + stream.resume() + }, + pause () { + stream.pause() + }, + get paused () { + return stream.isPaused() + }, + get aborted () { + return stream.destroyed + }, + get reason () { + return stream.errored + }, + abort (reason) { + stream.destroy(reason ?? new AbortError()) + } + } + + stream + .on('error', function (err) { + if (!this.readableEnded) { + if (typeof handler.onResponseError === 'function') { + handler.onResponseError(controller, err) + } else { + throw err + } + } + }) + .on('close', function () { + if (!this.errored) { + handler.onResponseEnd?.(controller, {}) + } + }) + + handler.onRequestStart?.(controller, context) + + if (stream.destroyed) { + return + } + + // Add the age header + // https://www.rfc-editor.org/rfc/rfc9111.html#name-age + // TODO (fix): What if headers.age already exists? + const headers = age != null ? { ...result.headers, age: String(age) } : result.headers + + handler.onResponseStart?.(controller, result.statusCode, result.statusMessage, headers) + + if (opts.method === 'HEAD') { + stream.destroy() + } else { + stream.on('data', function (chunk) { + handler.onResponseData?.(controller, chunk) + }) + } +} + +/** + * @param {DispatchFn} dispatch + * @param {import('../../types/cache-interceptor.d.ts').default.CacheHandlerOptions} globalOpts + * @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} cacheKey + * @param {import('../../types/dispatcher.d.ts').default.DispatchHandler} handler + * @param {import('../../types/dispatcher.d.ts').default.RequestOptions} opts + * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives | undefined} reqCacheControl + * @param {import('../../types/cache-interceptor.d.ts').default.GetResult | undefined} result + */ +function handleResult ( + dispatch, + globalOpts, + cacheKey, + handler, + opts, + reqCacheControl, + result +) { + if (!result) { + return handleUncachedResponse(dispatch, globalOpts, cacheKey, handler, opts, reqCacheControl) + } + + if (!result.body && opts.method !== 'HEAD') { + throw new Error('body is undefined but method isn\'t HEAD') + } + + const now = Date.now() + if (now > result.deleteAt) { + // Response is expired, cache store shouldn't have given this to us + return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) + } + + const age = Math.round((now - result.cachedAt) / 1000) + if (reqCacheControl?.['max-age'] && age >= reqCacheControl['max-age']) { + // Response is considered expired for this specific request + // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.1 + return dispatch(opts, handler) + } + + // Check if the response is stale + if (needsRevalidation(result, reqCacheControl)) { + if (util.isStream(opts.body) && util.bodyLength(opts.body) !== 0) { + // If body is is stream we can't revalidate... + // TODO (fix): This could be less strict... + return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) + } + + let withinStaleIfErrorThreshold = false + const staleIfErrorExpiry = result.cacheControlDirectives['stale-if-error'] ?? reqCacheControl?.['stale-if-error'] + if (staleIfErrorExpiry) { + withinStaleIfErrorThreshold = now < (result.staleAt + (staleIfErrorExpiry * 1000)) + } + + // We need to revalidate the response + return dispatch( + { + ...opts, + headers: { + ...opts.headers, + 'if-modified-since': new Date(result.cachedAt).toUTCString(), + 'if-none-match': result.etag + } + }, + new CacheRevalidationHandler( + (success, context) => { + if (success) { + sendCachedValue(handler, opts, result, age, context) + } else if (util.isStream(result.body)) { + result.body.on('error', () => {}).destroy() + } + }, + new CacheHandler(globalOpts, cacheKey, handler), + withinStaleIfErrorThreshold + ) + ) + } + + // Dump request body. + if (util.isStream(opts.body)) { + opts.body.on('error', () => {}).destroy() + } + + sendCachedValue(handler, opts, result, age, null) +} + /** * @param {import('../../types/cache-interceptor.d.ts').default.CacheOptions} [opts] * @returns {import('../../types/dispatcher.d.ts').default.DispatcherComposeInterceptor} @@ -107,19 +286,16 @@ module.exports = (opts = {}) => { return dispatch => { return (opts, handler) => { - // TODO (fix): What if e.g. opts.headers has if-modified-since header? Or other headers - // that make things ambigious? - if (!opts.origin || safeMethodsToNotCache.includes(opts.method)) { // Not a method we want to cache or we don't have the origin, skip return dispatch(opts, handler) } - const requestCacheControl = opts.headers?.['cache-control'] + const reqCacheControl = opts.headers?.['cache-control'] ? parseCacheControlHeader(opts.headers['cache-control']) : undefined - if (requestCacheControl?.['no-store']) { + if (reqCacheControl?.['no-store']) { return dispatch(opts, handler) } @@ -127,172 +303,29 @@ module.exports = (opts = {}) => { * @type {import('../../types/cache-interceptor.d.ts').default.CacheKey} */ const cacheKey = makeCacheKey(opts) - - // TODO (perf): For small entries support returning a Buffer instead of a stream. - // Maybe store should return { staleAt, headers, body, etc... } instead of a stream + stream.value? - // Where body can be a Buffer, string, stream or blob? const result = store.get(cacheKey) - if (!result) { - if (requestCacheControl?.['only-if-cached']) { - // We only want cached responses - // https://www.rfc-editor.org/rfc/rfc9111.html#name-only-if-cached - sendGatewayTimeout(handler) - return true - } - - return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) - } - /** - * @param {import('../../types/cache-interceptor.d.ts').default.GetResult} result - * @param {number} age - */ - const respondWithCachedValue = ({ headers, statusCode, statusMessage, body }, age, context) => { - const stream = util.isStream(body) - ? body - : Readable.from(body ?? []) - - assert(!stream.destroyed, 'stream should not be destroyed') - assert(!stream.readableDidRead, 'stream should not be readableDidRead') - - const controller = { - resume () { - stream.resume() - }, - pause () { - stream.pause() - }, - get paused () { - return stream.isPaused() - }, - get aborted () { - return stream.destroyed - }, - get reason () { - return stream.errored - }, - abort (reason) { - stream.destroy(reason ?? new AbortError()) - } - } - - stream - .on('error', function (err) { - if (!this.readableEnded) { - if (typeof handler.onResponseError === 'function') { - handler.onResponseError(controller, err) - } else { - throw err - } - } - }) - .on('close', function () { - if (!this.errored) { - handler.onResponseEnd?.(controller, {}) - } - }) - - handler.onRequestStart?.(controller, context) - - if (stream.destroyed) { - return - } - - // Add the age header - // https://www.rfc-editor.org/rfc/rfc9111.html#name-age - // TODO (fix): What if headers.age already exists? - headers = age != null ? { ...headers, age: String(age) } : headers - - handler.onResponseStart?.(controller, statusCode, statusMessage, headers) - - if (opts.method === 'HEAD') { - stream.destroy() - } else { - stream.on('data', function (chunk) { - handler.onResponseData?.(controller, chunk) - }) - } - } - - /** - * @param {import('../../types/cache-interceptor.d.ts').default.GetResult} result - */ - const handleResult = (result) => { - // TODO (perf): Readable.from path can be optimized... - - if (!result.body && opts.method !== 'HEAD') { - throw new Error('stream is undefined but method isn\'t HEAD') - } - - const age = Math.round((Date.now() - result.cachedAt) / 1000) - if (requestCacheControl?.['max-age'] && age >= requestCacheControl['max-age']) { - // Response is considered expired for this specific request - // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.2.1.1 - return dispatch(opts, handler) - } - - // Check if the response is stale - if (needsRevalidation(result, age, requestCacheControl)) { - if (util.isStream(opts.body) && util.bodyLength(opts.body) !== 0) { - // If body is is stream we can't revalidate... - // TODO (fix): This could be less strict... - return dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) - } - - // We need to revalidate the response - return dispatch( - { - ...opts, - headers: { - ...opts.headers, - 'if-modified-since': new Date(result.cachedAt).toUTCString(), - etag: result.etag - } - }, - new CacheRevalidationHandler( - (success, context) => { - if (success) { - respondWithCachedValue(result, age, context) - } else if (util.isStream(result.body)) { - result.body.on('error', () => {}).destroy() - } - }, - new CacheHandler(globalOpts, cacheKey, handler) - ) + if (result && typeof result.then === 'function') { + result.then(result => { + handleResult(dispatch, + globalOpts, + cacheKey, + handler, + opts, + reqCacheControl, + result ) - } - - // Dump request body. - if (util.isStream(opts.body)) { - opts.body.on('error', () => {}).destroy() - } - - respondWithCachedValue(result, age, null) - } - - if (typeof result.then === 'function') { - result.then((result) => { - if (!result) { - if (requestCacheControl?.['only-if-cached']) { - // We only want cached responses - // https://www.rfc-editor.org/rfc/rfc9111.html#name-only-if-cached - sendGatewayTimeout(handler) - return true - } - - dispatch(opts, new CacheHandler(globalOpts, cacheKey, handler)) - } else { - handleResult(result) - } - }, err => { - if (typeof handler.onError === 'function') { - handler.onError(err) - } else { - throw err - } }) } else { - handleResult(result) + handleResult( + dispatch, + globalOpts, + cacheKey, + handler, + opts, + reqCacheControl, + result + ) } return true diff --git a/lib/util/cache.js b/lib/util/cache.js index 97dda5a8058..4e1635a911c 100644 --- a/lib/util/cache.js +++ b/lib/util/cache.js @@ -96,32 +96,13 @@ function assertCacheValue (value) { /** * @see https://www.rfc-editor.org/rfc/rfc9111.html#name-cache-control * @see https://www.iana.org/assignments/http-cache-directives/http-cache-directives.xhtml - * - * @typedef {{ - * 'max-stale'?: number; - * 'min-fresh'?: number; - * 'max-age'?: number; - * 's-maxage'?: number; - * 'stale-while-revalidate'?: number; - * 'stale-if-error'?: number; - * public?: true; - * private?: true | string[]; - * 'no-store'?: true; - * 'no-cache'?: true | string[]; - * 'must-revalidate'?: true; - * 'proxy-revalidate'?: true; - * immutable?: true; - * 'no-transform'?: true; - * 'must-understand'?: true; - * 'only-if-cached'?: true; - * }} CacheControlDirectives - * + * @param {string | string[]} header - * @returns {CacheControlDirectives} + * @returns {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} */ function parseCacheControlHeader (header) { /** - * @type {import('../util/cache.js').CacheControlDirectives} + * @type {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} */ const output = {} diff --git a/test/cache-interceptor/cache-store-test-utils.js b/test/cache-interceptor/cache-store-test-utils.js index e04d899f575..6ac6402980b 100644 --- a/test/cache-interceptor/cache-store-test-utils.js +++ b/test/cache-interceptor/cache-store-test-utils.js @@ -31,6 +31,7 @@ function cacheStoreTests (CacheStore) { statusCode: 200, statusMessage: '', headers: { foo: 'bar' }, + cacheControlDirectives: {}, cachedAt: Date.now(), staleAt: Date.now() + 10000, deleteAt: Date.now() + 20000 @@ -57,6 +58,7 @@ function cacheStoreTests (CacheStore) { deepStrictEqual(await readResponse(readResult), { ...requestValue, etag: undefined, + cacheControlDirectives: {}, body: requestBody }) @@ -71,6 +73,7 @@ function cacheStoreTests (CacheStore) { statusCode: 200, statusMessage: '', headers: { foo: 'bar' }, + cacheControlDirectives: {}, cachedAt: Date.now(), staleAt: Date.now() + 10000, deleteAt: Date.now() + 20000 @@ -94,6 +97,7 @@ function cacheStoreTests (CacheStore) { deepStrictEqual(await readResponse(readResult), { ...anotherValue, etag: undefined, + cacheControlDirectives: {}, body: anotherBody }) }) @@ -109,6 +113,7 @@ function cacheStoreTests (CacheStore) { statusCode: 200, statusMessage: '', headers: { foo: 'bar' }, + cacheControlDirectives: {}, cachedAt: Date.now() - 10000, staleAt: Date.now() - 1, deleteAt: Date.now() + 20000 @@ -128,6 +133,7 @@ function cacheStoreTests (CacheStore) { deepStrictEqual(await readResponse(readResult), { ...requestValue, etag: undefined, + cacheControlDirectives: {}, body: requestBody }) }) @@ -177,6 +183,7 @@ function cacheStoreTests (CacheStore) { vary: { 'some-header': 'hello world' }, + cacheControlDirectives: {}, cachedAt: Date.now(), staleAt: Date.now() + 10000, deleteAt: Date.now() + 20000 @@ -201,6 +208,7 @@ function cacheStoreTests (CacheStore) { deepStrictEqual(await readResponse(readStream), { ...responseValue, etag: undefined, + cacheControlDirectives: {}, body: requestBody }) diff --git a/test/cache-interceptor/sqlite-cache-store-tests.js b/test/cache-interceptor/sqlite-cache-store-tests.js index 9a420501842..edbd62f72fe 100644 --- a/test/cache-interceptor/sqlite-cache-store-tests.js +++ b/test/cache-interceptor/sqlite-cache-store-tests.js @@ -69,6 +69,7 @@ test('SqliteCacheStore works nicely with multiple stores', async (t) => { deepStrictEqual(await readResponse(readable), { ...requestValue, etag: undefined, + cacheControlDirectives: undefined, body: requestBody }) @@ -78,6 +79,7 @@ test('SqliteCacheStore works nicely with multiple stores', async (t) => { deepStrictEqual(await readResponse(readable), { ...requestValue, etag: undefined, + cacheControlDirectives: undefined, body: requestBody }) }) diff --git a/test/interceptors/cache.js b/test/interceptors/cache.js index 05432a303e9..6da582d707d 100644 --- a/test/interceptors/cache.js +++ b/test/interceptors/cache.js @@ -6,7 +6,6 @@ const { createServer } = require('node:http') const { once } = require('node:events') const FakeTimers = require('@sinonjs/fake-timers') const { Client, interceptors, cacheStores } = require('../../index') -const { tick } = require('../../lib/util/timers') describe('Cache Interceptor', () => { test('doesn\'t cache request w/ no cache-control header', async () => { @@ -227,14 +226,13 @@ describe('Cache Interceptor', () => { const clock = FakeTimers.install({ shouldClearNativeTimers: true }) - tick(0) const server = createServer((req, res) => { res.setHeader('cache-control', 'public, s-maxage=1, stale-while-revalidate=10') requestsToOrigin++ if (requestsToOrigin > 1) { - equal(req.headers['etag'], '"asd123"') + equal(req.headers['if-none-match'], '"asd123"') if (requestsToOrigin === 3) { res.end('asd123') @@ -273,7 +271,6 @@ describe('Cache Interceptor', () => { strictEqual(await response.body.text(), 'asd') clock.tick(1500) - tick(1500) // Now we send two more requests. Both of these should reach the origin, // but now with a conditional header asking if the resource has been @@ -435,12 +432,194 @@ describe('Cache Interceptor', () => { } }) + for (const maxAgeHeader of ['s-maxage', 'max-age']) { + test(`stale-while-revalidate w/ ${maxAgeHeader}`, async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + let requestsToOrigin = 0 + let revalidationRequests = 0 + const server = createServer((req, res) => { + if (req.headers['if-none-match']) { + revalidationRequests++ + if (req.headers['if-none-match'] !== '"asd"') { + fail(`etag mismatch: ${req.headers['if-none-match']}`) + } + + res.statusCode = 304 + res.end() + } else { + requestsToOrigin++ + res.setHeader('cache-control', 'public, max-age=1, stale-while-revalidate=4') + res.setHeader('etag', '"asd"') + res.end('asd') + } + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + clock.uninstall() + server.close() + await client.close() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + strictEqual(revalidationRequests, 0) + + // Send first request, this will hit the origin + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + strictEqual(revalidationRequests, 0) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + // Send second request, this will be cached. + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + strictEqual(revalidationRequests, 0) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + clock.tick(1500) + + // Send third request, this should be revalidated + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + strictEqual(revalidationRequests, 1) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + clock.tick(5000) + + // Send fourth request, this should be a new request entirely + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 2) + strictEqual(revalidationRequests, 1) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + }) + } + + test('stale-if-error from response works as expected', async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + let requestsToOrigin = 0 + const server = createServer((_, res) => { + requestsToOrigin++ + if (requestsToOrigin === 1) { + // First request + res.setHeader('cache-control', 'public, s-maxage=10, stale-if-error=20') + res.end('asd') + } else { + res.statusCode = 500 + res.end('') + } + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + clock.uninstall() + server.close() + await client.close() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + // Send first request. This will hit the origin and succeed + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + // Send second request. It isn't stale yet, so this should be from the + // cache and succeed + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + clock.tick(15000) + + // Send third request. This is now stale, the revalidation request should + // fail but the response should still be served from cache. + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 2) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + clock.tick(25000) + + // Send fourth request. We're now outside the stale-if-error threshold and + // should see the error. + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 3) + equal(response.statusCode, 500) + } + }) + describe('Client-side directives', () => { test('max-age', async () => { const clock = FakeTimers.install({ shouldClearNativeTimers: true }) - tick(0) let requestsToOrigin = 0 const server = createServer((_, res) => { @@ -494,7 +673,6 @@ describe('Cache Interceptor', () => { strictEqual(await response.body.text(), 'asd') clock.tick(6000) - tick(6000) // Send fourth request w/ the directive, age should be 6 now so this // should hit the origin @@ -516,7 +694,6 @@ describe('Cache Interceptor', () => { const clock = FakeTimers.install({ shouldClearNativeTimers: true }) - tick(0) const server = createServer((req, res) => { res.setHeader('cache-control', 'public, s-maxage=1, stale-while-revalidate=10') @@ -558,7 +735,6 @@ describe('Cache Interceptor', () => { strictEqual(await response.body.text(), 'asd') clock.tick(1500) - tick(1500) // Now we send a second request. This should be within the max stale // threshold, so a request shouldn't be made to the origin @@ -589,7 +765,6 @@ describe('Cache Interceptor', () => { const clock = FakeTimers.install({ shouldClearNativeTimers: true }) - tick(0) const server = createServer((req, res) => { requestsToOrigin++ @@ -623,7 +798,6 @@ describe('Cache Interceptor', () => { // Fast forward more. Response has 8sec TTL left after clock.tick(2000) - tick(2000) // Now we send a second request. This should be within the threshold, so // a request shouldn't be made to the origin @@ -638,7 +812,6 @@ describe('Cache Interceptor', () => { // Fast forward more. Response has 2sec TTL left after clock.tick(6000) - tick(6000) // Send the second request again, this time it shouldn't be within the // threshold and a request should be made to the origin. @@ -807,5 +980,109 @@ describe('Cache Interceptor', () => { }) equal(response.statusCode, 504) }) + + test('stale-if-error', async () => { + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + + let requestsToOrigin = 0 + const server = createServer((_, res) => { + requestsToOrigin++ + if (requestsToOrigin === 1) { + // First request, send stale-while-revalidate to keep the value in the cache + res.setHeader('cache-control', 'public, s-maxage=10, stale-while-revalidate=20') + res.end('asd') + } else { + res.statusCode = 500 + res.end('') + } + }).listen(0) + + const client = new Client(`http://localhost:${server.address().port}`) + .compose(interceptors.cache()) + + after(async () => { + clock.uninstall() + server.close() + await client.close() + }) + + await once(server, 'listening') + + strictEqual(requestsToOrigin, 0) + + // Send first request. This will hit the origin and succeed + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + // Send second request. It isn't stale yet, so this should be from the + // cache and succeed + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 1) + equal(response.statusCode, 200) + equal(await response.body.text(), 'asd') + } + + clock.tick(15000) + + // Send third request. This is now stale, the revalidation request should + // fail but the response should still be served from cache. + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET', + headers: { + '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. + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET' + }) + equal(requestsToOrigin, 3) + equal(response.statusCode, 500) + } + + clock.tick(25000) + + // Send fifth request. We're now outside the stale-if-error threshold and + // should see the error. + { + const response = await client.request({ + origin: 'localhost', + path: '/', + method: 'GET', + headers: { + 'cache-control': 'stale-if-error=20' + } + }) + equal(requestsToOrigin, 4) + equal(response.statusCode, 500) + } + }) }) }) diff --git a/test/types/cache-interceptor.test-d.ts b/test/types/cache-interceptor.test-d.ts index b219a528750..6fb66955de5 100644 --- a/test/types/cache-interceptor.test-d.ts +++ b/test/types/cache-interceptor.test-d.ts @@ -25,6 +25,7 @@ expectAssignable({ statusCode: 200, statusMessage: 'OK', headers: {}, + cacheControlDirectives: {}, cachedAt: 0, staleAt: 0, deleteAt: 0 @@ -34,12 +35,37 @@ expectAssignable({ statusCode: 200, statusMessage: 'OK', headers: {}, - vary: {}, + vary: { + foo: 'bar' + }, + cacheControlDirectives: { + 'max-stale': 0, + 'min-fresh': 0, + 'max-age': 0, + 's-maxage': 0, + 'stale-while-revalidate': 0, + 'stale-if-error': 0, + public: true, + private: true, + 'no-store': true, + 'no-cache': true, + 'must-revalidate': true, + 'proxy-revalidate': true, + immutable: true, + 'no-transform': true, + 'must-understand': true, + 'only-if-cached': true + }, cachedAt: 0, staleAt: 0, deleteAt: 0 }) +expectAssignable({ + private: [''], + 'no-cache': [''] +}) + expectNotAssignable({}) expectNotAssignable({ statusCode: '123', diff --git a/types/cache-interceptor.d.ts b/types/cache-interceptor.d.ts index 9302a5a7d2a..05c767db283 100644 --- a/types/cache-interceptor.d.ts +++ b/types/cache-interceptor.d.ts @@ -22,6 +22,25 @@ declare namespace CacheHandler { methods?: CacheMethods[] } + export interface CacheControlDirectives { + 'max-stale'?: number; + 'min-fresh'?: number; + 'max-age'?: number; + 's-maxage'?: number; + 'stale-while-revalidate'?: number; + 'stale-if-error'?: number; + public?: true; + private?: true | string[]; + 'no-store'?: true; + 'no-cache'?: true | string[]; + 'must-revalidate'?: true; + 'proxy-revalidate'?: true; + immutable?: true; + 'no-transform'?: true; + 'must-understand'?: true; + 'only-if-cached'?: true; + } + export interface CacheKey { origin: string method: string @@ -35,6 +54,7 @@ declare namespace CacheHandler { headers: Record vary?: Record etag?: string + cacheControlDirectives?: CacheControlDirectives cachedAt: number staleAt: number deleteAt: number @@ -52,6 +72,7 @@ declare namespace CacheHandler { headers: Record etag?: string body: null | Readable | Iterable | AsyncIterable | Buffer | Iterable | AsyncIterable | string + cacheControlDirectives: CacheControlDirectives, cachedAt: number staleAt: number deleteAt: number