From 27d3ece85dc70e55d4439a306cc0a2e61aa25275 Mon Sep 17 00:00:00 2001 From: ommizrah Date: Wed, 3 Apr 2019 18:34:36 +0300 Subject: [PATCH 1/5] feat(retry): add config to reset error count on successful emission This PR adds the ability to reset the error counter on successful emissions using the `retry` operator. The current behavior for `retry(n)` is to call error if n errors occurred, regardless of whether or not they were consecutive. Now one would be able to use `retry(n, true)` to have the count reset so that only n consecutive errors will cause the observable to fail. --- package-lock.json | 6 +-- spec/Observable-spec.ts | 5 +- spec/operators/retry-spec.ts | 84 +++++++++++++++++++++++++++++++-- src/internal/operators/retry.ts | 25 ++++++++-- 4 files changed, 108 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6ae2946c0b..05a7dead58 100644 --- a/package-lock.json +++ b/package-lock.json @@ -170,9 +170,9 @@ "dev": true }, "@types/mocha": { - "version": "2.2.48", - "resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-2.2.48.tgz", - "integrity": "sha512-nlK/iyETgafGli8Zh9zJVCTicvU3iajSkRwOh3Hhiva598CMqNJ4NcVCGMTGKpGpTYj/9R8RLzS9NAykSSCqGw==", + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-5.0.0.tgz", + "integrity": "sha512-ZS0vBV7Jn5Z/Q4T3VXauEKMDCV8nWOtJJg90OsDylkYJiQwcWtKuLzohWzrthBkerUF7DLMmJcwOPEP0i/AOXw==", "dev": true }, "@types/node": { diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts index 7790aa9c8d..2930dfdbd5 100644 --- a/spec/Observable-spec.ts +++ b/spec/Observable-spec.ts @@ -18,7 +18,10 @@ function expectFullObserver(val: any) { /** @test {Observable} */ describe('Observable', () => { let originalConfigPromise: any; - before(() => originalConfigPromise = config.Promise); + before(() => { + originalConfigPromise = config.Promise; + } + ); after(() => { config.Promise = originalConfigPromise; diff --git a/spec/operators/retry-spec.ts b/spec/operators/retry-spec.ts index 87c640c9e6..8f2aaa6ed1 100644 --- a/spec/operators/retry-spec.ts +++ b/spec/operators/retry-spec.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing'; import { retry, map, take, mergeMap, concat, multicast, refCount } from 'rxjs/operators'; -import { Observable, Observer, of, throwError, Subject } from 'rxjs'; +import { Observable, Observer, defer, range, of, throwError, Subject } from 'rxjs'; declare function asDiagram(arg: string): Function; @@ -58,16 +58,94 @@ describe('retry operator', () => { retry(retries - 1) ).subscribe( (x: number) => { - expect(x).to.equal(42); + done("shouldn't next"); }, (err: any) => { expect(errors).to.equal(2); done(); }, () => { - expect('this was called').to.be.true; + done("shouldn't complete"); }); }); + it('should retry a number of times, then call error handler (with resetOnSuccess)', (done: MochaDone) => { + let errors = 0; + const retries = 2; + Observable.create((observer: Observer) => { + observer.next(42); + observer.complete(); + }).pipe( + map((x: any) => { + errors += 1; + throw 'bad'; + }), + retry(retries - 1, true) + ).subscribe( + (x: number) => { + done("shouldn't next"); + }, + (err: any) => { + expect(errors).to.equal(2); + done(); + }, () => { + done("shouldn't complete"); + }); + }); + + it('should retry a number of times, then call next handler without error, then retry and complete', (done: MochaDone) => { + let index = 0; + let errors = 0; + const retries = 2; + defer(() => range(0, 4 - index)).pipe( + mergeMap(() => { + index++; + if (index === 1 || index === 3) { + errors++; + return throwError('bad'); + } else { + return of(42); + } + }), + retry(retries - 1, true) + ).subscribe( + (x: number) => { + expect(x).to.equal(42); + }, + (err: any) => { + done("shouldn't error"); + }, () => { + expect(errors).to.equal(retries); + done(); + }); + }); + + it('should retry a number of times, then call next handler without error, then retry and error', (done: MochaDone) => { + let index = 0; + let errors = 0; + const retries = 2; + defer(() => range(0, 4 - index)).pipe( + mergeMap(() => { + index++; + if (index === 1 || index === 3) { + errors++; + return throwError('bad'); + } else { + return of(42); + } + }), + retry(retries - 1, false) + ).subscribe( + (x: number) => { + expect(x).to.equal(42); + }, + (err: any) => { + expect(errors).to.equal(retries); + done(); + }, () => { + done("shouldn't complete"); + }); + }); + it('should retry until successful completion', (done: MochaDone) => { let errors = 0; const retries = 10; diff --git a/src/internal/operators/retry.ts b/src/internal/operators/retry.ts index a6ce566ebf..6c517882c3 100644 --- a/src/internal/operators/retry.ts +++ b/src/internal/operators/retry.ts @@ -46,21 +46,23 @@ import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; * ``` * * @param {number} count - Number of retry attempts before failing. + * @param {boolean} resetOnSuccess - When set to `true` every successful emission will reset the error count * @return {Observable} The source Observable modified with the retry logic. * @method retry * @owner Observable */ -export function retry(count: number = -1): MonoTypeOperatorFunction { - return (source: Observable) => source.lift(new RetryOperator(count, source)); +export function retry(count: number = -1, resetOnSuccess = false): MonoTypeOperatorFunction { + return (source: Observable) => source.lift(new RetryOperator(count, source, resetOnSuccess)); } class RetryOperator implements Operator { constructor(private count: number, - private source: Observable) { + private source: Observable, + private resetOnSuccess: boolean) { } call(subscriber: Subscriber, source: any): TeardownLogic { - return source.subscribe(new RetrySubscriber(subscriber, this.count, this.source)); + return source.subscribe(new RetrySubscriber(subscriber, this.count, this.source, this.resetOnSuccess)); } } @@ -70,11 +72,24 @@ class RetryOperator implements Operator { * @extends {Ignored} */ class RetrySubscriber extends Subscriber { + private readonly initialCount: number; + constructor(destination: Subscriber, private count: number, - private source: Observable) { + private source: Observable, + private resetOnSuccess: boolean + ) { super(destination); + this.initialCount = this.count; } + + next(value?: T): void { + super.next(value); + if (this.resetOnSuccess) { + this.count = this.initialCount; + } + } + error(err: any) { if (!this.isStopped) { const { source, count } = this; From d1ed91d55d241ce119736a618aac43328ea029e4 Mon Sep 17 00:00:00 2001 From: ommizrah Date: Sun, 12 May 2019 11:29:41 +0300 Subject: [PATCH 2/5] feat(retry): add config parameter added overloaded signature to the `retry` operator that accepts a config object --- spec/operators/retry-spec.ts | 6 +++--- src/internal/operators/retry.ts | 29 ++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/spec/operators/retry-spec.ts b/spec/operators/retry-spec.ts index 8f2aaa6ed1..353d6f3f7c 100644 --- a/spec/operators/retry-spec.ts +++ b/spec/operators/retry-spec.ts @@ -79,7 +79,7 @@ describe('retry operator', () => { errors += 1; throw 'bad'; }), - retry(retries - 1, true) + retry({count: retries - 1, resetOnSuccess: true}) ).subscribe( (x: number) => { done("shouldn't next"); @@ -106,7 +106,7 @@ describe('retry operator', () => { return of(42); } }), - retry(retries - 1, true) + retry({count: retries - 1, resetOnSuccess: true}) ).subscribe( (x: number) => { expect(x).to.equal(42); @@ -133,7 +133,7 @@ describe('retry operator', () => { return of(42); } }), - retry(retries - 1, false) + retry({count: retries - 1, resetOnSuccess: false}) ).subscribe( (x: number) => { expect(x).to.equal(42); diff --git a/src/internal/operators/retry.ts b/src/internal/operators/retry.ts index 6c517882c3..5ca47e95eb 100644 --- a/src/internal/operators/retry.ts +++ b/src/internal/operators/retry.ts @@ -4,6 +4,11 @@ import { Observable } from '../Observable'; import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; +export interface RetryConfig { + count: number; + resetOnSuccess?: boolean; +} + /** * Returns an Observable that mirrors the source Observable with the exception of an `error`. If the source Observable * calls `error`, this method will resubscribe to the source Observable for a maximum of `count` resubscriptions (given @@ -51,18 +56,28 @@ import { MonoTypeOperatorFunction, TeardownLogic } from '../types'; * @method retry * @owner Observable */ -export function retry(count: number = -1, resetOnSuccess = false): MonoTypeOperatorFunction { - return (source: Observable) => source.lift(new RetryOperator(count, source, resetOnSuccess)); +export function retry(count?: number): MonoTypeOperatorFunction; +export function retry(config: RetryConfig): MonoTypeOperatorFunction; +export function retry(configOrCount: number | RetryConfig = -1): MonoTypeOperatorFunction { + let config: RetryConfig; + if (configOrCount && typeof configOrCount === 'object') { + config = configOrCount as RetryConfig; + } else { + config = { + count: configOrCount as number + }; + } + return (source: Observable) => source.lift(new RetryOperator(config.count, config.resetOnSuccess, source)); } class RetryOperator implements Operator { constructor(private count: number, - private source: Observable, - private resetOnSuccess: boolean) { + private resetOnSuccess: boolean, + private source: Observable) { } call(subscriber: Subscriber, source: any): TeardownLogic { - return source.subscribe(new RetrySubscriber(subscriber, this.count, this.source, this.resetOnSuccess)); + return source.subscribe(new RetrySubscriber(subscriber, this.count, this.resetOnSuccess, this.source)); } } @@ -76,8 +91,8 @@ class RetrySubscriber extends Subscriber { constructor(destination: Subscriber, private count: number, - private source: Observable, - private resetOnSuccess: boolean + private resetOnSuccess: boolean, + private source: Observable ) { super(destination); this.initialCount = this.count; From 6329895f6ef782f9fbea9e6450317bf5e383f512 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Thu, 30 Jan 2020 19:37:55 +1000 Subject: [PATCH 3/5] chore: restore package-lock.json --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 05a7dead58..6ae2946c0b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -170,9 +170,9 @@ "dev": true }, "@types/mocha": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-5.0.0.tgz", - "integrity": "sha512-ZS0vBV7Jn5Z/Q4T3VXauEKMDCV8nWOtJJg90OsDylkYJiQwcWtKuLzohWzrthBkerUF7DLMmJcwOPEP0i/AOXw==", + "version": "2.2.48", + "resolved": "https://registry.npmjs.org/@types/mocha/-/mocha-2.2.48.tgz", + "integrity": "sha512-nlK/iyETgafGli8Zh9zJVCTicvU3iajSkRwOh3Hhiva598CMqNJ4NcVCGMTGKpGpTYj/9R8RLzS9NAykSSCqGw==", "dev": true }, "@types/node": { From 7b3754eaa1a1742eaf8e4a9c91deabea78c1af64 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Thu, 30 Jan 2020 19:44:44 +1000 Subject: [PATCH 4/5] chore: appease TypeScript --- src/internal/operators/retry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/operators/retry.ts b/src/internal/operators/retry.ts index 5ca47e95eb..0252ae5dab 100644 --- a/src/internal/operators/retry.ts +++ b/src/internal/operators/retry.ts @@ -67,7 +67,7 @@ export function retry(configOrCount: number | RetryConfig = -1): MonoTypeOper count: configOrCount as number }; } - return (source: Observable) => source.lift(new RetryOperator(config.count, config.resetOnSuccess, source)); + return (source: Observable) => source.lift(new RetryOperator(config.count, !!config.resetOnSuccess, source)); } class RetryOperator implements Operator { From ee7eab0d1ed4b0967dad76911cf0321c89edde91 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Thu, 30 Jan 2020 19:52:57 +1000 Subject: [PATCH 5/5] chore: revert change to Observable spec --- spec/Observable-spec.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts index 2930dfdbd5..7790aa9c8d 100644 --- a/spec/Observable-spec.ts +++ b/spec/Observable-spec.ts @@ -18,10 +18,7 @@ function expectFullObserver(val: any) { /** @test {Observable} */ describe('Observable', () => { let originalConfigPromise: any; - before(() => { - originalConfigPromise = config.Promise; - } - ); + before(() => originalConfigPromise = config.Promise); after(() => { config.Promise = originalConfigPromise;