From dad270afcf496de74b4392024191715d7dbef4f5 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 18 Aug 2020 18:10:08 -0500 Subject: [PATCH] feat(throwError): now accepts a factory to create the error (#5647) * feat(throwError): now accepts a factory to create the error - Adds feature to allow the error instance the consumer is notified with to be created at the moment of notification. - Updates tests to use "run mode". - Improves documentation, including recommendations on when `throwError` is not necessary. BREAKING CHANGE: In an extreme corner case for usage, `throwError` is no longer able to emit a function as an error directly. If you need to push a function as an error, you will have to use the factory function to return the function like so: `throwError(() => functionToEmit)`, in other words `throwError(() => () => console.log('called later'))`. resolves #5617 * chore: clean up and address comments --- api_guard/dist/types/index.d.ts | 4 +- spec-dtslint/observables/throwError-spec.ts | 1 + spec/observables/throwError-spec.ts | 83 ++++++++-- src/internal/observable/throwError.ts | 170 +++++++++++++------- 4 files changed, 181 insertions(+), 77 deletions(-) diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index 741c7a01c9..e4c4d13c32 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -563,7 +563,9 @@ export declare type Tail = ((...args: X) => any) extends ((arg: export declare type TeardownLogic = Unsubscribable | Function | void; -export declare function throwError(error: any, scheduler?: SchedulerLike): Observable; +export declare function throwError(errorFactory: () => any): Observable; +export declare function throwError(error: any): Observable; +export declare function throwError(errorOrErrorFactory: any, scheduler: SchedulerLike): Observable; export interface TimeInterval { interval: number; diff --git a/spec-dtslint/observables/throwError-spec.ts b/spec-dtslint/observables/throwError-spec.ts index 852d6d14b1..9f905ddb17 100644 --- a/spec-dtslint/observables/throwError-spec.ts +++ b/spec-dtslint/observables/throwError-spec.ts @@ -4,6 +4,7 @@ it('should accept any type and return never observable', () => { const a = throwError(1); // $ExpectType Observable const b = throwError('a'); // $ExpectType Observable const c = throwError({a: 1}); // $ExpectType Observable + const d = throwError(() => ({ a: 2 })); // $ExpectType Observable }); it('should support scheduler', () => { diff --git a/spec/observables/throwError-spec.ts b/spec/observables/throwError-spec.ts index 05cf24d2cc..f54101e844 100644 --- a/spec/observables/throwError-spec.ts +++ b/spec/observables/throwError-spec.ts @@ -1,32 +1,83 @@ +/** @prettier */ import { expect } from 'chai'; import { TestScheduler } from 'rxjs/testing'; import { throwError } from 'rxjs'; -import { expectObservable } from '../helpers/marble-testing'; +import { observableMatcher } from '../helpers/observableMatcher'; -declare const rxTestScheduler: TestScheduler; - -/** @test {throw} */ +/** @test {throwError} */ describe('throwError', () => { + let rxTest: TestScheduler; + + beforeEach(() => { + rxTest = new TestScheduler(observableMatcher); + }); + it('should create a cold observable that just emits an error', () => { - const expected = '#'; - const e1 = throwError('error'); - expectObservable(e1).toBe(expected); + rxTest.run(({ expectObservable }) => { + const expected = '#'; + const e1 = throwError('error'); + expectObservable(e1).toBe(expected); + }); }); it('should emit one value', (done) => { let calls = 0; - throwError('bad').subscribe(() => { - done(new Error('should not be called')); - }, (err) => { - expect(++calls).to.equal(1); - expect(err).to.equal('bad'); - done(); - }); + throwError('bad').subscribe( + () => { + done(new Error('should not be called')); + }, + (err) => { + expect(++calls).to.equal(1); + expect(err).to.equal('bad'); + done(); + } + ); }); it('should accept scheduler', () => { - const e = throwError('error', rxTestScheduler); + rxTest.run(({ expectObservable }) => { + const e = throwError('error', rxTest); + + expectObservable(e).toBe('#'); + }); + }); + + it('should accept a factory function', () => { + let calls = 0; + let errors: any[] = []; + + const source = throwError(() => ({ + call: ++calls, + message: 'LOL', + })); + + source.subscribe({ + next: () => { + throw new Error('this should not happen'); + }, + error: (err) => { + errors.push(err); + }, + }); + + source.subscribe({ + next: () => { + throw new Error('this should not happen'); + }, + error: (err) => { + errors.push(err); + }, + }); - expectObservable(e).toBe('#'); + expect(errors).to.deep.equal([ + { + call: 1, + message: 'LOL', + }, + { + call: 2, + message: 'LOL', + }, + ]); }); }); diff --git a/src/internal/observable/throwError.ts b/src/internal/observable/throwError.ts index 98edfc7b45..054fae432c 100644 --- a/src/internal/observable/throwError.ts +++ b/src/internal/observable/throwError.ts @@ -1,84 +1,134 @@ +/** @prettier */ import { Observable } from '../Observable'; import { SchedulerLike } from '../types'; import { Subscriber } from '../Subscriber'; /** - * Creates an Observable that emits no items to the Observer and immediately - * emits an error notification. + * Creates an observable that will create an error instance and push it to the consumer as an error + * immediately upon subscription. * - * Just emits 'error', and nothing else. - * + * Just errors and does nothing else * * ![](throw.png) * - * This static operator is useful for creating a simple Observable that only - * emits the error notification. It can be used for composing with other - * Observables, such as in a {@link mergeMap}. + * This creation function is useful for creating an observable that will create an error and error every + * time it is subscribed to. Generally, inside of most operators when you might want to return an errored + * observable, this is unnecessary. In most cases, such as in the inner return of {@link concatMap}, + * {@link mergeMap}, {@link defer}, and many others, you can simply throw the error, and RxJS will pick + * that up and notify the consumer of the error. + * + * ## Example + * + * Create a simple observable that will create a new error with a timestamp and log it + * and the message every time you subscribe to it. + * + * ```ts + * import { throwError } from 'rxjs'; + * + * let errorCount = 0; + * + * const errorWithTimestamp$ = throwError(() => { + * const error: any = new Error(`This is error number ${++errorCount}`); + * error.timestamp = Date.now(); + * return error; + * }); + * + * errorWithTimesptamp$.subscribe({ + * error: err => console.log(err.timestamp, err.message) + * }); + * + * errorWithTimesptamp$.subscribe({ + * error: err => console.log(err.timestamp, err.message) + * }); + * + * // Logs the timestamp and a new error message each subscription; + * ``` + * + * ## Unnecessary usage + * + * Using `throwError` inside of an operator or creation function + * with a callback, is usually not necessary: * - * ## Examples - * ### Emit the number 7, then emit an error * ```ts - * import { throwError, concat, of } from 'rxjs'; + * import { throwError, timer, of } from 'rxjs'; + * import { concatMap } from 'rxjs/operators'; * - * const result = concat(of(7), throwError(new Error('oops!'))); - * result.subscribe(x => console.log(x), e => console.error(e)); + * const delays$ = of(1000, 2000, Infinity, 3000); * - * // Logs: - * // 7 - * // Error: oops! + * delays$.pipe( + * concatMap(ms => { + * if (ms < 10000) { + * return timer(ms); + * } else { + * // This is probably overkill. + * return throwError(() => new Error(`Invalid time ${ms}`)); + * } + * }) + * ) + * .subscribe({ + * next: console.log, + * error: console.error + * }); * ``` * - * --- + * You can just throw the error instead: * - * ### Map and flatten numbers to the sequence 'a', 'b', 'c', but throw an error for 2 * ```ts - * import { throwError, interval, of } from 'rxjs'; - * import { mergeMap } from 'rxjs/operators'; - * - * interval(1000).pipe( - * mergeMap(x => x === 2 - * ? throwError('Twos are bad') - * : of('a', 'b', 'c') - * ), - * ).subscribe(x => console.log(x), e => console.error(e)); - * - * // Logs: - * // a - * // b - * // c - * // a - * // b - * // c - * // Twos are bad + * import { throwError, timer, of } from 'rxjs'; + * import { concatMap } from 'rxjs/operators'; + * + * const delays$ = of(1000, 2000, Infinity, 3000); + * + * delays$.pipe( + * concatMap(ms => { + * if (ms < 10000) { + * return timer(ms); + * } else { + * // Cleaner and easier to read for most folks. + * throw new Error(`Invalid time ${ms}`); + * } + * }) + * ) + * .subscribe({ + * next: console.log, + * error: console.error + * }); * ``` * - * @see {@link Observable} - * @see {@link empty} - * @see {@link never} - * @see {@link of} - * - * @param {any} error The particular Error to pass to the error notification. - * @param {SchedulerLike} [scheduler] A {@link SchedulerLike} to use for scheduling - * the emission of the error notification. - * @return {Observable} An error Observable: emits only the error notification - * using the given error argument. - * @static true - * @name throwError - * @owner Observable + * @param errorFactory A factory function that will create the error instance that is pushed. */ -export function throwError(error: any, scheduler?: SchedulerLike): Observable { +export function throwError(errorFactory: () => any): Observable; + +/** + * Returns an observable that will error with the specified error immediately upon subscription. + * + * @param error The error instance to emit + * @deprecated Removed in v8. Instead, pass a factory function to `throwError(() => new Error('test'))`. This is + * because it will create the error at the moment it should be created and capture a more appropriate stack trace. If + * for some reason you need to create the error ahead of time, you can still do that: `const err = new Error('test'); throwError(() => err);`. + */ +export function throwError(error: any): Observable; + +/** + * Notifies the consumer of an error using a given scheduler by scheduling it at delay `0` upon subscription. + * + * @param errorOrErrorFactory An error instance or error factory + * @param scheduler A scheduler to use to schedule the error notification + * @deprecated Use `throwError` in combination with {@link observeOn}: + * `throwError(() => new Error('test')).pipe(observeOn(scheduler));` + */ +export function throwError(errorOrErrorFactory: any, scheduler: SchedulerLike): Observable; + +export function throwError(errorOrErrorFactory: any, scheduler?: SchedulerLike): Observable { if (!scheduler) { - return new Observable(subscriber => subscriber.error(error)); + return new Observable((subscriber) => + subscriber.error(typeof errorOrErrorFactory === 'function' ? errorOrErrorFactory() : errorOrErrorFactory) + ); } else { - return new Observable(subscriber => scheduler.schedule(dispatch as any, 0, { error, subscriber })); + return new Observable((subscriber) => + scheduler.schedule(() => { + subscriber.error(typeof errorOrErrorFactory === 'function' ? errorOrErrorFactory() : errorOrErrorFactory); + }) + ); } } - -interface DispatchArg { - error: any; - subscriber: Subscriber; -} - -function dispatch({ error, subscriber }: DispatchArg) { - subscriber.error(error); -}