refactor: Better errors in BackoffStrategyFactory

Closes #161.

README.md

@@ -340,11 +340,11 @@ policy.waitBeforeRetry(BackoffStrategyFactory.exponentialBackoff(100, false, 3))
 // 0 100 300 900 2700 …
 policy.waitBeforeRetry(BackoffStrategyFactory.exponentialBackoff(100, true, 3));
 
-// wait for a [random between 0-100, inclusive] ms before each retry
-policy.waitBeforeRetry(BackoffStrategyFactory.jitteredBackoff(0, 100));
+// wait for a [random between 1-100, inclusive] ms before each retry
+policy.waitBeforeRetry(BackoffStrategyFactory.jitteredBackoff(1, 100));
 
-// retry immediately for the first time, then wait for a [random between 0-100, inclusive] ms before each retry
-policy.waitBeforeRetry(BackoffStrategyFactory.jitteredBackoff(0, 100, true));
+// retry immediately for the first time, then wait for a [random between 1-100, inclusive] ms before each retry
+policy.waitBeforeRetry(BackoffStrategyFactory.jitteredBackoff(1, 100, true));
 ```
 
 For using `jitteredBackoff` in Node.js environments, you will need to inject a Node.js-based entropy source into the default RandomGenerator ([@diplomatiq/crypto-random](https://github.com/Diplomatiq/crypto-random) requires `window.crypto.getRandomValues` to be available by default). Create the following in your project:

src/policies/proactive/timeoutPolicy/timeoutPolicy.ts

@@ -16,7 +16,7 @@ export class TimeoutPolicy<ResultType> extends ProactivePolicy<ResultType> {
             throw new Error('timeoutMs must be integer');
         }
 
-        if (timeoutMs < 0) {
+        if (timeoutMs <= 0) {
             throw new Error('timeoutMs must be greater than 0');
         }
 

src/policies/reactive/retryPolicy/backoffStrategyFactory.ts

@@ -3,12 +3,16 @@ import { RandomGenerator } from '../../../interfaces/randomGenerator';
 
 export class BackoffStrategyFactory {
     public static constantBackoff(delayMs: number, fastFirst = false): (currentRetryCount: number) => number {
+        this.validateNumericArgument(delayMs, 'delayMs');
+
         return fastFirst
             ? (currentRetryCount: number): number => (currentRetryCount === 1 ? 0 : delayMs)
             : (): number => delayMs;
     }
 
     public static linearBackoff(delayMs: number, fastFirst = false): (currentRetryCount: number) => number {
+        this.validateNumericArgument(delayMs, 'delayMs');
+
         return fastFirst
             ? (currentRetryCount: number): number => delayMs * (currentRetryCount - 1)
             : (currentRetryCount: number): number => delayMs * currentRetryCount;
@@ -19,6 +23,9 @@ export class BackoffStrategyFactory {
         fastFirst = false,
         base = 2,
     ): (currentRetryCount: number) => number {
+        this.validateNumericArgument(delayMs, 'delayMs');
+        this.validateNumericArgument(base, 'base');
+
         return fastFirst
             ? (currentRetryCount: number): number =>
                   currentRetryCount === 1 ? 0 : delayMs * base ** (currentRetryCount - 2)
@@ -31,6 +38,13 @@ export class BackoffStrategyFactory {
         fastFirst = false,
         randomGenerator: RandomGenerator = new DefaultRandomGenerator(),
     ): (currentRetryCount: number) => Promise<number> {
+        this.validateNumericArgument(minDelayMs, 'minDelayMs');
+        this.validateNumericArgument(maxDelayMs, 'maxDelayMs');
+
+        if (maxDelayMs <= minDelayMs) {
+            throw new Error('maxDelayMs must be greater than minDelayMs');
+        }
+
         return fastFirst
             ? async (currentRetryCount: number): Promise<number> => {
                   if (currentRetryCount === 1) {
@@ -44,4 +58,18 @@ export class BackoffStrategyFactory {
                   return ms;
               };
     }
+
+    private static validateNumericArgument(arg: number, name: string): void {
+        if (!Number.isInteger(arg)) {
+            throw new Error(`${name} must be integer`);
+        }
+
+        if (arg <= 0) {
+            throw new Error(`${name} must be greater than 0`);
+        }
+
+        if (!Number.isSafeInteger(arg)) {
+            throw new Error(`${name} must be less than or equal to 2^53 - 1`);
+        }
+    }
 }

test/specs/backoffStrategyFactory.test.ts

@@ -23,6 +23,38 @@ describe('BackoffStrategyFactory', (): void => {
             expect(strategy(4)).to.equal(100);
             expect(strategy(5)).to.equal(100);
         });
+
+        it('should throw error when setting delayMs to 0', (): void => {
+            try {
+                BackoffStrategyFactory.constantBackoff(0);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting delayMs to <0', (): void => {
+            try {
+                BackoffStrategyFactory.constantBackoff(-1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting delayMs to a non-integer', (): void => {
+            try {
+                BackoffStrategyFactory.constantBackoff(0.1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be integer');
+            }
+        });
+
+        it('should throw error when setting delayMs to a non-safe integer', (): void => {
+            try {
+                BackoffStrategyFactory.constantBackoff(2 ** 53);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be less than or equal to 2^53 - 1');
+            }
+        });
     });
 
     describe('linearBackoff', (): void => {
@@ -43,6 +75,38 @@ describe('BackoffStrategyFactory', (): void => {
             expect(strategy(4)).to.equal(300);
             expect(strategy(5)).to.equal(400);
         });
+
+        it('should throw error when setting delayMs to 0', (): void => {
+            try {
+                BackoffStrategyFactory.linearBackoff(0);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting delayMs to <0', (): void => {
+            try {
+                BackoffStrategyFactory.linearBackoff(-1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting delayMs to a non-integer', (): void => {
+            try {
+                BackoffStrategyFactory.linearBackoff(0.1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be integer');
+            }
+        });
+
+        it('should throw error when setting delayMs to a non-safe integer', (): void => {
+            try {
+                BackoffStrategyFactory.linearBackoff(2 ** 53);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be less than or equal to 2^53 - 1');
+            }
+        });
     });
 
     describe('exponentialBackoff', (): void => {
@@ -81,13 +145,77 @@ describe('BackoffStrategyFactory', (): void => {
             expect(strategy(4)).to.equal(900);
             expect(strategy(5)).to.equal(2700);
         });
+
+        it('should throw error when setting delayMs to 0', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(0);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting delayMs to <0', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(-1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting delayMs to a non-integer', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(0.1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be integer');
+            }
+        });
+
+        it('should throw error when setting delayMs to a non-safe integer', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(2 ** 53);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('delayMs must be less than or equal to 2^53 - 1');
+            }
+        });
+
+        it('should throw error when setting base to 0', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(100, false, 0);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('base must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting base to <0', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(100, false, -1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('base must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting base to a non-integer', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(100, false, 0.1);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('base must be integer');
+            }
+        });
+
+        it('should throw error when setting base to a non-safe integer', (): void => {
+            try {
+                BackoffStrategyFactory.exponentialBackoff(100, false, 2 ** 53);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('base must be less than or equal to 2^53 - 1');
+            }
+        });
     });
 
     describe('jitteredBackoff', (): void => {
         it('should produce a jittered backoff strategy', async (): Promise<void> => {
             const entropyProvider = new NodeJsEntropyProvider();
             const randomGenerator = new RandomGenerator(entropyProvider);
-            const strategy = BackoffStrategyFactory.jitteredBackoff(0, 100, false, randomGenerator);
+            const strategy = BackoffStrategyFactory.jitteredBackoff(1, 100, false, randomGenerator);
 
             expect(await strategy(1))
                 .to.be.at.least(0)
@@ -115,7 +243,7 @@ describe('BackoffStrategyFactory', (): void => {
         > => {
             const entropyProvider = new NodeJsEntropyProvider();
             const randomGenerator = new RandomGenerator(entropyProvider);
-            const strategy = BackoffStrategyFactory.jitteredBackoff(0, 100, true, randomGenerator);
+            const strategy = BackoffStrategyFactory.jitteredBackoff(1, 100, true, randomGenerator);
 
             expect(await strategy(1)).to.equal(0);
 
@@ -143,7 +271,7 @@ describe('BackoffStrategyFactory', (): void => {
             // @ts-ignore
             global.window = windowMock();
 
-            const strategy = BackoffStrategyFactory.jitteredBackoff(0, 100);
+            const strategy = BackoffStrategyFactory.jitteredBackoff(1, 100);
 
             expect(await strategy(1))
                 .to.be.at.least(0)
@@ -169,5 +297,115 @@ describe('BackoffStrategyFactory', (): void => {
             // @ts-ignore
             global.window = undefined;
         });
+
+        it('should throw error when setting minDelayMs to 0', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(0, 100, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('minDelayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting minDelayMs to <0', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(-1, 100, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('minDelayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting minDelayMs to a non-integer', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(0.1, 100, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('minDelayMs must be integer');
+            }
+        });
+
+        it('should throw error when setting minDelayMs to a non-safe integer', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(2 ** 53, 100, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('minDelayMs must be less than or equal to 2^53 - 1');
+            }
+        });
+
+        it('should throw error when setting maxDelayMs to 0', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(1, 0, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('maxDelayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting maxDelayMs to <0', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(1, -1, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('maxDelayMs must be greater than 0');
+            }
+        });
+
+        it('should throw error when setting maxDelayMs to a non-integer', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(1, 0.1, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('maxDelayMs must be integer');
+            }
+        });
+
+        it('should throw error when setting maxDelayMs to a non-safe integer', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(1, 2 ** 53, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('maxDelayMs must be less than or equal to 2^53 - 1');
+            }
+        });
+
+        it('should throw error when setting minDelayMs = maxDelayMs', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(1, 1, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('maxDelayMs must be greater than minDelayMs');
+            }
+        });
+
+        it('should throw error when setting minDelayMs > maxDelayMs', (): void => {
+            const entropyProvider = new NodeJsEntropyProvider();
+            const randomGenerator = new RandomGenerator(entropyProvider);
+
+            try {
+                BackoffStrategyFactory.jitteredBackoff(2, 1, false, randomGenerator);
+            } catch (ex) {
+                expect((ex as Error).message).to.equal('maxDelayMs must be greater than minDelayMs');
+            }
+        });
     });
 });

test/specs/timeoutPolicy.test.ts

@@ -243,6 +243,17 @@ describe('TimeoutPolicy', (): void => {
         }
     });
 
+    it('should throw error when setting timeoutAfter to 0', (): void => {
+        const policy = new TimeoutPolicy<void>();
+
+        try {
+            policy.timeoutAfter(0);
+            expect.fail('did not throw');
+        } catch (ex) {
+            expect((ex as Error).message).to.equal('timeoutMs must be greater than 0');
+        }
+    });
+
     it('should throw error when setting timeoutAfter to <0', (): void => {
         const policy = new TimeoutPolicy<void>();