Fix stale update lock error (#7)

* If the item has soft-expired and we get multiple requests then don't error if the lock never gets acquired, but return the stale data

* Handle if returnStale is false

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@acuris/leprechaun-cache",
-  "version": "0.0.11",
+  "version": "0.0.12",
   "private": false,
   "description": "Caching library that supports double checked caching and stale returns to avoid stampede and slow responses",
   "keywords": [

src/leprechaun-cache.ts

@@ -76,20 +76,27 @@ export class LeprechaunCache<T extends Cacheable = Cacheable> {
   }
 
   public async refresh(key: string): Promise<T> {
-    return this.updateCache(key, this.softTtlMs, true)
+    return this.updateCache(key, this.softTtlMs)
   }
 
   private async doGet(key: string, ttl: number): Promise<T> {
     const result = await this.cacheStore.get(this.keyPrefix + key)
     if (!result) {
-      return this.updateCache(key, ttl, true)
+      return this.updateCache(key, ttl)
     }
 
     if (result.expiresAt > Date.now()) {
       return result.data
     }
 
-    const update = this.updateCache(key, ttl, !this.returnStale)
+    const update = this.updateCache(key, ttl).catch(e => {
+      if (this.returnStale) {
+        this.onBackgroundError(e)
+        return result.data
+      }
+
+      throw e
+    })
 
     if (!this.returnStale) {
       return update
@@ -132,17 +139,8 @@ export class LeprechaunCache<T extends Cacheable = Cacheable> {
     return lock
   }
 
-  private async getLock(key: string, doSpinLock: boolean): Promise<LockResult> {
-    return doSpinLock
-      ? this.spinLock(key)
-      : {
-          lockId: (await this.cacheStore.lock(this.keyPrefix + key, this.lockTtlMs)) || '',
-          didSpin: false
-        }
-  }
-
-  private async updateCache(key: string, ttl: number, doSpinLock: boolean): Promise<T> {
-    const lock = await this.getLock(key, doSpinLock)
+  private async updateCache(key: string, ttl: number): Promise<T> {
+    const lock = await this.spinLock(key)
 
     if (!lock.lockId) {
       throw new Error('unable to acquire lock and no data in cache')

test/unit/leprechaun-cache.spec.ts

@@ -306,6 +306,78 @@ describe('Leprechaun Cache', () => {
     expect(results2).to.deep.equal(data2)
   })
 
+  it('should return the stale version (with returnStale true) of the data if the update for the latest version fails due to cache lock', async () => {
+    const data = {
+      some: 'data'
+    }
+
+    const onMiss = async (_: string) => {
+      return data
+    }
+
+    const key = 'key'
+
+    const cache = new LeprechaunCache({
+      softTtlMs: 80,
+      hardTtlMs: 10000,
+      waitForUnlockMs: 10,
+      spinMs: 50,
+      lockTtlMs: 1000,
+      cacheStore: memoryCacheStore,
+      returnStale: true,
+      onMiss,
+      waitTimeMs: 400
+    })
+
+    //initial population:
+    await cache.get(key)
+    await delay(100) //delay for the ttl, so the item is now out of date
+
+    //Force lock the key, so that the update won't work
+    memoryCacheStore.lock(key, 10000)
+    const result = await cache.get(key)
+
+    //we expect result to be data
+    expect(result).to.deep.equal(data)
+  })
+
+  it('should not return the stale version (with returnStale false) of the data if the update for the latest version fails due to cache lock, it should error', async () => {
+    const data = {
+      some: 'data'
+    }
+
+    const onMiss = async (_: string) => {
+      return data
+    }
+
+    const key = 'key'
+
+    const cache = new LeprechaunCache({
+      softTtlMs: 80,
+      hardTtlMs: 10000,
+      waitForUnlockMs: 10,
+      spinMs: 50,
+      lockTtlMs: 1000,
+      cacheStore: memoryCacheStore,
+      returnStale: false,
+      onMiss,
+      waitTimeMs: 400
+    })
+
+    //initial population:
+    await cache.get(key)
+    await delay(100) //delay for the ttl, so the item is now out of date
+
+    //Force lock the key, so that the update won't work
+    memoryCacheStore.lock(key, 10000)
+    try {
+      await cache.get(key)
+      expect(false).to.be.true
+    } catch (e) {
+      expect(true).to.be.true
+    }
+  })
+
   it('should save and return undefined and null and false correctly', async () => {
     const onMissStub = sandbox.stub()
     onMissStub.withArgs('key-undefined').resolves(undefined)