Revert nowAbsolute, add regression test (#3850)

Signed-off-by: Matteo Collina <[email protected]>

Author: mcollina

benchmarks/timers/now-absolute.mjs

@@ -1,7 +1,6 @@
 'use strict'
 
 const { Writable } = require('node:stream')
-const { nowAbsolute } = require('../util/timers.js')
 
 /**
  * @typedef {import('../../types/cache-interceptor.d.ts').default.CacheKey} CacheKey
@@ -77,7 +76,7 @@ class MemoryCacheStore {
 
     const topLevelKey = `${key.origin}:${key.path}`
 
-    const now = nowAbsolute()
+    const now = Date.now()
     const entry = this.#entries.get(topLevelKey)?.find((entry) => (
       entry.deleteAt > now &&
       entry.method === key.method &&

lib/cache/memory-cache-store.js

@@ -7,7 +7,6 @@ const {
   parseVaryHeader,
   isEtagUsable
 } = require('../util/cache')
-const { nowAbsolute } = require('../util/timers.js')
 
 function noop () {}
 
@@ -123,7 +122,7 @@ class CacheHandler extends DecoratorHandler {
       return downstreamOnHeaders()
     }
 
-    const now = nowAbsolute()
+    const now = Date.now()
     const staleAt = determineStaleAt(now, headers, cacheControlDirectives)
     if (staleAt) {
       const varyDirectives = this.#cacheKey.headers && headers.vary
@@ -311,7 +310,7 @@ function determineStaleAt (now, headers, cacheControlDirectives) {
     // https://www.rfc-editor.org/rfc/rfc9111.html#section-5.3
     const expiresDate = new Date(headers.expire)
     if (expiresDate instanceof Date && !isNaN(expiresDate)) {
-      return now + (nowAbsolute() - expiresDate.getTime())
+      return now + (Date.now() - expiresDate.getTime())
     }
   }
 

lib/handler/cache-handler.js

@@ -7,7 +7,6 @@ const CacheHandler = require('../handler/cache-handler')
 const MemoryCacheStore = require('../cache/memory-cache-store')
 const CacheRevalidationHandler = require('../handler/cache-revalidation-handler')
 const { assertCacheStore, assertCacheMethods, makeCacheKey, parseCacheControlHeader } = require('../util/cache.js')
-const { nowAbsolute } = require('../util/timers.js')
 
 const AGE_HEADER = Buffer.from('age')
 
@@ -56,7 +55,7 @@ function needsRevalidation (result, age, cacheControlDirectives) {
     return true
   }
 
-  const now = nowAbsolute()
+  const now = Date.now()
   if (now > result.staleAt) {
     // Response is stale
     if (cacheControlDirectives?.['max-stale']) {
@@ -186,6 +185,7 @@ module.exports = (opts = {}) => {
         if (typeof handler.onHeaders === 'function') {
           // Add the age header
           // https://www.rfc-editor.org/rfc/rfc9111.html#name-age
+          const age = Math.round((Date.now() - result.cachedAt) / 1000)
 
           // TODO (fix): What if rawHeaders already contains age header?
           rawHeaders = [...rawHeaders, AGE_HEADER, Buffer.from(`${age}`)]
@@ -216,7 +216,7 @@ module.exports = (opts = {}) => {
           throw new Error('stream is undefined but method isn\'t HEAD')
         }
 
-        const age = Math.round((nowAbsolute() - result.cachedAt) / 1000)
+        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

lib/interceptor/cache.js

@@ -21,13 +21,6 @@
  */
 let fastNow = 0
 
-/**
- * The fastNowAbsolute variable contains the rough result of Date.now()
- *
- * @type {number}
- */
-let fastNowAbsolute = Date.now()
-
 /**
  * RESOLUTION_MS represents the target resolution time in milliseconds.
  *
@@ -129,8 +122,6 @@ function onTick () {
    */
   fastNow += TICK_MS
 
-  fastNowAbsolute = Date.now()
-
   /**
    * The `idx` variable is used to iterate over the `fastTimers` array.
    * Expired timers are removed by replacing them with the last element in the array.
@@ -399,9 +390,6 @@ module.exports = {
   now () {
     return fastNow
   },
-  nowAbsolute () {
-    return fastNowAbsolute
-  },
   /**
    * Trigger the onTick function to process the fastTimers array.
    * Exported for testing purposes only.
@@ -431,11 +419,5 @@ module.exports = {
    * Marking as deprecated to discourage any use outside of testing.
    * @deprecated
    */
-  kFastTimer,
-  /**
-   * Exporting for testing purposes only.
-   * Marking as deprecated to discourage any use outside of testing.
-   * @deprecated
-   */
-  RESOLUTION_MS
+  kFastTimer
 }

lib/util/timers.js

@@ -5,7 +5,6 @@ const { deepStrictEqual, notEqual, equal } = require('node:assert')
 const { Readable } = require('node:stream')
 const { once } = require('node:events')
 const MemoryCacheStore = require('../../lib/cache/memory-cache-store')
-const { nowAbsolute } = require('../../lib/util/timers.js')
 
 cacheStoreTests(MemoryCacheStore)
 
@@ -33,9 +32,9 @@ function cacheStoreTests (CacheStore) {
         statusCode: 200,
         statusMessage: '',
         rawHeaders: [Buffer.from('1'), Buffer.from('2'), Buffer.from('3')],
-        cachedAt: nowAbsolute(),
-        staleAt: nowAbsolute() + 10000,
-        deleteAt: nowAbsolute() + 20000
+        cachedAt: Date.now(),
+        staleAt: Date.now() + 10000,
+        deleteAt: Date.now() + 20000
       }
       const requestBody = ['asd', '123']
 
@@ -73,9 +72,9 @@ function cacheStoreTests (CacheStore) {
         statusCode: 200,
         statusMessage: '',
         rawHeaders: [Buffer.from('1'), Buffer.from('2'), Buffer.from('3')],
-        cachedAt: nowAbsolute(),
-        staleAt: nowAbsolute() + 10000,
-        deleteAt: nowAbsolute() + 20000
+        cachedAt: Date.now(),
+        staleAt: Date.now() + 10000,
+        deleteAt: Date.now() + 20000
       }
       const anotherBody = ['asd2', '1234']
 
@@ -111,9 +110,9 @@ function cacheStoreTests (CacheStore) {
         statusCode: 200,
         statusMessage: '',
         rawHeaders: [Buffer.from('1'), Buffer.from('2'), Buffer.from('3')],
-        cachedAt: nowAbsolute() - 10000,
-        staleAt: nowAbsolute() - 1,
-        deleteAt: nowAbsolute() + 20000
+        cachedAt: Date.now() - 10000,
+        staleAt: Date.now() - 1,
+        deleteAt: Date.now() + 20000
       }
       const requestBody = ['part1', 'part2']
 
@@ -144,9 +143,9 @@ function cacheStoreTests (CacheStore) {
       const requestValue = {
         statusCode: 200,
         statusMessage: '',
-        cachedAt: nowAbsolute() - 20000,
-        staleAt: nowAbsolute() - 10000,
-        deleteAt: nowAbsolute() - 5
+        cachedAt: Date.now() - 20000,
+        staleAt: Date.now() - 10000,
+        deleteAt: Date.now() - 5
       }
       const requestBody = ['part1', 'part2']
 
@@ -178,9 +177,9 @@ function cacheStoreTests (CacheStore) {
         vary: {
           'some-header': 'hello world'
         },
-        cachedAt: nowAbsolute(),
-        staleAt: nowAbsolute() + 10000,
-        deleteAt: nowAbsolute() + 20000
+        cachedAt: Date.now(),
+        staleAt: Date.now() + 10000,
+        deleteAt: Date.now() + 20000
       }
       const requestBody = ['part1', 'part2']
 

test/cache-interceptor/cache-stores.js

@@ -0,0 +1,47 @@
+'use strict'
+
+const { test, after } = require('node:test')
+const { strictEqual } = require('node:assert')
+const { createServer } = require('node:http')
+const { once } = require('node:events')
+const { request, Client, interceptors } = require('../../index')
+const { setTimeout: sleep } = require('timers/promises')
+
+test('revalidates the request when the response is stale', async () => {
+  let count = 0
+  const server = createServer((req, res) => {
+    res.setHeader('Cache-Control', 'public, max-age=1')
+    res.end('hello world ' + count++)
+  })
+
+  server.listen(0)
+  await once(server, 'listening')
+
+  const dispatcher = new Client(`http://localhost:${server.address().port}`)
+    .compose(interceptors.cache())
+
+  after(async () => {
+    server.close()
+    await dispatcher.close()
+  })
+
+  const url = `http://localhost:${server.address().port}`
+
+  {
+    const res = await request(url, { dispatcher })
+    strictEqual(await res.body.text(), 'hello world 0')
+  }
+
+  {
+    const res = await request(url, { dispatcher })
+    strictEqual(await res.body.text(), 'hello world 0')
+  }
+
+  await sleep(1000)
+
+  {
+    const res = await request(url, { dispatcher })
+
+    strictEqual(await res.body.text(), 'hello world 1')
+  }
+})

test/interceptors/cache-fastimers-fix.js

@@ -161,7 +161,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')
@@ -207,7 +206,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

test/interceptors/cache.js

@@ -255,16 +255,4 @@ describe('timers', () => {
 
     await t.completed
   })
-
-  test('nowAbsolute', (t) => {
-    t = tspl(t, { plan: 1 })
-
-    const actualNow = Date.now()
-
-    const lowerBound = actualNow - timers.RESOLUTION_MS
-    const upperBound = actualNow + timers.RESOLUTION_MS
-    const fastNowAbsolute = timers.nowAbsolute()
-
-    t.equal(fastNowAbsolute >= lowerBound && fastNowAbsolute <= upperBound, true)
-  })
 })