diff --git a/src/executor.spec.ts b/src/executor.spec.ts index 8771b7c..6f7a66b 100644 --- a/src/executor.spec.ts +++ b/src/executor.spec.ts @@ -140,6 +140,59 @@ describe('IdempotentExecutor.run method', () => { expect(action).toHaveBeenCalledTimes(1); }); + it.each([0, -1, Number.NaN])( + 'should reject invalid timeout values (%s)', + async (timeout) => { + const action = jest.fn().mockResolvedValue('action result'); + + await expect( + executor.run(`invalid-timeout-${String(timeout)}`, action, { + timeout, + }), + ).rejects.toThrow( + new RangeError( + 'Timeout must be a positive finite number in milliseconds', + ), + ); + expect(action).toHaveBeenCalledTimes(0); + }, + ); + + it('should normalize timeout and compute integer lock retry settings', async () => { + const action = jest.fn().mockResolvedValue('action result'); + const usingSpy = jest.spyOn( + ( + executor as unknown as { + redlock: { + using: (...args: unknown[]) => Promise; + }; + } + ).redlock, + 'using', + ); + + await executor.run('normalized-timeout-key', action, { timeout: 399.1 }); + + expect(usingSpy).toHaveBeenCalledTimes(1); + const usingArgs = usingSpy.mock.calls[0] as [ + string[], + number, + { + retryCount: number; + retryDelay: number; + automaticExtensionThreshold: number; + }, + ]; + + expect(usingArgs[1]).toBe(400); + expect(Number.isInteger(usingArgs[2].retryCount)).toBe(true); + expect(usingArgs[2].retryCount).toBe(2); + expect(usingArgs[2].retryDelay).toBe(200); + expect(Number.isInteger(usingArgs[2].automaticExtensionThreshold)).toBe( + true, + ); + }); + it('should timeout lock if action takes too long', async () => { const action = jest .fn() diff --git a/src/executor.ts b/src/executor.ts index 7787c46..9d423c3 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -55,6 +55,9 @@ interface IdempotentExecutorOptions { export class IdempotentExecutor { private static readonly CACHE_KEY_PREFIX = 'idempotent-executor-result'; private static readonly LOCK_KEY_PREFIX = 'idempotent-executor-lock'; + private static readonly DEFAULT_TIMEOUT_MS = 60000; + private static readonly LOCK_RETRY_DELAY_MS = 200; + private static readonly LOCK_EXTENSION_BUFFER_MS = 100; private redlock: Redlock; private cache: RedisCache; @@ -100,7 +103,8 @@ export class IdempotentExecutor { action: () => Promise, options?: Partial>, ): Promise { - const timeout = options?.timeout ?? 60000; + const timeout = this.normalizeTimeout(options?.timeout); + const lockSettings = this.buildLockSettings(timeout); const valueSerializer = options?.valueSerializer ?? new JSONSerializer(); const errorSerializer = options?.errorSerializer ?? new DefaultErrorSerializer(); @@ -111,11 +115,7 @@ export class IdempotentExecutor { return await this.redlock.using( [lockKey], timeout, - { - retryCount: timeout / 200, - retryDelay: 200, - automaticExtensionThreshold: timeout / 2, - }, + lockSettings, async () => { // Retrieve the cached result of the action. const cachedResult = await this.getCachedResult( @@ -401,6 +401,44 @@ export class IdempotentExecutor { return value; } + /** + * Validates and normalizes the lock timeout in milliseconds. + */ + private normalizeTimeout(timeout?: number): number { + const timeoutToUse = timeout ?? IdempotentExecutor.DEFAULT_TIMEOUT_MS; + if (!Number.isFinite(timeoutToUse) || timeoutToUse <= 0) { + throw new RangeError( + 'Timeout must be a positive finite number in milliseconds', + ); + } + + return Math.ceil(timeoutToUse); + } + + /** + * Computes lock settings with explicit integer values. + */ + private buildLockSettings(timeout: number): { + retryCount: number; + retryDelay: number; + automaticExtensionThreshold: number; + } { + const retryCount = Math.max( + 0, + Math.floor(timeout / IdempotentExecutor.LOCK_RETRY_DELAY_MS), + ); + const automaticExtensionThreshold = Math.min( + Math.floor(timeout / 2), + timeout - IdempotentExecutor.LOCK_EXTENSION_BUFFER_MS, + ); + + return { + retryCount, + retryDelay: IdempotentExecutor.LOCK_RETRY_DELAY_MS, + automaticExtensionThreshold, + }; + } + /** * Creates the cache key for action results. */