From 9c7776e90b89cf54441221a9c9a542f23a93c9a6 Mon Sep 17 00:00:00 2001 From: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:18:22 +0300 Subject: [PATCH] fix(cache): skip expired entries when searching sqlite cache In `#findValue`, when iterating cached values sorted by `deleteAt ASC`, encountering an expired entry caused an immediate `return undefined`. This meant a valid, non-expired cache entry was missed whenever an expired entry existed for the same URL and method (for example when `vary` produced multiple rows). Skip expired entries with `continue` so the search keeps looking for a match. Co-Authored-By: Claude Co-Authored-By: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com> Co-Authored-By: Nikita Skovoroda Signed-off-by: Nikita Skovoroda --- lib/cache/sqlite-cache-store.js | 2 +- .../sqlite-cache-store-tests.js | 75 +++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/lib/cache/sqlite-cache-store.js b/lib/cache/sqlite-cache-store.js index 7cb4aa7e246..867d16cf7bb 100644 --- a/lib/cache/sqlite-cache-store.js +++ b/lib/cache/sqlite-cache-store.js @@ -409,7 +409,7 @@ module.exports = class SqliteCacheStore { const now = Date.now() for (const value of values) { if (now >= value.deleteAt && !canBeExpired) { - return undefined + continue } let matches = true diff --git a/test/cache-interceptor/sqlite-cache-store-tests.js b/test/cache-interceptor/sqlite-cache-store-tests.js index 47cf5651687..bbf7480d587 100644 --- a/test/cache-interceptor/sqlite-cache-store-tests.js +++ b/test/cache-interceptor/sqlite-cache-store-tests.js @@ -3,6 +3,7 @@ const { test } = require('node:test') const { notEqual, strictEqual, deepStrictEqual } = require('node:assert') const { rm } = require('node:fs/promises') +const FakeTimers = require('@sinonjs/fake-timers') const { cacheStoreTests, writeBody, compareGetResults } = require('./cache-store-test-utils.js') const { runtimeFeatures } = require('../../lib/util/runtime-features.js') @@ -156,6 +157,80 @@ test('SqliteCacheStore two writes', { skip: runtimeFeatures.has('sqlite') === fa } }) +test('SqliteCacheStore skips expired entry to find non-expired match', { skip: runtimeFeatures.has('sqlite') === false }, async (t) => { + const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js') + + const clock = FakeTimers.install({ + shouldClearNativeTimers: true + }) + t.after(() => clock.uninstall()) + + const store = new SqliteCacheStore({ + maxCount: 100 + }) + + const keyA = { + origin: 'localhost', + path: '/', + method: 'GET', + headers: { 'x-vary': 'a' } + } + + const valueA = { + statusCode: 200, + statusMessage: '', + headers: { foo: 'bar' }, + vary: { 'x-vary': 'a' }, + cacheControlDirectives: {}, + cachedAt: Date.now(), + staleAt: Date.now() + 1000, + deleteAt: Date.now() + 1000 + } + + const bodyA = [Buffer.from('first')] + + { + const writable = store.createWriteStream(keyA, valueA) + notEqual(writable, undefined) + writeBody(writable, bodyA) + } + + // Advance past valueA's deleteAt + clock.tick(2000) + + const keyB = { + origin: 'localhost', + path: '/', + method: 'GET', + headers: { 'x-vary': 'b' } + } + + const valueB = { + statusCode: 200, + statusMessage: '', + headers: { foo: 'baz' }, + vary: { 'x-vary': 'b' }, + cacheControlDirectives: {}, + cachedAt: Date.now(), + staleAt: Date.now() + 10000, + deleteAt: Date.now() + 10000 + } + + const bodyB = [Buffer.from('second')] + + { + const writable = store.createWriteStream(keyB, valueB) + notEqual(writable, undefined) + writeBody(writable, bodyB) + } + + // The expired entry (valueA) is sorted first by deleteAt ASC; the + // store must skip it and still return the matching valueB. + const result = store.get(structuredClone(keyB)) + notEqual(result, undefined) + await compareGetResults(result, valueB, bodyB) +}) + test('SqliteCacheStore write & read', { skip: runtimeFeatures.has('sqlite') === false }, async () => { const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')