Unlock on error (#6)

* Unlock on error

* Bump package version

package.json

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

src/leprechaun-cache.ts

@@ -156,33 +156,39 @@ export class LeprechaunCache<T extends Cacheable = Cacheable> {
         return result.data
       }
     }
-
-    const data = await this.onMiss(key)
-
-    //Set and unlock in the actual cache asynchronously
-    this.setAndUnlock(
-      key,
-      {
-        data,
-        expiresAt: Date.now() + ttl
-      },
-      lock
-    )
-
-    return data
+    try {
+      const data = await this.onMiss(key)
+      //Set and unlock asynchronously so we don't delay the response
+      this.setAndUnlock(
+        key,
+        {
+          data,
+          expiresAt: Date.now() + ttl
+        },
+        lock
+      )
+
+      return data
+    } catch (e) {
+      this.unlock(key, lock)
+      throw e
+    }
   }
 
-  private async setAndUnlock(key: string, cacheData: CacheItem<T>, lock: LockResult) {
+  private async unlock(key: string, lock: LockResult) {
     try {
-      await this.cacheStore.set(this.keyPrefix + key, cacheData, this.hardTtlMs)
+      await this.cacheStore.unlock(this.keyPrefix + key, lock.lockId)
     } catch (e) {
       this.onBackgroundError(e)
     }
-    //unlock in separate try-catch block so that unlock will be attempted even if set fails
+  }
+
+  private async setAndUnlock(key: string, cacheData: CacheItem<T>, lock: LockResult) {
     try {
-      await this.cacheStore.unlock(this.keyPrefix + key, lock.lockId)
+      await this.cacheStore.set(this.keyPrefix + key, cacheData, this.hardTtlMs)
     } catch (e) {
       this.onBackgroundError(e)
     }
+    await this.unlock(key, lock)
   }
 }

test/unit/leprechaun-cache.spec.ts

@@ -116,6 +116,32 @@ describe('Leprechaun Cache', () => {
     expect(onMiss).calledTwice
   })
 
+  it('should unlock if the onMiss handler throws an exception', async () => {
+    const key = 'key'
+    const onMiss = sandbox.stub().rejects(new Error('Error'))
+
+    const cache = new LeprechaunCache({
+      softTtlMs: 80,
+      hardTtlMs: 10000,
+      waitForUnlockMs: 10,
+      spinMs: 10,
+      lockTtlMs: 10000,
+      cacheStore: memoryCacheStore,
+      returnStale: true,
+      onMiss
+    })
+    try {
+      await cache.get(key)
+      expect(false).to.be.true //Should not reach as the exception should be rethrown
+    } catch (e) {}
+    const data = {
+      some: 'data'
+    }
+    onMiss.resolves(data)
+    const result = await cache.get('key') //If it's locked then this will fail due to the short waitForUnlockMs
+    expect(result).to.deep.equal(data)
+  })
+
   it('will return the update if it takes less time than the waitTimeMs handler to resolve', async () => {
     const data1 = {
       some: 'data'