From 0892a2d8d0c294c9fa476fd0276f33081c96f51d Mon Sep 17 00:00:00 2001
From: Ben Lesh <ben@benlesh.com>
Date: Mon, 19 Mar 2018 16:14:42 -0700
Subject: [PATCH] fix(config.useDeprecatedSynchronousErrorThrowing): reentrant
 error throwing 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
---
 spec/Observable-spec.ts           | 50 ++++++++++++++++++++++++++++++-
 spec/operators/repeatWhen-spec.ts |  4 +++
 src/internal/Observable.ts        |  2 +-
 src/internal/Subscriber.ts        |  1 +
 4 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts
index 12fcea4bf4..c13c876243 100644
--- a/spec/Observable-spec.ts
+++ b/spec/Observable-spec.ts
@@ -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');
 
@@ -533,9 +535,38 @@ 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', () => {
@@ -543,8 +574,25 @@ describe('Observable', () => {
           .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;
       });
     });
   });
diff --git a/spec/operators/repeatWhen-spec.ts b/spec/operators/repeatWhen-spec.ts
index a02dfc6ced..968b009c38 100644
--- a/spec/operators/repeatWhen-spec.ts
+++ b/spec/operators/repeatWhen-spec.ts
@@ -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;
@@ -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) => {
diff --git a/src/internal/Observable.ts b/src/internal/Observable.ts
index 3ef3640c47..1ff29df883 100644
--- a/src/internal/Observable.ts
+++ b/src/internal/Observable.ts
@@ -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) {
diff --git a/src/internal/Subscriber.ts b/src/internal/Subscriber.ts
index 39b4d979ab..6b4bc62435 100644
--- a/src/internal/Subscriber.ts
+++ b/src/internal/Subscriber.ts
@@ -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 {