Skip to content

Commit

Permalink
feat(timeout): remove errorToSend argument, always throw TimeoutErr…
Browse files Browse the repository at this point in the history
…or (#2172)

The `errorToSend` argument did not make much sense, so we are removing it ahead of the 5.0 release. All timeout errors will be thrown as `TimeoutError`, which can be tested with an assertion against `instanceof Rx.TimeoutError`.

BREAKING CHANGE: `timeout` no longer accepts the `errorToSend` argument

related #2141
  • Loading branch information
benlesh authored Dec 7, 2016
1 parent 5f2e849 commit 98ea3d2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 57 deletions.
64 changes: 20 additions & 44 deletions spec/operators/timeout-spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as Rx from '../../dist/cjs/Rx';
import { expect } from 'chai';
declare const {hot, cold, asDiagram, expectObservable, expectSubscriptions};

declare const rxTestScheduler: Rx.TestScheduler;
Expand All @@ -13,22 +14,23 @@ describe('Observable.prototype.timeout', () => {
const e1subs = '^ ! ';
const expected = '-----# ';

const result = e1.timeout(50, null, rxTestScheduler);
const result = e1.timeout(50, rxTestScheduler);

expectObservable(result).toBe(expected, null, defaultTimeoutError);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should timeout after specified timeout period and send the passed error', () => {
const e1 = cold('-');
const e1subs = '^ !';
const expected = '-----#';
const value = 'hello';

const result = e1.timeout(50, value, rxTestScheduler);

expectObservable(result).toBe(expected, null, value);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
it('should emit and error of an instanceof TimeoutError on timeout', () => {
const e1 = cold('-------a--b--|');
const result = e1.timeout(50, rxTestScheduler);
result.subscribe(() => {
throw new Error('this should not next');
}, err => {
expect(err).to.be.an.instanceof(Rx.TimeoutError);
}, () => {
throw new Error('this should not complete');
});
rxTestScheduler.flush();
});

it('should not timeout if source completes within absolute timeout period', () => {
Expand All @@ -38,7 +40,7 @@ describe('Observable.prototype.timeout', () => {

const timeoutValue = new Date(rxTestScheduler.now() + (expected.length + 2) * 10);

expectObservable(e1.timeout(timeoutValue, null, rxTestScheduler)).toBe(expected);
expectObservable(e1.timeout(timeoutValue, rxTestScheduler)).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

Expand All @@ -47,7 +49,7 @@ describe('Observable.prototype.timeout', () => {
const e1subs = '^ !';
const expected = '--a--b--c--d--e--|';

expectObservable(e1.timeout(50, null, rxTestScheduler)).toBe(expected);
expectObservable(e1.timeout(50, rxTestScheduler)).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

Expand All @@ -57,7 +59,7 @@ describe('Observable.prototype.timeout', () => {
const e1subs = '^ ! ';
const expected = '--a--b--c-- ';

const result = e1.timeout(50, null, rxTestScheduler);
const result = e1.timeout(50, rxTestScheduler);

expectObservable(result, unsub).toBe(expected);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
Expand All @@ -71,7 +73,7 @@ describe('Observable.prototype.timeout', () => {

const result = e1
.mergeMap((x: string) => Observable.of(x))
.timeout(50, null, rxTestScheduler)
.timeout(50, rxTestScheduler)
.mergeMap((x: string) => Observable.of(x));

expectObservable(result, unsub).toBe(expected);
Expand All @@ -85,31 +87,18 @@ describe('Observable.prototype.timeout', () => {
const expected = '---a---b---c----# ';
const values = {a: 'a', b: 'b', c: 'c'};

const result = e1.timeout(50, null, rxTestScheduler);
const result = e1.timeout(50, rxTestScheduler);

expectObservable(result).toBe(expected, values, defaultTimeoutError);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should timeout after a specified delay with passed error while source emits', () => {
const value = 'hello';
const e1 = hot('---a---b---c------d---e---|');
const e1subs = '^ ! ';
const expected = '---a---b---c----# ';
const values = {a: 'a', b: 'b', c: 'c'};

const result = e1.timeout(50, value, rxTestScheduler);

expectObservable(result).toBe(expected, values, value);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should timeout at a specified Date', () => {
const e1 = cold('-');
const e1subs = '^ !';
const expected = '----------#';

const result = e1.timeout(new Date(rxTestScheduler.now() + 100), null, rxTestScheduler);
const result = e1.timeout(new Date(rxTestScheduler.now() + 100), rxTestScheduler);

expectObservable(result).toBe(expected, null, defaultTimeoutError);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
Expand All @@ -121,22 +110,9 @@ describe('Observable.prototype.timeout', () => {
const expected = '--a--b--c-# ';
const values = {a: 'a', b: 'b', c: 'c'};

const result = e1.timeout(new Date(rxTestScheduler.now() + 100), null, rxTestScheduler);
const result = e1.timeout(new Date(rxTestScheduler.now() + 100), rxTestScheduler);

expectObservable(result).toBe(expected, values, defaultTimeoutError);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});

it('should timeout specified Date with passed error while source emits', () => {
const value = 'hello';
const e1 = hot('--a--b--c--d--e--|');
const e1subs = '^ ! ';
const expected = '--a--b--c-# ';
const values = {a: 'a', b: 'b', c: 'c'};

const result = e1.timeout(new Date(rxTestScheduler.now() + 100), value, rxTestScheduler);

expectObservable(result).toBe(expected, values, value);
expectSubscriptions(e1.subscriptions).toBe(e1subs);
});
});
24 changes: 11 additions & 13 deletions src/operator/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,30 @@ import { TeardownLogic } from '../Subscription';
import { TimeoutError } from '../util/TimeoutError';

/**
* @param due
* @param errorToSend
* @param scheduler
* @param {number} due
* @param {Scheduler} [scheduler]
* @return {Observable<R>|WebSocketSubject<T>|Observable<T>}
* @method timeout
* @owner Observable
*/
export function timeout<T>(this: Observable<T>, due: number | Date,
errorToSend: any = null,
export function timeout<T>(this: Observable<T>,
due: number | Date,
scheduler: Scheduler = async): Observable<T> {
const absoluteTimeout = isDate(due);
const waitFor = absoluteTimeout ? (+due - scheduler.now()) : Math.abs(<number>due);
const error = errorToSend || new TimeoutError();
return this.lift(new TimeoutOperator(waitFor, absoluteTimeout, error, scheduler));
return this.lift(new TimeoutOperator(waitFor, absoluteTimeout, scheduler, new TimeoutError()));
}

class TimeoutOperator<T> implements Operator<T, T> {
constructor(private waitFor: number,
private absoluteTimeout: boolean,
private errorToSend: any,
private scheduler: Scheduler) {
private scheduler: Scheduler,
private errorInstance: TimeoutError) {
}

call(subscriber: Subscriber<T>, source: any): TeardownLogic {
return source._subscribe(new TimeoutSubscriber<T>(
subscriber, this.absoluteTimeout, this.waitFor, this.errorToSend, this.scheduler
subscriber, this.absoluteTimeout, this.waitFor, this.scheduler, this.errorInstance
));
}
}
Expand All @@ -57,8 +55,8 @@ class TimeoutSubscriber<T> extends Subscriber<T> {
constructor(destination: Subscriber<T>,
private absoluteTimeout: boolean,
private waitFor: number,
private errorToSend: any,
private scheduler: Scheduler) {
private scheduler: Scheduler,
private errorInstance: TimeoutError) {
super(destination);
this.scheduleTimeout();
}
Expand Down Expand Up @@ -97,6 +95,6 @@ class TimeoutSubscriber<T> extends Subscriber<T> {
}

notifyTimeout(): void {
this.error(this.errorToSend);
this.error(this.errorInstance);
}
}

0 comments on commit 98ea3d2

Please sign in to comment.