diff --git a/lib/handler/cache-revalidation-handler.js b/lib/handler/cache-revalidation-handler.js index 14560baf859..729cb57d03d 100644 --- a/lib/handler/cache-revalidation-handler.js +++ b/lib/handler/cache-revalidation-handler.js @@ -1,5 +1,6 @@ 'use strict' +const assert = require('node:assert') const DecoratorHandler = require('../handler/decorator-handler') /** @@ -19,7 +20,7 @@ const DecoratorHandler = require('../handler/decorator-handler') class CacheRevalidationHandler extends DecoratorHandler { #successful = false /** - * @type {((boolean) => void)} + * @type {((boolean) => void) | null} */ #callback /** @@ -27,6 +28,7 @@ class CacheRevalidationHandler extends DecoratorHandler { */ #handler + #abort /** * @param {(boolean) => void} callback Function to call if the cached value is valid * @param {import('../../types/dispatcher.d.ts').default.DispatchHandlers} handler @@ -44,11 +46,7 @@ class CacheRevalidationHandler extends DecoratorHandler { onConnect (abort) { this.#successful = false - - // TODO (fix): This is suspect... - if (typeof this.#handler.onConnect === 'function') { - this.#handler.onConnect(abort) - } + this.#abort = abort } /** @@ -66,12 +64,21 @@ class CacheRevalidationHandler extends DecoratorHandler { resume, statusMessage ) { + assert(this.#callback != null) + // https://www.rfc-editor.org/rfc/rfc9111.html#name-handling-a-validation-respo - if (statusCode === 304) { - this.#successful = true + this.#successful = statusCode === 304 + this.#callback(this.#successful) + this.#callback = null + + if (this.#successful) { return true } + if (typeof this.#handler.onConnect === 'function') { + this.#handler.onConnect(this.#abort) + } + if (typeof this.#handler.onHeaders === 'function') { return this.#handler.onHeaders( statusCode, @@ -108,9 +115,11 @@ class CacheRevalidationHandler extends DecoratorHandler { * @param {string[] | null} rawTrailers */ onComplete (rawTrailers) { - this.#callback(this.#successful) + if (this.#successful) { + return + } - if (!this.#successful && typeof this.#handler.onComplete === 'function') { + if (typeof this.#handler.onComplete === 'function') { this.#handler.onComplete(rawTrailers) } } @@ -121,10 +130,19 @@ class CacheRevalidationHandler extends DecoratorHandler { * @param {Error} err */ onError (err) { - this.#callback(false) + if (this.#successful) { + return + } + + if (this.#callback) { + this.#callback(false) + this.#callback = null + } if (typeof this.#handler.onError === 'function') { this.#handler.onError(err) + } else { + throw err } } } diff --git a/lib/interceptor/cache.js b/lib/interceptor/cache.js index b56c32c5545..a16793b5c8b 100644 --- a/lib/interceptor/cache.js +++ b/lib/interceptor/cache.js @@ -39,6 +39,9 @@ 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) @@ -66,7 +69,9 @@ module.exports = (opts = {}) => { if (typeof handler.onError === 'function') { handler.onError(err) } else { - throw err + process.nextTick(() => { + throw err + }) } } }) @@ -122,47 +127,39 @@ module.exports = (opts = {}) => { return dispatch(opts, new CacheHandler(globalOpts, opts, handler)) } - try { - // Check if the response is stale - const now = Date.now() - if (now < value.staleAt) { - // Dump body. - if (util.isStream(opts.body)) { - opts.body.on('error', () => {}).resume() - } - respondWithCachedValue(stream, value) - } else 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... - dispatch(opts, new CacheHandler(globalOpts, opts, handler)) - } else { - // Need to revalidate the response - dispatch( - { - ...opts, - headers: { - ...opts.headers, - 'if-modified-since': new Date(value.cachedAt).toUTCString() + // Check if the response is stale + const now = Date.now() + if (now < value.staleAt) { + // Dump request body. + if (util.isStream(opts.body)) { + opts.body.on('error', () => {}).destroy() + } + respondWithCachedValue(stream, value) + } else 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... + dispatch(opts, new CacheHandler(globalOpts, opts, handler)) + } else { + // Need to revalidate the response + dispatch( + { + ...opts, + headers: { + ...opts.headers, + 'if-modified-since': new Date(value.cachedAt).toUTCString() + } + }, + new CacheRevalidationHandler( + (success) => { + if (success) { + respondWithCachedValue(stream, value) + } else { + stream.on('error', () => {}).destroy() } }, - new CacheRevalidationHandler( - (success) => { - if (success) { - respondWithCachedValue(stream, value) - } else { - stream.on('error', () => {}).destroy() - } - }, - new CacheHandler(globalOpts, opts, handler) - ) + new CacheHandler(globalOpts, opts, handler) ) - } - } catch (err) { - if (typeof handler.onError === 'function') { - handler.onError(err) - } else { - throw err - } + ) } }