refactor(SafeSubscriber): simplify methods

- Simplifies the call patterns in SafeSubscriber to not be quite so insane. - Note: for some reason one mocha test is passing, but still printing out the error it is catching in `expect(fn).to.throw()` in the console. Annoying, but I stepped through the RxJS code and verified the behavior was correct. Related #5646
ReactiveX · Aug 26, 2020 · 54ace0b · 54ace0b
1 parent 15f13e9
commit 54ace0b
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 74 deletions.
diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts
@@ -612,14 +612,12 @@ describe('Observable', () => {
 
     describe('if config.useDeprecatedSynchronousErrorHandling === true', () => {
       beforeEach(() => {
-        const _warn = console.warn;
-        console.warn = noop;
+        config.quietBadConfig = true;
         config.useDeprecatedSynchronousErrorHandling = true;
-        console.warn = _warn;
       });
 
       it('should throw synchronously', () => {
-        expect(() => throwError(new Error()).subscribe()).to.throw();
+        expect(() => throwError(new Error()).subscribe()).to.throw(Error);
       });
 
       it('should rethrow if sink has syncErrorThrowable = false', () => {
@@ -637,10 +635,8 @@ describe('Observable', () => {
       });
 
       afterEach(() => {
-        const _log = console.log;
-        console.log = noop;
         config.useDeprecatedSynchronousErrorHandling = false;
-        console.log = _log;
+        config.quietBadConfig = false;
       });
     });
   });

diff --git a/src/internal/Subscriber.ts b/src/internal/Subscriber.ts
@@ -187,96 +187,59 @@ export class SafeSubscriber<T> extends Subscriber<T> {
     this._complete = complete!;
   }
 
-  next(value?: T): void {
+  next(value: T): void {
     if (!this.isStopped && this._next) {
-      const { _parentSubscriber } = this;
-      if (!config.useDeprecatedSynchronousErrorHandling || !_parentSubscriber.syncErrorThrowable) {
-        this.__tryOrUnsub(this._next, value);
-      } else if (this.__tryOrSetError(_parentSubscriber, this._next, value)) {
-        this.unsubscribe();
+      try {
+        this._next(value);
+      } catch (err) {
+        this._throw(err);
       }
     }
   }
 
-  error(err?: any): void {
+  error(err: any): void {
     if (!this.isStopped) {
-      const { _parentSubscriber } = this;
-      const { useDeprecatedSynchronousErrorHandling } = config;
       if (this._error) {
-        if (!useDeprecatedSynchronousErrorHandling || !_parentSubscriber.syncErrorThrowable) {
-          this.__tryOrUnsub(this._error, err);
-          this.unsubscribe();
-        } else {
-          this.__tryOrSetError(_parentSubscriber, this._error, err);
-          this.unsubscribe();
+        try {
+          this._error(err);
+        } catch (err) {
+          this._throw(err);
+          return;
         }
-      } else if (!_parentSubscriber.syncErrorThrowable) {
         this.unsubscribe();
-        if (useDeprecatedSynchronousErrorHandling) {
-          throw err;
-        }
-        hostReportError(err);
       } else {
-        if (useDeprecatedSynchronousErrorHandling) {
-          _parentSubscriber.syncErrorValue = err;
-          _parentSubscriber.syncErrorThrown = true;
-        } else {
-          hostReportError(err);
-        }
-        this.unsubscribe();
+        this._throw(err);
       }
     }
   }
 
-  complete(): void {
-    if (!this.isStopped) {
+  private _throw(err: any) {
+    this.unsubscribe();
+    if (config.useDeprecatedSynchronousErrorHandling) {
       const { _parentSubscriber } = this;
-      if (this._complete) {
-        const wrappedComplete = () => this._complete.call(this);
-
-        if (!config.useDeprecatedSynchronousErrorHandling || !_parentSubscriber.syncErrorThrowable) {
-          this.__tryOrUnsub(wrappedComplete);
-          this.unsubscribe();
-        } else {
-          this.__tryOrSetError(_parentSubscriber, wrappedComplete);
-          this.unsubscribe();
-        }
+      if (_parentSubscriber?.syncErrorThrowable) {
+        _parentSubscriber.syncErrorValue = err;
+        _parentSubscriber.syncErrorThrown = true;
       } else {
-        this.unsubscribe();
-      }
-    }
-  }
-
-  private __tryOrUnsub(fn: Function, value?: any): void {
-    try {
-      fn(value);
-    } catch (err) {
-      this.unsubscribe();
-      if (config.useDeprecatedSynchronousErrorHandling) {
         throw err;
-      } else {
-        hostReportError(err);
       }
+    } else {
+      hostReportError(err);
     }
   }
 
-  private __tryOrSetError(parent: Subscriber<T>, fn: Function, value?: any): boolean {
-    if (!config.useDeprecatedSynchronousErrorHandling) {
-      throw new Error('bad call');
-    }
-    try {
-      fn(value);
-    } catch (err) {
-      if (config.useDeprecatedSynchronousErrorHandling) {
-        parent.syncErrorValue = err;
-        parent.syncErrorThrown = true;
-        return true;
-      } else {
-        hostReportError(err);
-        return true;
+  complete(): void {
+    if (!this.isStopped) {
+      if (this._complete) {
+        try {
+          this._complete();
+        } catch (err) {
+          this._throw(err);
+          return;
+        }
       }
+      this.unsubscribe();
     }
-    return false;
   }
 
   unsubscribe() {