cache: fix stale-while-revalidate and stale-if-error

Closes nodejs#3853 Signed-off-by: flakey5 <[email protected]>
flakey5 · Nov 24, 2024 · 45c0793 · 45c0793
1 parent 43bfeb4
commit 45c0793
Show file tree

Hide file tree

Showing 9 changed files with 571 additions and 233 deletions.
diff --git a/lib/cache/memory-cache-store.js b/lib/cache/memory-cache-store.js
@@ -90,6 +90,7 @@ class MemoryCacheStore {
           rawHeaders: entry.rawHeaders,
           body: entry.body,
           etag: entry.etag,
+          cacheControlDirectives: entry.cacheControlDirectives,
           cachedAt: entry.cachedAt,
           staleAt: entry.staleAt,
           deleteAt: entry.deleteAt

diff --git a/lib/handler/cache-handler.js b/lib/handler/cache-handler.js
@@ -138,6 +138,7 @@ class CacheHandler extends DecoratorHandler {
         statusMessage,
         rawHeaders: strippedHeaders,
         vary: varyDirectives,
+        cacheControlDirectives,
         cachedAt: now,
         staleAt,
         deleteAt
@@ -227,7 +228,7 @@ class CacheHandler extends DecoratorHandler {
  *
  * @param {number} statusCode
  * @param {Record<string, string | string[]>} 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) {
@@ -274,7 +275,7 @@ function canCacheResponse (statusCode, headers, cacheControlDirectives) {
 /**
  * @param {number} now
  * @param {Record<string, string | string[]>} 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
  */
@@ -310,22 +311,29 @@ 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 {Buffer[]} rawHeaders
  * @param {string[]} parsedRawHeaders
- * @param {import('../util/cache.js').CacheControlDirectives} cacheControlDirectives
+ * @param {import('../../types/cache-interceptor.d.ts').default.CacheControlDirectives} cacheControlDirectives
  * @returns {Buffer[]}
  */
 function stripNecessaryHeaders (rawHeaders, parsedRawHeaders, cacheControlDirectives) {

diff --git a/lib/handler/cache-revalidation-handler.js b/lib/handler/cache-revalidation-handler.js
@@ -19,22 +19,30 @@ const DecoratorHandler = require('../handler/decorator-handler')
  */
 class CacheRevalidationHandler extends DecoratorHandler {
   #successful = false
+
   /**
    * @type {((boolean) => void) | null}
    */
   #callback
+
   /**
    * @type {(import('../../types/dispatcher.d.ts').default.DispatchHandlers)}
    */
   #handler
 
+  /**
+   * @type {boolean}
+   */
+  #allowErrorStatusCodes
+
   #abort
 
   /**
    * @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')
     }
@@ -43,6 +51,7 @@ class CacheRevalidationHandler extends DecoratorHandler {
 
     this.#callback = callback
     this.#handler = handler
+    this.#allowErrorStatusCodes = allowErrorStatusCodes
   }
 
   onConnect (abort) {
@@ -68,7 +77,9 @@ class CacheRevalidationHandler extends DecoratorHandler {
     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.#callback = null