fix(zone.js): zone.js patches rxjs should check null for unsubscribe (#35990)

JiaLiPassion · AndrewKushnir · commit 3fa895298d9c · 2020-03-16T09:01:17.000-07:00
Close #31687, #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 #35990
diff --git a/packages/zone.js/lib/rxjs/rxjs.ts b/packages/zone.js/lib/rxjs/rxjs.ts
@@ -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: {
@@ -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);
+              }
+            };
+          }
         }
       }
     });
diff --git a/packages/zone.js/test/rxjs/rxjs.Observable.retry.spec.ts b/packages/zone.js/test/rxjs/rxjs.Observable.retry.spec.ts
@@ -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();
+          });
+    });
+  });
+});
diff --git a/packages/zone.js/test/rxjs/rxjs.spec.ts b/packages/zone.js/test/rxjs/rxjs.spec.ts
@@ -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';
