From 0288d2c0035e2ec5290b38683ef1431c85a1aef0 Mon Sep 17 00:00:00 2001 From: yisraelx Date: Tue, 27 Feb 2018 20:25:29 +0200 Subject: [PATCH] fix(retry): change times default and preventing memory leakage and handling error options * change the default value of times from 1 to 'Infinity'. * preventing memory leakage in the N call. * error handling of options timer, filter and interval when using functions. --- modules/-constructor/index.ts | 1 + modules/-constructor/package.json | 1 + modules/-prototype/index.ts | 1 - modules/-prototype/package.json | 1 - modules/retry/index.ts | 62 +++++++++++++++------------- modules/retry/package.json | 1 - test/retry.spec.ts | 67 ++++++++++++++++++++++++++++--- 7 files changed, 98 insertions(+), 36 deletions(-) diff --git a/modules/-constructor/index.ts b/modules/-constructor/index.ts index ec6ae64..93fef86 100644 --- a/modules/-constructor/index.ts +++ b/modules/-constructor/index.ts @@ -12,6 +12,7 @@ export { default as exec } from '@promises/exec'; export { default as isPromise } from '@promises/is-promise'; export { default as promisify, IPromisifyOptions } from '@promises/promisify'; export { default as parallel } from '@promises/parallel'; +export { default as retry, IRetryOptions, IRetryFilterInfo, IRetryTimeInfo } from '@promises/retry'; export { default as series } from '@promises/series'; export { default as timeout } from '@promises/timeout'; export { default as timesParallel } from '@promises/times-parallel'; diff --git a/modules/-constructor/package.json b/modules/-constructor/package.json index e02df09..e6e30b6 100644 --- a/modules/-constructor/package.json +++ b/modules/-constructor/package.json @@ -41,6 +41,7 @@ "@promises/is-promise": "^0.2.0", "@promises/parallel": "^0.2.0", "@promises/promisify": "^0.2.0", + "@promises/retry": "^0.2.0", "@promises/series": "^0.2.0", "@promises/timeout": "^0.2.0", "@promises/times-parallel": "^0.2.0", diff --git a/modules/-prototype/index.ts b/modules/-prototype/index.ts index eb24e01..2ef1b4d 100644 --- a/modules/-prototype/index.ts +++ b/modules/-prototype/index.ts @@ -23,7 +23,6 @@ export { default as reduceRightSeries } from '@promises/reduce-right-series'; export { default as rejectParallel } from '@promises/reject-parallel'; export { default as rejectSeries } from '@promises/reject-series'; export { default as reset } from '@promises/reset'; -export { default as retry, IRetryOptions, IRetryFilterInfo, IRetryTimeInfo } from '@promises/retry'; export { default as sleep } from '@promises/sleep'; export { default as someParallel } from '@promises/some-parallel'; export { default as someSeries } from '@promises/some-series'; diff --git a/modules/-prototype/package.json b/modules/-prototype/package.json index 99028bb..5ff3b9e 100644 --- a/modules/-prototype/package.json +++ b/modules/-prototype/package.json @@ -52,7 +52,6 @@ "@promises/reject-parallel": "^0.2.0", "@promises/reject-series": "^0.2.0", "@promises/reset": "^0.2.0", - "@promises/retry": "^0.2.0", "@promises/sleep": "^0.2.0", "@promises/some-parallel": "^0.2.0", "@promises/some-series": "^0.2.0", diff --git a/modules/retry/index.ts b/modules/retry/index.ts index 28ad0a7..d16a8ac 100644 --- a/modules/retry/index.ts +++ b/modules/retry/index.ts @@ -6,8 +6,7 @@ import Promises from '@promises/core'; import exec from '@promises/exec'; -import timeout from '@promises/timeout'; -import '@promises/timer'; +import _timer from '@promises/timer'; import { IOptionalPromise } from '@promises/interfaces'; export interface IRetryOptions { @@ -33,40 +32,49 @@ export interface IRetryFilterInfo { * @example * * ```typescript - * let count = 0; - * let promises = retry(()=>{ - * if(count++ < 2) throw 'error'; + * let count: number = 0; + * + * let promise: Promise = retry(()=>{ + * if(count++ < 2) throw 'error'; * return 'foo'; * }, {times: 3}) * - * promises.then((result) => { + * promises.then((result: string) => { * console.log(result); // => 'foo' * }) * ``` */ function retryStatic(fn: () => IOptionalPromise, options?: IRetryOptions): Promises { - let { times = 1, interval, timer, filter = (error) => true } = options || {} as any; - return retryHelper(fn, { times, interval, timer, filter, counter: 1 }) as Promises; -} + return new Promises((resolve, reject) => { + let {times = Infinity, interval, timer, filter = (error) => true} = options || {} as any; + let counter: number = 0; + let lastTimer: number, lastInterval: number; -function retryHelper(fn, options) { - let { counter, times, timer, interval, filter, lastTimer, lastInterval } = options; - let step = exec(fn); - if (timer != null) { - lastTimer = typeof timer === 'function' ? timer({ counter, last: lastTimer, times }) : timer; - step = step.timer(lastTimer); - } + let next = () => { + counter++; + let step: Promises = exec(fn); + + if (timer != null) { + try { + lastTimer = typeof timer === 'function' ? timer({counter, last: lastTimer, times}) : timer; + } catch (error) { + return reject(error); + } + step = _timer(step, lastTimer); + } + + step.then(resolve, (error: any) => { + if ((times === counter) || !filter({error, times, counter})) { + throw error; + } + + lastInterval = typeof interval === 'function' ? interval({counter, last: lastInterval, times}) : interval; + setTimeout(next, lastInterval); + }).catch(reject); + }; - step = step.catch((error) => { - if ((times === counter) || !filter({ error, times, counter })) throw error; - lastInterval = typeof interval === 'function' ? interval({ counter, last: lastInterval, times }) : interval; - counter++; - return timeout((resolve) => { - let result = retryHelper(fn, { counter, times, timer, interval, filter, lastTimer, lastInterval }); - resolve(result); - }, lastInterval); + next(); }); - return step; } export default retryStatic; @@ -83,11 +91,11 @@ declare module '@promises/core' { * let promises = Promises.retry(()=>{ * if(count++ < 2) throw 'error'; * return 'foo'; - * }, {times: 3}) + * }, {times: 3}); * * promises.then((result) => { * console.log(result); // => 'foo' - * }) + * }); * ``` */ export let retry: typeof retryStatic; diff --git a/modules/retry/package.json b/modules/retry/package.json index ce8423b..e69285f 100644 --- a/modules/retry/package.json +++ b/modules/retry/package.json @@ -35,7 +35,6 @@ "@promises/core": "^0.2.0", "@promises/exec": "^0.2.0", "@promises/interfaces": "^0.2.0", - "@promises/timeout": "^0.2.0", "@promises/timer": "^0.2.0" } } diff --git a/test/retry.spec.ts b/test/retry.spec.ts index 7cff7f3..aa33327 100644 --- a/test/retry.spec.ts +++ b/test/retry.spec.ts @@ -2,19 +2,19 @@ import Promises from '@promises/core'; import retry from '@promises/retry'; describe('retry', () => { - it('should be try 1 time', () => { + it('should be try 1 time and reject', () => { let count = 0; return retry(() => { count++; throw 'error'; - }).catch((error) => { + }, { times: 1 }).catch((error) => { expect(error).toBe('error'); expect(count).toBe(1); }); }); - it('should be try 3 times', () => { + it('should be try 3 times and reject', () => { let times = 3; let count = 0; @@ -27,12 +27,25 @@ describe('retry', () => { }); }); + it('should be try without times and resolve', () => { + let count = 0; + + return retry(() => { + if (count++ < 5) { + throw 'error'; + } + return 'resolve'; + }).then((result) => { + expect(result).toBe('resolve'); + expect(count).toBe(6); + }); + }); it('should be interval prev function', () => { let startTime = Date.now(); let times = 5; let count = 0; - let interval = ({last}) => { + let interval = ({ last }) => { return (last || 1) * 2; }; return retry(() => { @@ -71,7 +84,7 @@ describe('retry', () => { it('should be timer prev function fail', () => { let times = 3; let count = 0; - let timer = ({counter}) => { + let timer = ({ counter }) => { return counter; }; return retry(() => { @@ -90,7 +103,7 @@ describe('retry', () => { it('should be filter error', () => { let times = 3; let count = 0; - let filter = ({error}) => { + let filter = ({ error }) => { return error === 'error'; }; return retry(() => { @@ -101,4 +114,46 @@ describe('retry', () => { expect(count).toBe(2); }); }); + + it('should be reject on timer error', () => { + let timer = () => { throw 'timer'; }; + let count: number = 0; + return retry(() => { + count++; + throw 'error'; + }, { timer, times: 2 }).then(() => { + throw 'resolve'; + }).catch((error) => { + expect(error).toBe('timer'); + expect(count).toBe(1); + }); + }); + + it('should be reject on filter error', () => { + let filter = () => { throw 'filter'; }; + let count: number = 0; + return retry(() => { + count++; + throw 'error'; + }, { filter, times: 2 }).then(() => { + throw 'resolve'; + }).catch((error) => { + expect(error).toBe('filter'); + expect(count).toBe(1); + }); + }); + + it('should be reject on interval error', () => { + let interval = () => { throw 'interval'; }; + let count: number = 0; + return retry(() => { + count++; + throw 'error'; + }, { interval, times: 2 }).then(() => { + throw 'resolve'; + }).catch((error) => { + expect(error).toBe('interval'); + expect(count).toBe(1); + }); + }); });