deleteByTag in combination with grace period doesn't invalidate cached value

[yss14]

I've encountered an issue where calling deleteByTag does not properly invalidate cached values when a grace period is configured. After invalidation, the cache still returns the stale ("old") value for getOrSet.

Initially, I suspected this behavior was limited to distributed setups using a bus, but it also occurs with a single cache instance. This suggests the issue is not related to multi-node synchronization but possibly to how deleteByTag interacts with the grace logic.

For reproduction, I quickly wrote two test cases in tagging.spec.ts on my local machine:

test('remove by tags with grace period', async ({ assert }) => {
    const [cache] = new CacheFactory().withL1L2Config().create()

    const v1 = await cache.getOrSet({
      key: 'baz',
      factory: () => 1,
      tags: ['x', 'z'],
      ttl: '2min',
      grace: '10min',
    })

    assert.deepEqual(v1, 1)

    await cache.deleteByTag({ tags: ['x'] })

    const v2get = await cache.get({ key: 'baz' })
    const v2getorset = await cache.getOrSet({
      key: 'baz',
      factory: () => 2,
      tags: ['x', 'z'],
      ttl: '2min',
      grace: '10min',
    })

    assert.isUndefined(v2get)
    assert.deepEqual(v2getorset, 2)
  })

  test('remove by tags with bus and grace period', async ({ assert }) => {
    const [readCache] = new CacheFactory().withL1L2Config().create()
    const [invalidationCache] = new CacheFactory().withL1L2Config().create()

    const v1 = await readCache.getOrSet({
      key: 'baz',
      factory: () => 1,
      tags: ['x', 'z'],
      ttl: '2min',
      grace: '10min',
    })

    assert.deepEqual(v1, 1)

    await invalidationCache.deleteByTag({ tags: ['x'] })
    //await invalidationCache.delete({ key: 'baz' })  <- this works btw.

    const v2get = await readCache.get({ key: 'baz' })
    const v2getorset = await readCache.getOrSet({
      key: 'baz',
      factory: () => 2,
      tags: ['x', 'z'],
      ttl: '2min',
      grace: '10min',
    })

    assert.isUndefined(v2get)
    assert.deepEqual(v2getorset, 2)
  })

Result:

> cross-env NODE_NO_WARNINGS=1 node --enable-source-maps --loader=ts-node/esm bin/test.ts "--suite=unit" "--files=tests/tagging.spec.ts"


unit / Tagging | Internals (tests/tagging.spec.ts)
  ✔ deleteByTag should store invalidated tags with timestamps (7.08ms)
  ✔ set should store value with tags (2.1ms)

unit / Tagging | deleteByTag (tests/tagging.spec.ts)
  ✔ basic (2.94ms)
  ✔ can remove by tag (6.51ms)
  ✔ can remove multiple tags (3.6ms)
  ✔ remove by tags with bus (6.01ms)
  ✖ remove by tags with grace period (2.74ms)
  ✖ remove by tags with bus and grace period (2.99ms)
  ✔ remove multiple tags with bus (8.59ms)
  ✔ tags and namespaces (3.56ms)
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ERRORS ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

❯ Tagging | deleteByTag / remove by tags with grace period

- Expected  - 1
+ Received  + 1

- 2
+ 1

ℹ AssertionError: expected 1 to deeply equal 2

 ⁃ at Assert.deepEqual (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+assert@4.0.1_@japa+runner@4.2.0/node_modules/@japa/assert/build/index.js:282:19)
 ⁃ at Object.executor (tests/tagging.spec.ts:192:12)

   187 ┃        ttl: '2min',
   188 ┃        grace: '10min',
   189 ┃      })
   190 ┃  
   191 ┃      assert.isUndefined(v2get)
 ❯ 192 ┃      assert.deepEqual(v2getorset, 2)
   193 ┃    })
   194 ┃  
   195 ┃    test('remove by tags with bus and grace period', async ({ assert }) => {
   196 ┃      const [readCache] = new CacheFactory().withL1L2Config().create()
   197 ┃      const [invalidationCache] = new CacheFactory().withL1L2Config().create()

 ⁃ at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
 ⁃ at async #wrapTestInTimeout (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:1054:7)
 ⁃ at async TestRunner.run (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:1102:7)
 ⁃ at async Test.exec (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:1482:5)
 ⁃ at async GroupRunner.run (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:345:7)
 ⁃ at async Group.exec (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:513:5)
 ⁃ at async SuiteRunner.run (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:1809:7)
 ⁃ at async Suite.exec (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+core@10.3.0/node_modules/@japa/core/build/index.js:1936:5)
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[1/2]─

❯ Tagging | deleteByTag / remove by tags with bus and grace period

- Expected  - 1
+ Received  + 1

- 2
+ 1

ℹ AssertionError: expected 1 to deeply equal 2

 ⁃ at Assert.deepEqual (/Users/yss/Dev/open-source/bentocache/node_modules/.pnpm/@japa+assert@4.0.1_@japa+runner@4.2.0/node_modules/@japa/assert/build/index.js:282:19)
 ⁃ at Object.executor (tests/tagging.spec.ts:222:12)

   217 ┃        ttl: '2min',
   218 ┃        grace: '10min',
   219 ┃      })
   220 ┃  
   221 ┃      assert.isUndefined(v2get)
 ❯ 222 ┃      assert.deepEqual(v2getorset, 2)
   223 ┃    })

At this point, I'm not sure whether this is intended behavior or a bug. However, I would lean toward it being a bug—especially since the interaction between deleteByTag and the grace period doesn’t appear to be covered by the existing test suite.

[Julien-R44]

Hey,
so you might end up hating me for this 🤣 because yeah, I totally realize this API might not feel super intuitive, and I hesitated a lot before settling on it. Maybe I took the wrong decision....

When you use grace, by default it switches to kinda SWR ( Stale while revalidate ) mode. If there's a cached but stale value, it'll return it immediately while running the factory in the background. That way, next time you call it, you'll get the fresh value instantly

This behavior can be tweaked via timeout option (soft timeout): https://bentocache.dev/docs/options#timeout
As you can see, the default is 0, which means it does exactly what I just described.

  const [cache] = new CacheFactory().withL1L2Config().create()

  // Here, we have no choice but to wait for the factory to execute
  // because there's nothing in the cache, not even a stale value.
  const v1 = await cache.getOrSet({
    key: 'baz',
    factory: () => 1,
    tags: ['x', 'z'],
    ttl: '2min',
    grace: '10min',
  }) 

  await cache.deleteByTag({ tags: ['x'] })

  // Now, `grace` is configured, and `timeout` is not set, so it defaults to 0.
  // That means getOrSet will return the stale value immediately,
  // and run the factory in the background. Next call will get the fresh value.
  const v2getorset = await cache.getOrSet({
    key: 'baz',
    factory: () => 2,
    tags: ['x', 'z'],
    ttl: '2min',
    grace: '10min',
  })

  assert.isUndefined(v2get)
  assert.deepEqual(v2getorset, 2)

If you want to avoid this behavior, which might make sense depending on your use case, you can just bump up the soft timeout:

  const v2getorset = await cache.getOrSet({
    key: 'baz',
    factory: () => 2,
    tags: ['x', 'z'],
    ttl: '2min',
    grace: '10min',
    // if the factory takes less than 2s to run, great, we return the fresh value.
    // if it takes more, return the stale value and keep running the factory in the background
    timeout: '2s', 
  })

Does it make sense to you?
And thanks a lot for writing a quick test to explain your problem! really helps to understand the problem

[yss14]

@Julien-R44 Thx for the quick reply and great explanation!

Under the assumption that deleteByTag only invalidates, but not completely removes the entry from the cache, your explanation makes totally sense.
However, I'm a bit confused now by the naming of the methods and their effects.
I assumed that deleteByTag from the name works the same way as delete and deleteMany and completely removes the cache entry from the cache, forcing a fresh result on the next getOrSet call, independent of grace periods.
However, your explanation sounds more like deleteByTag works like expire, so the cache value is only marked as stale and with that the interplay with grace periods make totally sense, is that correct?
If so, wouldn't it make sense to rename deleteByTag to expireByTag so the naming and effects are aligned?