Skip to content

Commit 2bce0e3

Browse files
benleshcartant
andcommitted
fix: errors thrown from iterables now properly propagated (#5444)
* fix: errors thrown from iterables now properly propagated * chore: fix lint failures Co-authored-by: Nicholas Jamieson <nicholas@cartant.com>
1 parent 796c889 commit 2bce0e3

File tree

3 files changed

+63
-13
lines changed

3 files changed

+63
-13
lines changed

spec/operators/mergeMap-spec.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
11
import { expect } from 'chai';
2-
import { mergeMap, map } from 'rxjs/operators';
2+
import { mergeMap, map, delay } from 'rxjs/operators';
33
import { asapScheduler, defer, Observable, from, of, timer } from 'rxjs';
44
import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing';
55
import { asInteropObservable } from '../helpers/interop-helper';
6+
import { TestScheduler } from 'rxjs/testing';
7+
import { observableMatcher } from '../helpers/observableMatcher';
68

7-
declare const type: Function;
89
declare const asDiagram: Function;
910

1011
/** @test {mergeMap} */
1112
describe('mergeMap', () => {
13+
let rxTest: TestScheduler;
14+
15+
// TODO: Convert the rest of these tests to use run mode!
16+
beforeEach(() => {
17+
rxTest = new TestScheduler(observableMatcher);
18+
});
19+
1220
asDiagram('mergeMap(i => 10*i\u2014\u201410*i\u2014\u201410*i\u2014| )')
1321
('should map-and-flatten each item to an Observable', () => {
1422
const e1 = hot('--1-----3--5-------|');
@@ -821,14 +829,27 @@ describe('mergeMap', () => {
821829
}, 0);
822830
});
823831

824-
type('should support type signatures', () => {
825-
let o: Observable<number>;
826-
827-
/* tslint:disable:no-unused-variable */
828-
let a1: Observable<string> = o.pipe(mergeMap(x => x.toString()));
829-
let a2: Observable<string> = o.pipe(mergeMap(x => x.toString(), 3));
830-
let a3: Observable<{ o: number; i: string; }> = o.pipe(mergeMap(x => x.toString(), (o, i) => ({ o, i })));
831-
let a4: Observable<{ o: number; i: string; }> = o.pipe(mergeMap(x => x.toString(), (o, i) => ({ o, i }), 3));
832-
/* tslint:enable:no-unused-variable */
832+
// NOTE: From https://github.com/ReactiveX/rxjs/issues/5436
833+
it('should properly handle errors from iterables that are processed after some async', () => {
834+
rxTest.run(({ cold, expectObservable }) => {
835+
const noXError = new Error('we do not allow x');
836+
const source = cold('-----A------------B-----|', { A: ['o', 'o', 'o'], B: ['o', 'x', 'o']});
837+
const expected = ' -----(ooo)--------(o#)';
838+
const iterable = function* (data: string[]) {
839+
for (let d of data) {
840+
if (d === 'x') {
841+
throw noXError;
842+
}
843+
yield d;
844+
}
845+
};
846+
const result = source.pipe(
847+
mergeMap(x => of(x).pipe(
848+
delay(0),
849+
mergeMap(iterable)
850+
))
851+
);
852+
expectObservable(result).toBe(expected, undefined, noXError);
853+
});
833854
});
834855
});

spec/util/subscribeToResult-spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,28 @@ describe('subscribeToResult', () => {
119119
expect(expected).to.be.equal(42);
120120
});
121121

122+
// NOTE: From https://github.com/ReactiveX/rxjs/issues/5436
123+
it('should pass along errors from an iterable', () => {
124+
const generator = function* () {
125+
yield 1;
126+
yield 2;
127+
yield 3;
128+
throw 'bad';
129+
};
130+
131+
const results: any[] = [];
132+
let foundError: any = null;
133+
134+
const subscriber = new OuterSubscriber({
135+
next: x => results.push(x),
136+
error: err => foundError = err
137+
});
138+
139+
subscribeToResult(subscriber, generator());
140+
expect(results).to.deep.equal([1, 2, 3]);
141+
expect(foundError).to.equal('bad');
142+
});
143+
122144
it('should subscribe to to an object that implements Symbol.observable', (done) => {
123145
const observableSymbolObject = { [$$symbolObservable]: () => of(42) };
124146

src/internal/util/subscribeToIterable.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,16 @@ import { Subscriber } from '../Subscriber';
22
import { iterator as Symbol_iterator } from '../symbol/iterator';
33

44
export const subscribeToIterable = <T>(iterable: Iterable<T>) => (subscriber: Subscriber<T>) => {
5-
const iterator = iterable[Symbol_iterator]();
5+
const iterator = (iterable as any)[Symbol_iterator]();
6+
67
do {
7-
const item = iterator.next();
8+
let item: IteratorResult<T>;
9+
try {
10+
item = iterator.next();
11+
} catch (err) {
12+
subscriber.error(err);
13+
return;
14+
}
815
if (item.done) {
916
subscriber.complete();
1017
break;

0 commit comments

Comments
 (0)