diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index 2ab0581fcb..e686f6dcb7 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -277,7 +277,6 @@ export declare function generate(options: GenerateOptions): Observab export interface GlobalConfig { onStoppedNotification: ((notification: ObservableNotification, subscriber: Subscriber) => void) | null; onUnhandledError: ((err: any) => void) | null; - useDeprecatedSynchronousErrorHandling: boolean; } export declare function groupBy(key: (value: T) => K, options: BasicGroupByOptions): OperatorFunction>; diff --git a/spec/Observable-spec.ts b/spec/Observable-spec.ts index 2ea4a56765..69e0e8e2c8 100644 --- a/spec/Observable-spec.ts +++ b/spec/Observable-spec.ts @@ -591,228 +591,6 @@ describe('Observable', () => { }); expect(caught).to.be.true; }); - - - describe('if config.useDeprecatedSynchronousErrorHandling === true', () => { - beforeEach(() => { - config.useDeprecatedSynchronousErrorHandling = true; - }); - - it('should throw synchronously', () => { - expect(() => throwError(() => new Error('thrown error')).subscribe()).to.throw(Error, 'thrown error'); - }); - - it('should rethrow if next handler throws', () => { - const observable = new Observable((observer) => { - observer.next(1); - }); - - const sink = Subscriber.create(() => { - throw 'error!'; - }); - - expect(() => { - observable.subscribe(sink); - }).to.throw('error!'); - }); - - - // From issue: https://github.com/ReactiveX/rxjs/issues/5979 - it('should still rethrow synchronous errors from next handlers on synchronous observables', () => { - expect(() => { - of('test').pipe( - // Any operators here - map(x => x + '!!!'), - map(x => x + x), - map(x => x + x), - map(x => x + x), - ).subscribe({ - next: () => { - throw new Error( - 'hi there!' - ) - } - }) - }).to.throw('hi there!'); - }); - - it('should rethrow synchronous errors from flattened observables', () => { - expect(() => { - of(1) - .pipe(concatMap(() => throwError(() => new Error('Ahoy! An error!')))) - .subscribe(console.log); - }).to.throw('Ahoy! An error!'); - - expect(() => { - of(1) - .pipe(switchMap(() => throwError(() => new Error('Avast! Thar be a new error!')))) - .subscribe(console.log); - }).to.throw('Avast! Thar be a new error!'); - }); - - it('should teardown even with a synchronous error', () => { - let called = false; - const badObservable = new Observable((subscriber) => { - subscriber.add(() => { - called = true; - }); - - subscriber.error(new Error('bad')); - }); - - try { - badObservable.subscribe(); - } catch (err) { - // do nothing - } - expect(called).to.be.true; - }); - - it('should teardown even with a synchronous thrown error', () => { - let called = false; - const badObservable = new Observable((subscriber) => { - subscriber.add(() => { - called = true; - }); - - throw new Error('bad'); - }); - - try { - badObservable.subscribe(); - } catch (err) { - // do nothing - } - expect(called).to.be.true; - }); - - - it('should handle empty string sync errors', () => { - const badObservable = new Observable(() => { - throw ''; - }); - - let caught = false; - try { - badObservable.subscribe(); - } catch (err) { - caught = true; - expect(err).to.equal(''); - } - expect(caught).to.be.true; - }); - - it('should execute finalize even with a sync error', () => { - let called = false; - const badObservable = new Observable((subscriber) => { - subscriber.error(new Error('bad')); - }).pipe( - finalize(() => { - called = true; - }) - ); - - try { - badObservable.subscribe(); - } catch (err) { - // do nothing - } - expect(called).to.be.true; - }); - - it('should execute finalize even with a sync thrown error', () => { - let called = false; - const badObservable = new Observable(() => { - throw new Error('bad'); - }).pipe( - finalize(() => { - called = true; - }) - ); - - try { - badObservable.subscribe(); - } catch (err) { - // do nothing - } - expect(called).to.be.true; - }); - - it('should execute finalize in order even with a sync error', () => { - const results: any[] = []; - const badObservable = new Observable((subscriber) => { - subscriber.error(new Error('bad')); - }).pipe( - finalize(() => { - results.push(1); - }), - finalize(() => { - results.push(2) - }) - ); - - try { - badObservable.subscribe(); - } catch (err) { - // do nothing - } - expect(results).to.deep.equal([1, 2]); - }); - - it('should execute finalize in order even with a sync thrown error', () => { - const results: any[] = []; - const badObservable = new Observable(() => { - throw new Error('bad'); - }).pipe( - finalize(() => { - results.push(1); - }), - finalize(() => { - results.push(2) - }) - ); - - try { - badObservable.subscribe(); - } catch (err) { - // do nothing - } - expect(results).to.deep.equal([1, 2]); - }); - - // https://github.com/ReactiveX/rxjs/issues/6271 - it('should not have a run-time error if no errors are thrown and there are operators', () => { - expect(() => { - of(1, 2, 3).pipe( - map(x => x + x), - map(x => Math.log(x)) - ) - .subscribe(); - }).not.to.throw(); - }); - - it('should call teardown if sync unsubscribed', () => { - let called = false; - const observable = new Observable(() => () => (called = true)); - const subscription = observable.subscribe(); - subscription.unsubscribe(); - - expect(called).to.be.true; - }); - - it('should call registered teardowns if sync unsubscribed', () => { - let called = false; - const observable = new Observable((subscriber) => subscriber.add(() => called = true)); - const subscription = observable.subscribe(); - subscription.unsubscribe(); - - expect(called).to.be.true; - }); - - afterEach(() => { - config.useDeprecatedSynchronousErrorHandling = false; - }); - }); }); describe('pipe', () => { diff --git a/spec/Subject-spec.ts b/spec/Subject-spec.ts index 3f922456ee..9a304cd007 100644 --- a/spec/Subject-spec.ts +++ b/spec/Subject-spec.ts @@ -759,166 +759,3 @@ describe('AnonymousSubject', () => { expect(subscribed).to.be.true; }); }); - -describe('useDeprecatedSynchronousErrorHandling', () => { - beforeEach(() => { - config.useDeprecatedSynchronousErrorHandling = true; - }); - - afterEach(() => { - config.useDeprecatedSynchronousErrorHandling = false; - }); - - it('should throw an error when nexting with a flattened, erroring inner observable', () => { - const subject = new Subject(); - subject.pipe(mergeMap(() => throwError(() => new Error('bad')))).subscribe(); - - expect(() => { - subject.next('wee'); - }).to.throw(Error, 'bad'); - }); - - it('should throw an error when nexting with a flattened, erroring inner observable with more than one operator', () => { - const subject = new Subject(); - subject.pipe(mergeMap(() => throwError(() => new Error('bad'))), map(x => x)).subscribe(); - - expect(() => { - subject.next('wee'); - }).to.throw(Error, 'bad'); - }); - - it('should throw an error when notifying an error with catchError returning an erroring inner observable', () => { - const subject = new Subject(); - subject.pipe(catchError(() => throwError(() => new Error('bad')))).subscribe(); - - expect(() => { - subject.error('wee'); - }).to.throw(Error, 'bad'); - }); - - it('should throw an error when nexting with an operator that errors synchronously', () => { - const subject = new Subject(); - subject.pipe(mergeMap(() => { - throw new Error('lol'); - })).subscribe(); - - expect(() => { - subject.next('wee'); - }).to.throw(Error, 'lol'); - }); - - - it('should throw an error when notifying an error with a catchError that errors synchronously', () => { - const subject = new Subject(); - subject.pipe(catchError(() => { - throw new Error('lol'); - })).subscribe(); - - expect(() => { - subject.error('wee'); - }).to.throw(Error, 'lol'); - }); - - it('should throw an error when nexting with an erroring next handler', () => { - const subject = new Subject(); - subject.subscribe(() => { - throw new Error('lol'); - }); - - expect(() => { - subject.next('wee'); - }).to.throw(Error, 'lol'); - }); - - it('should throw an error when notifying with an erroring error handler', () => { - const subject = new Subject(); - subject.subscribe({ - error: () => { - throw new Error('lol'); - } - }); - - expect(() => { - subject.error('wee'); - }).to.throw(Error, 'lol'); - }); - - it('should throw an error when notifying with an erroring complete handler', () => { - const subject = new Subject(); - subject.subscribe({ - complete: () => { - throw new Error('lol'); - } - }); - - expect(() => { - subject.complete(); - }).to.throw(Error, 'lol'); - }); - - it('should throw an error when notifying an complete, and concatenated with another observable that synchronously errors', () => { - const subject = new Subject(); - concat(subject, throwError(new Error('lol'))).subscribe(); - - expect(() => { - subject.complete(); - }).to.throw(Error, 'lol'); - }); - - it('should not throw on second error passed', () => { - const subject = new Subject(); - - subject.subscribe(); - - expect(() => { - subject.error(new Error('one')); - }).to.throw(Error, 'one'); - - expect(() => { - subject.error(new Error('two')); - }).not.to.throw(Error, 'two'); - }); - - it('should not throw on second error passed, even after having been operated on', () => { - const subject = new Subject(); - - subject.pipe(mergeMap(x => [x])).subscribe(); - - expect(() => { - subject.error(new Error('one')); - }).to.throw(Error, 'one'); - - expect(() => { - subject.error('two'); - }).not.to.throw(); - }); - - it('deep rethrowing 1', () => { - const subject1 = new Subject(); - const subject2 = new Subject(); - - subject2.subscribe(); - - subject1.subscribe({ - next: () => subject2.error(new Error('hahaha')) - }); - - expect(() => { - subject1.next('test'); - }).to.throw(Error, 'hahaha'); - }); - - it('deep rethrowing 2', () => { - const subject1 = new Subject(); - - subject1.subscribe({ - next: () => { - throwError(new Error('hahaha')).subscribe(); - } - }); - - expect(() => { - subject1.next('test'); - }).to.throw(Error, 'hahaha'); - }); -}); \ No newline at end of file diff --git a/src/internal/Observable.ts b/src/internal/Observable.ts index ec296be8cf..7ee317a5be 100644 --- a/src/internal/Observable.ts +++ b/src/internal/Observable.ts @@ -8,7 +8,6 @@ import { TeardownLogic, OperatorFunction, Subscribable, Observer } from './types import { observable as Symbol_observable } from './symbol/observable'; import { pipeFromArray } from './util/pipe'; import { isFunction } from './util/isFunction'; -import { errorContext } from './util/errorContext'; /** * A representation of any set of values over any amount of time. This is the most basic building block @@ -216,23 +215,21 @@ export class Observable implements Subscribable { ): Subscription { const subscriber = isSubscriber(observerOrNext) ? observerOrNext : new SafeSubscriber(observerOrNext, error, complete); - errorContext(() => { - const { operator, source } = this; - subscriber.add( - operator - ? // We're dealing with a subscription in the - // operator chain to one of our lifted operators. - operator.call(subscriber, source) - : source - ? // If `source` has a value, but `operator` does not, something that - // had intimate knowledge of our API, like our `Subject`, must have - // set it. We're going to just call `_subscribe` directly. - this._subscribe(subscriber) - : // In all other cases, we're likely wrapping a user-provided initializer - // function, so we need to catch errors and handle them appropriately. - this._trySubscribe(subscriber) - ); - }); + const { operator, source } = this; + subscriber.add( + operator + ? // We're dealing with a subscription in the + // operator chain to one of our lifted operators. + operator.call(subscriber, source) + : source + ? // If `source` has a value, but `operator` does not, something that + // had intimate knowledge of our API, like our `Subject`, must have + // set it. We're going to just call `_subscribe` directly. + this._subscribe(subscriber) + : // In all other cases, we're likely wrapping a user-provided initializer + // function, so we need to catch errors and handle them appropriately. + this._trySubscribe(subscriber) + ); return subscriber; } diff --git a/src/internal/Subject.ts b/src/internal/Subject.ts index 11ffb11dcb..0fef20a6d7 100644 --- a/src/internal/Subject.ts +++ b/src/internal/Subject.ts @@ -5,7 +5,6 @@ import { Subscription, EMPTY_SUBSCRIPTION } from './Subscription'; import { Observer, SubscriptionLike, TeardownLogic } from './types'; import { ObjectUnsubscribedError } from './util/ObjectUnsubscribedError'; import { arrRemove } from './util/arrRemove'; -import { errorContext } from './util/errorContext'; /** * A Subject is a special type of Observable that allows values to be @@ -55,42 +54,36 @@ export class Subject extends Observable implements SubscriptionLike { } next(value: T) { - errorContext(() => { - this._throwIfClosed(); - if (!this.isStopped) { - const copy = this.observers.slice(); - for (const observer of copy) { - observer.next(value); - } + this._throwIfClosed(); + if (!this.isStopped) { + const copy = this.observers.slice(); + for (const observer of copy) { + observer.next(value); } - }); + } } error(err: any) { - errorContext(() => { - this._throwIfClosed(); - if (!this.isStopped) { - this.hasError = this.isStopped = true; - this.thrownError = err; - const { observers } = this; - while (observers.length) { - observers.shift()!.error(err); - } + this._throwIfClosed(); + if (!this.isStopped) { + this.hasError = this.isStopped = true; + this.thrownError = err; + const { observers } = this; + while (observers.length) { + observers.shift()!.error(err); } - }); + } } complete() { - errorContext(() => { - this._throwIfClosed(); - if (!this.isStopped) { - this.isStopped = true; - const { observers } = this; - while (observers.length) { - observers.shift()!.complete(); - } + this._throwIfClosed(); + if (!this.isStopped) { + this.isStopped = true; + const { observers } = this; + while (observers.length) { + observers.shift()!.complete(); } - }); + } } unsubscribe() { diff --git a/src/internal/Subscriber.ts b/src/internal/Subscriber.ts index 7b881fb21a..341f38526f 100644 --- a/src/internal/Subscriber.ts +++ b/src/internal/Subscriber.ts @@ -6,7 +6,6 @@ import { reportUnhandledError } from './util/reportUnhandledError'; import { noop } from './util/noop'; import { nextNotification, errorNotification, COMPLETE_NOTIFICATION } from './NotificationFactories'; import { timeoutProvider } from './scheduler/timeoutProvider'; -import { captureError } from './util/errorContext'; /** * Implements the {@link Observer} interface and extends the @@ -182,13 +181,7 @@ function wrapForErrorHandling(handler: (arg?: any) => void, instance: SafeSubscr try { handler(...args); } catch (err) { - if (config.useDeprecatedSynchronousErrorHandling) { - captureError(err); - } else { - // Ideal path, we report this as an unhandled error, - // which is thrown on a new call stack. - reportUnhandledError(err); - } + reportUnhandledError(err); } }; } diff --git a/src/internal/config.ts b/src/internal/config.ts index 50bf8341a5..bb02367424 100644 --- a/src/internal/config.ts +++ b/src/internal/config.ts @@ -8,7 +8,6 @@ import { ObservableNotification } from './types'; export const config: GlobalConfig = { onUnhandledError: null, onStoppedNotification: null, - useDeprecatedSynchronousErrorHandling: false, }; /** @@ -39,18 +38,4 @@ export interface GlobalConfig { * behavior of the library. */ onStoppedNotification: ((notification: ObservableNotification, subscriber: Subscriber) => void) | null; - - /** - * If true, turns on synchronous error rethrowing, which is a deprecated behavior - * in v6 and higher. This behavior enables bad patterns like wrapping a subscribe - * call in a try/catch block. It also enables producer interference, a nasty bug - * where a multicast can be broken for all observers by a downstream consumer with - * an unhandled error. DO NOT USE THIS FLAG UNLESS IT'S NEEDED TO BUY TIME - * FOR MIGRATION REASONS. - * - * @deprecated As of version 8, RxJS will no longer support synchronous throwing - * of unhandled errors. All errors will be thrown on a separate call stack to prevent bad - * behaviors described above. Will be removed in v8. - */ - useDeprecatedSynchronousErrorHandling: boolean; } diff --git a/src/internal/operators/finalize.ts b/src/internal/operators/finalize.ts index aa4575be35..845d7b37fc 100644 --- a/src/internal/operators/finalize.ts +++ b/src/internal/operators/finalize.ts @@ -65,12 +65,7 @@ import { operate } from '../util/lift'; */ export function finalize(callback: () => void): MonoTypeOperatorFunction { return operate((source, subscriber) => { - // TODO: This try/finally was only added for `useDeprecatedSynchronousErrorHandling`. - // REMOVE THIS WHEN THAT HOT GARBAGE IS REMOVED IN V8. - try { - source.subscribe(subscriber); - } finally { - subscriber.add(callback); - } + source.subscribe(subscriber); + subscriber.add(callback); }); } diff --git a/src/internal/util/errorContext.ts b/src/internal/util/errorContext.ts deleted file mode 100644 index 6c4ffb170e..0000000000 --- a/src/internal/util/errorContext.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { config } from '../config'; - -let context: { errorThrown: boolean; error: any } | null = null; - -/** - * Handles dealing with errors for super-gross mode. Creates a context, in which - * any synchronously thrown errors will be passed to {@link captureError}. Which - * will record the error such that it will be rethrown after the call back is complete. - * TODO: Remove in v8 - * @param cb An immediately executed function. - */ -export function errorContext(cb: () => void) { - if (config.useDeprecatedSynchronousErrorHandling) { - const isRoot = !context; - if (isRoot) { - context = { errorThrown: false, error: null }; - } - cb(); - if (isRoot) { - const { errorThrown, error } = context!; - context = null; - if (errorThrown) { - throw error; - } - } - } else { - // This is the general non-deprecated path for everyone that - // isn't crazy enough to use super-gross mode (useDeprecatedSynchronousErrorHandling) - cb(); - } -} - -/** - * Captures errors only in super-gross mode. - * @param err the error to capture - */ -export function captureError(err: any) { - if (config.useDeprecatedSynchronousErrorHandling && context) { - context.errorThrown = true; - context.error = err; - } -}