Skip to content

Commit

Permalink
fix(config.useDeprecatedSynchronousErrorThrowing): reentrant error th…
Browse files Browse the repository at this point in the history
…rowing no longer trapped (#3449)

- Applies a fix from #3161 to prevent some sync errors from being trapped
- Updates tests to not be so noisy when the flag is flipped
- Tests that flipping the flag will console.warn and console.log appropriately
  • Loading branch information
benlesh authored Mar 19, 2018
1 parent b25db9f commit 0892a2d
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 2 deletions.
50 changes: 49 additions & 1 deletion spec/Observable-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { Observer, TeardownLogic } from '../src/internal/types';
import { cold, expectObservable, expectSubscriptions } from './helpers/marble-testing';
import { map } from '../src/internal/operators/map';
import * as HostReportErrorModule from '../src/internal/util/hostReportError';
import { noop } from '../src/internal/util/noop';

//tslint:disable-next-line
require('./helpers/test-helper');

Expand Down Expand Up @@ -533,18 +535,64 @@ describe('Observable', () => {
});
});

describe('if config.useDeprecatedSynchronousThrowing === true', () => {
describe('config.useDeprecatedSynchronousErrorHandling', () => {

it('should log when it is set and unset', () => {
const _log = console.log;
const logCalledWith: any[][] = [];
console.log = (...args: any[]) => {
logCalledWith.push(args);
};

const _warn = console.warn;
const warnCalledWith: any[][] = [];
console.warn = (...args: any[]) => {
warnCalledWith.push(args);
};

Rx.config.useDeprecatedSynchronousErrorHandling = true;
expect(warnCalledWith.length).to.equal(1);

Rx.config.useDeprecatedSynchronousErrorHandling = false;
expect(logCalledWith.length).to.equal(1);

console.log = _log;
console.warn = _warn;
});
});

describe('if config.useDeprecatedSynchronousErrorHandling === true', () => {
beforeEach(() => {
const _warn = console.warn;
console.warn = noop;
Rx.config.useDeprecatedSynchronousErrorHandling = true;
console.warn = _warn;
});

it('should throw synchronously', () => {
expect(() => Observable.throwError(new Error()).subscribe())
.to.throw();
});

it('should rethrow if sink has syncErrorThrowable = false', () => {
const observable = new Observable(observer => {
observer.next(1);
});

const sink = Rx.Subscriber.create(() => {
throw 'error!';
});

expect(() => {
observable.subscribe(sink);
}).to.throw('error!');
});

afterEach(() => {
const _log = console.log;
console.log = noop;
Rx.config.useDeprecatedSynchronousErrorHandling = false;
console.log = _log;
});
});
});
Expand Down
4 changes: 4 additions & 0 deletions spec/operators/repeatWhen-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('Observable.prototype.repeatWhen', () => {
let retried = false;
const expected = [1, 2, 1, 2];
let i = 0;
try {
Observable.of(1, 2)
.map((n: number) => {
return n;
Expand All @@ -58,6 +59,9 @@ describe('Observable.prototype.repeatWhen', () => {
expect(err).to.be.an('error', 'done');
done();
});
} catch (err) {
done(err);
}
});

it('should retry when notified and complete on returned completion', (done: MochaDone) => {
Expand Down
2 changes: 1 addition & 1 deletion src/internal/Observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export class Observable<T> implements Subscribable<T> {
if (operator) {
operator.call(sink, this.source);
} else {
sink.add(this.source ? this._subscribe(sink) : this._trySubscribe(sink));
sink.add(this.source || !sink.syncErrorThrowable ? this._subscribe(sink) : this._trySubscribe(sink));
}

if (config.useDeprecatedSynchronousErrorHandling) {
Expand Down
1 change: 1 addition & 0 deletions src/internal/Subscriber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export class Subscriber<T> extends Subscription implements Observer<T> {
}
if (typeof destinationOrNext === 'object') {
if (destinationOrNext instanceof Subscriber) {
this.syncErrorThrowable = destinationOrNext.syncErrorThrowable;
this.destination = (<Subscriber<any>> destinationOrNext);
(<any> this.destination).add(this);
} else {
Expand Down

0 comments on commit 0892a2d

Please sign in to comment.