Skip to content

Commit

Permalink
fix(zone.js): zone.js patches rxjs should check null for unsubscribe (a…
Browse files Browse the repository at this point in the history
…ngular#35990)

Close angular#31687, angular#31684

Zone.js patches rxjs internal `_subscribe` and `_unsubscribe` methods, but zone.js doesn't do null check, so in some operator such as `retryWhen`, the `_unsubscribe` will be set to null, and will cause
zone patched version throw error.

In this PR, if `_subscribe` and `_unsubscribe` is null, will not do the patch.

PR Close angular#35990
  • Loading branch information
JiaLiPassion authored and AndrewKushnir committed Mar 16, 2020
1 parent 5463462 commit 3fa8952
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 21 deletions.
53 changes: 32 additions & 21 deletions packages/zone.js/lib/rxjs/rxjs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,22 +50,28 @@ type ZoneSubscriberContext = {
},
set: function(this: Observable<any>, subscribe: any) {
(this as any)._zone = Zone.current;
(this as any)._zoneSubscribe = function(this: ZoneSubscriberContext) {
if (this._zone && this._zone !== Zone.current) {
const tearDown = this._zone.run(subscribe, this, arguments as any);
if (tearDown && typeof tearDown === 'function') {
const zone = this._zone;
return function(this: ZoneSubscriberContext) {
if (zone !== Zone.current) {
return zone.run(tearDown, this, arguments as any);
}
return tearDown.apply(this, arguments);
};
if (!subscribe) {
(this as any)._zoneSubscribe = subscribe;
} else {
(this as any)._zoneSubscribe = function(this: ZoneSubscriberContext) {
if (this._zone && this._zone !== Zone.current) {
const tearDown = this._zone.run(subscribe, this, arguments as any);
if (typeof tearDown === 'function') {
const zone = this._zone;
return function(this: ZoneSubscriberContext) {
if (zone !== Zone.current) {
return zone.run(tearDown, this, arguments as any);
}
return tearDown.apply(this, arguments);
};
} else {
return tearDown;
}
} else {
return subscribe.apply(this, arguments);
}
return tearDown;
}
return subscribe.apply(this, arguments);
};
};
}
}
},
subjectFactory: {
Expand Down Expand Up @@ -113,12 +119,17 @@ type ZoneSubscriberContext = {
},
set: function(this: Subscription, unsubscribe: any) {
(this as any)._zone = Zone.current;
(this as any)._zoneUnsubscribe = function() {
if (this._zone && this._zone !== Zone.current) {
return this._zone.run(unsubscribe, this, arguments);
}
return unsubscribe.apply(this, arguments);
};
if (!unsubscribe) {
(this as any)._zoneUnsubscribe = unsubscribe;
} else {
(this as any)._zoneUnsubscribe = function() {
if (this._zone && this._zone !== Zone.current) {
return this._zone.run(unsubscribe, this, arguments);
} else {
return unsubscribe.apply(this, arguments);
}
};
}
}
}
});
Expand Down
54 changes: 54 additions & 0 deletions packages/zone.js/test/rxjs/rxjs.Observable.retry.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Observable, of , timer} from 'rxjs';
import {delayWhen, map, retryWhen} from 'rxjs/operators';

describe('Observable.retryWhen', () => {
let log: any[];
let observable1: Observable<any>;
let defaultTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;

beforeEach(() => {
log = [];
jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000;
});

afterEach(() => { jasmine.DEFAULT_TIMEOUT_INTERVAL = defaultTimeout; });

it('retryWhen func callback should run in the correct zone', (done: DoneFn) => {
const constructorZone1: Zone = Zone.current.fork({name: 'Constructor Zone'});
const subscriptionZone: Zone = Zone.current.fork({name: 'Subscription Zone'});
let isErrorHandled = false;
observable1 = constructorZone1.run(() => {
return of (1, 2, 3).pipe(
map(v => {
if (v > 2 && !isErrorHandled) {
isErrorHandled = true;
throw v;
}
return v;
}),
retryWhen(err => err.pipe(delayWhen(v => timer(v)))));
});

subscriptionZone.run(() => {
observable1.subscribe(
(result: any) => {
log.push(result);
expect(Zone.current.name).toEqual(subscriptionZone.name);
},
(err: any) => { fail('should not call error'); },
() => {
log.push('completed');
expect(Zone.current.name).toEqual(subscriptionZone.name);
expect(log).toEqual([1, 2, 1, 2, 3, 'completed']);
done();
});
});
});
});
1 change: 1 addition & 0 deletions packages/zone.js/test/rxjs/rxjs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ import './rxjs.Observable.map.spec';
import './rxjs.Observable.race.spec';
import './rxjs.Observable.sample.spec';
import './rxjs.Observable.take.spec';
import './rxjs.Observable.retry.spec';
import './rxjs.Observable.timeout.spec';
import './rxjs.Observable.window.spec';

0 comments on commit 3fa8952

Please sign in to comment.