Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Observable): unhandled errors are now reported to HostReportErrors #3062

Merged
merged 1 commit into from
Nov 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
322 changes: 117 additions & 205 deletions spec/Observable-spec.ts

Large diffs are not rendered by default.

26 changes: 0 additions & 26 deletions spec/Subscriber-spec.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,10 @@
import { expect } from 'chai';
import * as sinon from 'sinon';
import * as Rx from '../src/Rx';

const Subscriber = Rx.Subscriber;

/** @test {Subscriber} */
describe('Subscriber', () => {
describe('when created through create()', () => {
it('should not call error() if next() handler throws an error', () => {
const errorSpy = sinon.spy();
const completeSpy = sinon.spy();

const subscriber = Subscriber.create(
(value: any) => {
if (value === 2) {
throw 'error!';
}
},
errorSpy,
completeSpy
);

subscriber.next(1);
expect(() => {
subscriber.next(2);
}).to.throw('error!');

expect(errorSpy).not.have.been.called;
expect(completeSpy).not.have.been.called;
});
});

it('should ignore next messages after unsubscription', () => {
let times = 0;

Expand Down
28 changes: 16 additions & 12 deletions spec/observables/dom/ajax-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('Observable.ajax', () => {
expect(axObjectStub).to.have.been.called;
});

it('should throw if not able to create XMLHttpRequest', () => {
it('should raise an error if not able to create XMLHttpRequest', () => {
root.XMLHttpRequest = null;
root.ActiveXObject = null;

Expand All @@ -66,9 +66,7 @@ describe('Observable.ajax', () => {
method: ''
};

expect(() => {
Rx.Observable.ajax(obj).subscribe();
}).to.throw();
Rx.Observable.ajax(obj).subscribe(null, err => expect(err).to.exist);
});

it('should create XMLHttpRequest for CORS', () => {
Expand Down Expand Up @@ -100,7 +98,7 @@ describe('Observable.ajax', () => {
expect(xDomainStub).to.have.been.called;
});

it('should throw if not able to create CORS request', () => {
it('should raise an error if not able to create CORS request', () => {
root.XMLHttpRequest = null;
root.XDomainRequest = null;

Expand All @@ -111,9 +109,7 @@ describe('Observable.ajax', () => {
withCredentials: true
};

expect(() => {
Rx.Observable.ajax(obj).subscribe();
}).to.throw();
Rx.Observable.ajax(obj).subscribe(null, err => expect(err).to.exist);
});

it('should set headers', () => {
Expand Down Expand Up @@ -805,7 +801,7 @@ describe('Observable.ajax', () => {

it('should work fine when XMLHttpRequest ontimeout property is monkey patched', function() {
Object.defineProperty(root.XMLHttpRequest.prototype, 'ontimeout', {
set: function (fn: (e: ProgressEvent) => any) {
set(fn: (e: ProgressEvent) => any) {
const wrapFn = (ev: ProgressEvent) => {
const result = fn.call(this, ev);
if (result === false) {
Expand All @@ -825,7 +821,11 @@ describe('Observable.ajax', () => {
};

Rx.Observable.ajax(ajaxRequest)
.subscribe();
.subscribe({
error(err) {
/* expected, ignore */
}
});

const request = MockXMLHttpRequest.mostRecent;
try {
Expand Down Expand Up @@ -881,7 +881,7 @@ describe('Observable.ajax', () => {

it('should work fine when XMLHttpRequest onerror property is monkey patched', function() {
Object.defineProperty(root.XMLHttpRequest.prototype, 'onerror', {
set: function (fn: (e: ProgressEvent) => any) {
set(fn: (e: ProgressEvent) => any) {
const wrapFn = (ev: ProgressEvent) => {
const result = fn.call(this, ev);
if (result === false) {
Expand All @@ -899,7 +899,11 @@ describe('Observable.ajax', () => {
Rx.Observable.ajax({
url: '/flibbertyJibbet'
})
.subscribe();
.subscribe({
error(err) {
/* expected */
}
});

const request = MockXMLHttpRequest.mostRecent;

Expand Down
54 changes: 1 addition & 53 deletions spec/observables/from-promise-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,56 +155,4 @@ describe('Observable.fromPromise', () => {
done();
});
});

if (typeof process === 'object' && Object.prototype.toString.call(process) === '[object process]') {
it('should globally throw unhandled errors on process', (done: MochaDone) => {
const originalException = process.listeners('uncaughtException');
process.removeAllListeners('uncaughtException');
process.once('uncaughtException', function (error) {
expect(error).to.be.an('error', 'fail');
originalException.forEach(l => process.addListener('uncaughtException', l));
done();
});

Observable.fromPromise(Promise.reject('bad'))
.subscribe(
(x: any) => { done(new Error('should not be called')); },
(e: any) => {
expect(e).to.equal('bad');
throw new Error('fail');
}, () => {
done(new Error('should not be called'));
});
});
} else if (typeof window === 'object' &&
(Object.prototype.toString.call(window) === '[object global]' || Object.prototype.toString.call(window) === '[object Window]')) {
it('should globally throw unhandled errors on window', (done: MochaDone) => {
const expected = ['Uncaught fail', 'fail', 'Script error.', 'uncaught exception: fail'];
const current = window.onerror;
window.onerror = null;

let invoked = false;
function onException(e) {
if (invoked) {
return;
}
invoked = true;
expect(expected).to.contain(e);
window.onerror = current;
done();
}

window.onerror = onException;

Observable.fromPromise(Promise.reject('bad'))
.subscribe(
(x: any) => { done(new Error('should not be called')); },
(e: any) => {
expect(e).to.equal('bad');
throw 'fail';
}, () => {
done(new Error('should not be called'));
});
});
}
});
});
5 changes: 3 additions & 2 deletions spec/observables/fromEvent-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,9 @@ describe('Observable.fromEvent', () => {
}
};

const subscribe = () => Observable.fromEvent(<any>obj, 'click').subscribe();
expect(subscribe).to.throw(TypeError, 'Invalid event target');
Observable.fromEvent(obj as any, 'click').subscribe({
error(err) { expect(err).to.deep.equal(new TypeError('Invalid event target')); }
});
});

it('should pass through options to addEventListener', () => {
Expand Down
38 changes: 0 additions & 38 deletions spec/operators/catch-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,44 +297,6 @@ describe('Observable.prototype.catch', () => {
sandbox.restore();
});

it('should chain a throw from a promise using throw', (done: MochaDone) => {
const subscribeSpy = sinon.spy();
const testError = new Error('BROKEN PROMISE');
Observable.fromPromise(Promise.reject(testError)).catch(err => {
throw new Error('BROKEN THROW');
}).subscribe(subscribeSpy);

trueSetTimeout(() => {
try {
timers.tick(1);
} catch (e) {
expect(subscribeSpy).not.to.be.called;
expect(e.message).to.equal('BROKEN THROW');
return done();
}
done(new Error('This should have thrown an error'));
}, 0);
});

it('should chain a throw from a promise using Observable.throw', (done: MochaDone) => {
const subscribeSpy = sinon.spy();
const testError = new Error('BROKEN PROMISE');
Observable.fromPromise(Promise.reject(testError)).catch(err =>
Observable.throw(new Error('BROKEN THROW'))
).subscribe(subscribeSpy);

trueSetTimeout(() => {
try {
timers.tick(1);
} catch (e) {
expect(subscribeSpy).not.to.be.called;
expect(e.message).to.equal('BROKEN THROW');
return done();
}
done(new Error('This should have thrown an error'));
}, 0);
});

it('should chain a throw from a promise using Observable.throw', (done: MochaDone) => {
const subscribeSpy = sinon.spy();
const errorSpy = sinon.spy();
Expand Down
9 changes: 5 additions & 4 deletions spec/operators/do-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const Subject = Rx.Subject;

/** @test {do} */
describe('Observable.prototype.do', () => {
asDiagram('do(x => console.log(x))')('should mirror multiple values and complete', () => {
asDiagram('do(x => console.log(x))')
('should mirror multiple values and complete', () => {
const e1 = cold('--1--2--3--|');
const e1subs = '^ !';
const expected = '--1--2--3--|';
Expand Down Expand Up @@ -68,8 +69,8 @@ describe('Observable.prototype.do', () => {

it('should handle everything with a Subject', (done: MochaDone) => {
const expected = [1, 2, 3];
const results = [];
const subject = new Subject();
const results: number[] = [];
const subject = new Subject<number>();

subject.subscribe({
next: (x: any) => {
Expand All @@ -85,7 +86,7 @@ describe('Observable.prototype.do', () => {
});

Observable.of(1, 2, 3)
.do(<Rx.Observer<number>>subject)
.do(subject)
.subscribe();
});

Expand Down
18 changes: 1 addition & 17 deletions spec/operators/map-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const Observable = Rx.Observable;
// function shortcuts
const addDrama = function (x) { return x + '!'; };
const identity = function (x) { return x; };
const throwError = function () { throw new Error(); };

/** @test {map} */
describe('Observable.prototype.map', () => {
Expand Down Expand Up @@ -89,16 +88,6 @@ describe('Observable.prototype.map', () => {
expectSubscriptions(a.subscriptions).toBe(asubs);
});

it('should propagate errors from subscribe', () => {
const r = () => {
Observable.of(1)
.map(identity)
.subscribe(throwError);
};

expect(r).to.throw();
});

it('should not map an empty observable', () => {
const a = cold('|');
const asubs = '(^!)';
Expand Down Expand Up @@ -187,19 +176,14 @@ describe('Observable.prototype.map', () => {
const expected = '--a--b---c----d--|';
const values = {a: 5, b: 14, c: 23, d: 32};

let invoked = 0;
const foo = {
value: 42
};
const r = a
.map(function (x: string, index: number) {
invoked++;
expect(this).to.equal(foo);
return (parseInt(x) + 1) + (index * 10);
}, foo)
.do(null, null, () => {
expect(invoked).to.equal(4);
});
}, foo);

expectObservable(r).toBe(expected, values);
expectSubscriptions(a.subscriptions).toBe(asubs);
Expand Down
15 changes: 1 addition & 14 deletions spec/operators/mapTo-spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect } from 'chai';

import * as Rx from '../../src/Rx';
import marbleTestingSignature = require('../helpers/marble-testing'); // tslint:disable-line:no-require-imports

Expand All @@ -10,9 +10,6 @@ declare const expectSubscriptions: typeof marbleTestingSignature.expectSubscript

const Observable = Rx.Observable;

// function shortcuts
const throwError = function () { throw new Error(); };

/** @test {mapTo} */
describe('Observable.prototype.mapTo', () => {
asDiagram('mapTo(\'a\')')('should map multiple values', () => {
Expand Down Expand Up @@ -61,16 +58,6 @@ describe('Observable.prototype.mapTo', () => {
expectSubscriptions(a.subscriptions).toBe(asubs);
});

it('should propagate errors from subscribe', () => {
const r = () => {
Observable.of(1)
.mapTo(-1)
.subscribe(throwError);
};

expect(r).to.throw();
});

it('should not map an empty observable', () => {
const a = cold('|');
const asubs = '(^!)';
Expand Down
14 changes: 1 addition & 13 deletions spec/util/subscribeToResult-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { subscribeToResult } from '../../src/util/subscribeToResult';
import { OuterSubscriber } from '../../src/OuterSubscriber';
import { $$iterator } from '../../src/symbol/iterator';
import $$symbolObservable from 'symbol-observable';
import { Observable } from '../../src/Observable';
import { Subject } from '../../src/Subject';

describe('subscribeToResult', () => {
it('should synchronously complete when subscribe to scalarObservable', () => {
Expand Down Expand Up @@ -180,14 +178,4 @@ describe('subscribeToResult', () => {

subscribeToResult(subscriber, null);
});

it('should not swallow exception in inner subscriber', () => {
const source = new Subject();

source.mergeMapTo(Observable.of(1, 2, 3)).subscribe(() => {
throw new Error('meh');
});

expect(() => source.next()).to.throw();
});
});
});
Loading