refactor!: do not throw anymore when can't acquire lock

Author: Julien-R44

packages/verrou/src/errors.ts

@@ -1,18 +1,10 @@
 import { createError } from '@poppinss/utils'
 
-/**
- * Thrown when the lock is not acquired in the allotted time
- */
-export const E_LOCK_TIMEOUT = createError(
-  `Lock was not acquired in the allotted time`,
-  'E_LOCK_TIMEOUT',
-)
-
 /**
  * Thrown when user tries to update/release/extend a lock that is not acquired by them
  */
 export const E_LOCK_NOT_OWNED = createError(
-  'Looks like you are trying to update a lock that is not acquired by you',
+  'Looks like you are trying to update or release a lock that is not acquired by you',
   'E_LOCK_NOT_OWNED',
 )
 
@@ -23,11 +15,3 @@ export const E_LOCK_STORAGE_ERROR = createError<[{ message: string }]>(
   'Lock storage error: %s',
   'E_LOCK_STORAGE_ERROR',
 )
-
-/**
- * Thrown when user tries to acquire a lock that is already acquired by someone else
- */
-export const E_LOCK_ALREADY_ACQUIRED = createError(
-  'Lock is already acquired',
-  'E_LOCK_ALREDY_ACQUIRED',
-)

packages/verrou/src/lock.ts

@@ -2,7 +2,6 @@ import { setTimeout } from 'node:timers/promises'
 import { InvalidArgumentsException } from '@poppinss/utils'
 
 import { resolveDuration } from './helpers.js'
-import { E_LOCK_ALREADY_ACQUIRED, E_LOCK_TIMEOUT } from './errors.js'
 import type {
   Duration,
   LockAcquireOptions,
@@ -88,13 +87,13 @@ export class Lock {
       /**
        * Check if we reached the maximum number of attempts
        */
-      if (attemptsDone === attemptsMax) throw new E_LOCK_TIMEOUT()
+      if (attemptsDone === attemptsMax) return false
 
       /**
        * Or check if we reached the timeout
        */
       const elapsed = Date.now() - start
-      if (timeout && elapsed > timeout) throw new E_LOCK_TIMEOUT()
+      if (timeout && elapsed > timeout) return false
 
       /**
        * Otherwise wait for the delay and try again
@@ -103,28 +102,33 @@ export class Lock {
     }
 
     this.#config.logger.debug({ key: this.#key }, 'Lock acquired')
+    return true
   }
 
   /**
    * Try to acquire the lock immediately or throw an error
    */
   async acquireImmediately() {
     const result = await this.#lockStore.save(this.#key, this.#owner, this.#ttl)
-    if (!result) throw new E_LOCK_ALREADY_ACQUIRED()
+    if (!result) return false
     this.#expirationTime = this.#ttl ? Date.now() + this.#ttl : null
 
     this.#config.logger.debug({ key: this.#key }, 'Lock acquired with acquireImmediately()')
+    return true
   }
 
   /**
    * Acquire the lock, run the callback and release the lock automatically
    * after the callback is done.
    * Also returns the callback return value
    */
-  async run<T>(callback: () => Promise<T>): Promise<T> {
+  async run<T>(callback: () => Promise<T>): Promise<[true, T] | [false, null]> {
+    const handle = await this.acquire()
+    if (!handle) return [false, null]
+
     try {
-      await this.acquire()
-      return await callback()
+      const result = await callback()
+      return [true, result]
     } finally {
       await this.release()
     }
@@ -134,12 +138,15 @@ export class Lock {
    * Same as `run` but try to acquire the lock immediately
    * Or throw an error if the lock is already acquired
    */
-  async runImmediately<T>(callback: () => Promise<T>): Promise<T> {
+  async runImmediately<T>(callback: () => Promise<T>): Promise<[true, T] | [false, null]> {
+    const handle = await this.acquireImmediately()
+    if (!handle) return [false, null]
+
     try {
-      await this.acquireImmediately()
-      return await callback()
+      const result = await callback()
+      return [true, result]
     } finally {
-      await this.release()
+      if (handle) await this.release()
     }
   }
 

packages/verrou/src/test_suite.ts

@@ -4,9 +4,9 @@ import type { Group } from '@japa/runner/core'
 import type { test as JapaTest } from '@japa/runner'
 import { setTimeout as sleep } from 'node:timers/promises'
 
+import { E_LOCK_NOT_OWNED } from '../index.js'
 import { LockFactory } from './lock_factory.js'
 import type { LockStore } from './types/main.js'
-import { E_LOCK_NOT_OWNED, E_LOCK_TIMEOUT } from '../index.js'
 
 export function registerStoreTestSuite(options: {
   test: typeof JapaTest
@@ -73,32 +73,33 @@ export function registerStoreTestSuite(options: {
     // @ts-expect-error poppinss/utils typing bug
   }).throws(E_LOCK_NOT_OWNED.message, E_LOCK_NOT_OWNED)
 
-  test('throws timeout error when lock is not acquired in time', async () => {
+  test('acquire returns false when lock is not acquired in time', async ({ assert }) => {
     const provider = new LockFactory(options.createStore(), {
       retry: { timeout: 500 },
     })
     const lock = provider.createLock('foo')
 
     await lock.acquire()
 
-    await lock.acquire()
-    // @ts-expect-error poppinss/utils typing bug
-  }).throws(E_LOCK_TIMEOUT.message, E_LOCK_TIMEOUT)
+    const handle = await lock.acquire()
+    assert.isFalse(handle)
+  })
 
   test('run passes result', async ({ assert }) => {
     const provider = new LockFactory(options.createStore())
     const lock = provider.createLock('foo')
 
-    const result = await lock.run(async () => 'hello world')
+    const [executed, result] = await lock.run(async () => 'hello world')
 
-    assert.equal(result, 'hello world')
+    assert.deepEqual(executed, true)
+    assert.deepEqual(result, 'hello world')
   })
 
   test('run passes result from a promise', async ({ assert }) => {
     const provider = new LockFactory(options.createStore())
     const lock = provider.createLock('foo')
 
-    const result = await lock.run(async () => Promise.resolve('hello world'))
+    const [, result] = await lock.run(async () => Promise.resolve('hello world'))
 
     assert.equal(result, 'hello world')
   })
@@ -139,13 +140,13 @@ export function registerStoreTestSuite(options: {
 
     assert.isFalse(flag)
 
-    const result = await lock.run(async () => {
+    const [, result] = await lock.run(async () => {
       assert.isTrue(flag)
       return '42'
     })
 
     assert.isTrue(flag)
-    assert.equal(result, '42')
+    assert.deepEqual(result, '42')
   })
 
   test('exceptions during run do not leave mutex in locked state', async ({ assert }) => {