From d3cc1399f08d52717455b49fe8325f96c10374ea Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 1 Sep 2020 15:36:25 -0500 Subject: [PATCH] fix(errors): Custom RxJS errors now all have a call stack NOTE: Because we have to workaround compilation-to-ES5 inadequacies until call stack locations will show as being inside of the constructor impelementation at the top level. This will go away as we are able to move the community to ES2015+ BREAKING CHANGE: Tests that are written with naive expectations against errors may fail now that errors have a proper `stack` property. In some testing frameworks, a deep equality check on two error instances will check the values in `stack`, which could be different. fixes #4250 --- spec/helpers/test-helper.ts | 12 +---- spec/operators/concatWith-spec.ts | 5 ++- spec/operators/single-spec.ts | 4 +- spec/operators/takeLast-spec.ts | 4 +- spec/util/ArgumentOutOfRangeError-spec.ts | 3 ++ spec/util/EmptyError-spec.ts | 3 ++ spec/util/ObjectUnsubscribedError-spec.ts | 3 ++ spec/util/TimeoutError-spec.ts | 3 ++ spec/util/UnsubscriptionError-spec.ts | 1 + src/internal/observable/dom/AjaxObservable.ts | 45 +++++++++---------- src/internal/operators/timeout.ts | 21 +++------ src/internal/util/ArgumentOutOfRangeError.ts | 25 ++++------- src/internal/util/EmptyError.ts | 19 +++----- src/internal/util/NotFoundError.ts | 25 ++++------- src/internal/util/ObjectUnsubscribedError.ts | 25 ++++------- src/internal/util/SequenceError.ts | 25 ++++------- src/internal/util/UnsubscriptionError.ts | 30 +++++-------- src/internal/util/createErrorClass.ts | 15 +++++++ 18 files changed, 113 insertions(+), 155 deletions(-) create mode 100644 src/internal/util/createErrorClass.ts diff --git a/spec/helpers/test-helper.ts b/spec/helpers/test-helper.ts index f8598aa3753..1b64ac16761 100644 --- a/spec/helpers/test-helper.ts +++ b/spec/helpers/test-helper.ts @@ -60,14 +60,4 @@ export const createObservableInputs = (value: T) => of( /** * Used to signify no subscriptions took place to `expectSubscriptions` assertions. */ -export const NO_SUBS: string[] = []; - -/** - * Does a deep equality assertion. Used to set up {@link TestScheduler}, so that - * trees of marbles can be compared. - * @param actual The value to run the expectation against. - * @param expected The value expected. - */ -export function assertDeepEquals (actual: any, expected: any) { - expect(actual).to.deep.equal(expected); -} +export const NO_SUBS: string[] = []; \ No newline at end of file diff --git a/spec/operators/concatWith-spec.ts b/spec/operators/concatWith-spec.ts index 7dcd9762c80..2cdfc14c2d1 100644 --- a/spec/operators/concatWith-spec.ts +++ b/spec/operators/concatWith-spec.ts @@ -2,14 +2,15 @@ import { expect } from 'chai'; import { of, Observable } from 'rxjs'; import { concatWith, mergeMap, take } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; -import { assertDeepEquals, NO_SUBS } from '../helpers/test-helper'; +import { NO_SUBS } from '../helpers/test-helper'; +import { observableMatcher } from '../helpers/observableMatcher'; /** @test {concat} */ describe('concat operator', () => { let rxTest: TestScheduler; beforeEach(() => { - rxTest = new TestScheduler(assertDeepEquals); + rxTest = new TestScheduler(observableMatcher); }); it('should concatenate two cold observables', () => { diff --git a/spec/operators/single-spec.ts b/spec/operators/single-spec.ts index 433bdc019dd..994caa74bd9 100644 --- a/spec/operators/single-spec.ts +++ b/spec/operators/single-spec.ts @@ -2,14 +2,14 @@ import { expect } from 'chai'; import { single, mergeMap, tap } from 'rxjs/operators'; import { of, EmptyError, SequenceError, NotFoundError, Observable } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; -import { assertDeepEquals } from '../helpers/test-helper'; +import { observableMatcher } from '../helpers/observableMatcher'; /** @test {single} */ describe('single operator', () => { let rxTest: TestScheduler; beforeEach(() => { - rxTest = new TestScheduler(assertDeepEquals); + rxTest = new TestScheduler(observableMatcher); }); it('should raise error from empty predicate if observable emits multiple time', () => { diff --git a/spec/operators/takeLast-spec.ts b/spec/operators/takeLast-spec.ts index 009aa92c3eb..3966096dc6b 100644 --- a/spec/operators/takeLast-spec.ts +++ b/spec/operators/takeLast-spec.ts @@ -2,14 +2,14 @@ import { expect } from 'chai'; import { takeLast, mergeMap } from 'rxjs/operators'; import { range, ArgumentOutOfRangeError, of } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; -import { assertDeepEquals } from '../helpers/test-helper'; +import { observableMatcher } from '../helpers/observableMatcher'; /** @test {takeLast} */ describe('takeLast operator', () => { let rxTest: TestScheduler; beforeEach(() => { - rxTest = new TestScheduler(assertDeepEquals); + rxTest = new TestScheduler(observableMatcher); }); it('should error for invalid arguments', () => { diff --git a/spec/util/ArgumentOutOfRangeError-spec.ts b/spec/util/ArgumentOutOfRangeError-spec.ts index fe0d6fa482c..30fd780fda0 100644 --- a/spec/util/ArgumentOutOfRangeError-spec.ts +++ b/spec/util/ArgumentOutOfRangeError-spec.ts @@ -10,4 +10,7 @@ describe('ArgumentOutOfRangeError', () => { it('Should have a message', () => { expect(error.message).to.be.equal('argument out of range'); }); + it('Should have a stack', () => { + expect(error.stack).to.be.a('string'); + }); }); diff --git a/spec/util/EmptyError-spec.ts b/spec/util/EmptyError-spec.ts index 669251780fe..64a2253b9ef 100644 --- a/spec/util/EmptyError-spec.ts +++ b/spec/util/EmptyError-spec.ts @@ -10,4 +10,7 @@ describe('EmptyError', () => { it('Should have a message', () => { expect(error.message).to.be.equal('no elements in sequence'); }); + it('Should have a stack', () => { + expect(error.stack).to.be.a('string'); + }); }); diff --git a/spec/util/ObjectUnsubscribedError-spec.ts b/spec/util/ObjectUnsubscribedError-spec.ts index d879af3f025..37c40b7ea9a 100644 --- a/spec/util/ObjectUnsubscribedError-spec.ts +++ b/spec/util/ObjectUnsubscribedError-spec.ts @@ -10,4 +10,7 @@ describe('ObjectUnsubscribedError', () => { it('Should have a message', () => { expect(error.message).to.be.equal('object unsubscribed'); }); + it('Should have a stack', () => { + expect(error.stack).to.be.a('string'); + }); }); diff --git a/spec/util/TimeoutError-spec.ts b/spec/util/TimeoutError-spec.ts index a520d92c541..56f5c2996ec 100644 --- a/spec/util/TimeoutError-spec.ts +++ b/spec/util/TimeoutError-spec.ts @@ -10,4 +10,7 @@ describe('TimeoutError', () => { it('Should have a message', () => { expect(error.message).to.be.equal('Timeout has occurred'); }); + it('Should have a stack', () => { + expect(error.stack).to.be.a('string'); + }); }); diff --git a/spec/util/UnsubscriptionError-spec.ts b/spec/util/UnsubscriptionError-spec.ts index 2799d680f55..fc54c833320 100644 --- a/spec/util/UnsubscriptionError-spec.ts +++ b/spec/util/UnsubscriptionError-spec.ts @@ -19,6 +19,7 @@ describe('UnsubscriptionError', () => { expect(err instanceof UnsubscriptionError).to.equal(true); expect(err.errors).to.deep.equal([err1, err2]); expect(err.name).to.equal('UnsubscriptionError'); + expect(err.stack).to.be.a('string'); } }); }); diff --git a/src/internal/observable/dom/AjaxObservable.ts b/src/internal/observable/dom/AjaxObservable.ts index 2fde7015699..e65add9aaff 100644 --- a/src/internal/observable/dom/AjaxObservable.ts +++ b/src/internal/observable/dom/AjaxObservable.ts @@ -2,6 +2,7 @@ import { Observable } from '../../Observable'; import { Subscriber } from '../../Subscriber'; import { TeardownLogic, PartialObserver } from '../../types'; +import { createErrorClass } from '../../util/createErrorClass'; export interface AjaxRequest { url?: string; @@ -322,28 +323,6 @@ export interface AjaxErrorCtor { new (message: string, xhr: XMLHttpRequest, request: AjaxRequest): AjaxError; } -const AjaxErrorImpl = (() => { - function AjaxErrorImpl(this: any, message: string, xhr: XMLHttpRequest, request: AjaxRequest): AjaxError { - Error.call(this); - this.message = message; - this.name = 'AjaxError'; - this.xhr = xhr; - this.request = request; - this.status = xhr.status; - this.responseType = xhr.responseType; - let response: any; - try { - response = getXHRResponse(xhr); - } catch (err) { - response = xhr.responseText; - } - this.response = response; - return this; - } - AjaxErrorImpl.prototype = Object.create(Error.prototype); - return AjaxErrorImpl; -})(); - /** * Thrown when an error occurs during an AJAX request. * This is only exported because it is useful for checking to see if an error @@ -353,7 +332,25 @@ const AjaxErrorImpl = (() => { * @class AjaxError * @see ajax */ -export const AjaxError: AjaxErrorCtor = AjaxErrorImpl as any; +export const AjaxError: AjaxErrorCtor = createErrorClass('AjaxError', function ( + this: any, + message: string, + xhr: XMLHttpRequest, + request: AjaxRequest +) { + this.message = message; + this.xhr = xhr; + this.request = request; + this.status = xhr.status; + this.responseType = xhr.responseType; + let response: any; + try { + response = getXHRResponse(xhr); + } catch (err) { + response = xhr.responseText; + } + this.response = response; +}); function getXHRResponse(xhr: XMLHttpRequest) { switch (xhr.responseType) { @@ -391,6 +388,8 @@ export interface AjaxTimeoutErrorCtor { new (xhr: XMLHttpRequest, request: AjaxRequest): AjaxTimeoutError; } +// NOTE: We are not using createErrorClass here, because we're deriving this from +// the AjaxError we defined above. const AjaxTimeoutErrorImpl = (() => { function AjaxTimeoutErrorImpl(this: any, xhr: XMLHttpRequest, request: AjaxRequest) { AjaxError.call(this, 'ajax timeout', xhr, request); diff --git a/src/internal/operators/timeout.ts b/src/internal/operators/timeout.ts index fc40e971ce5..757a01e4c3b 100644 --- a/src/internal/operators/timeout.ts +++ b/src/internal/operators/timeout.ts @@ -7,6 +7,7 @@ import { Subscription } from '../Subscription'; import { lift } from '../util/lift'; import { Observable } from '../Observable'; import { from } from '../observable/from'; +import { createErrorClass } from '../util/createErrorClass'; export interface TimeoutConfig { /** @@ -72,20 +73,6 @@ export interface TimeoutErrorCtor { new (info?: TimeoutInfo): TimeoutError; } -const TimeoutErrorImpl = (() => { - function TimeoutErrorImpl(this: any, info: TimeoutInfo | null = null) { - Error.call(this); - this.message = 'Timeout has occurred'; - this.name = 'TimeoutError'; - this.info = info; - return this; - } - - TimeoutErrorImpl.prototype = Object.create(Error.prototype); - - return TimeoutErrorImpl; -})(); - /** * An error thrown by the {@link operators/timeout} operator. * @@ -98,7 +85,11 @@ const TimeoutErrorImpl = (() => { * * @class TimeoutError */ -export const TimeoutError: TimeoutErrorCtor = TimeoutErrorImpl as any; +export const TimeoutError: TimeoutErrorCtor = createErrorClass('TimeoutError', function (info: TimeoutInfo | null = null) { + this.message = 'Timeout has occurred'; + this.name = 'TimeoutError'; + (this as any).info = info; +}); /** * If `with` is provided, this will return an observable that will switch to a different observable if the source diff --git a/src/internal/util/ArgumentOutOfRangeError.ts b/src/internal/util/ArgumentOutOfRangeError.ts index 983ce709aaf..6cb576f790d 100644 --- a/src/internal/util/ArgumentOutOfRangeError.ts +++ b/src/internal/util/ArgumentOutOfRangeError.ts @@ -1,23 +1,12 @@ -export interface ArgumentOutOfRangeError extends Error { -} +/** @prettier */ +import { createErrorClass } from './createErrorClass'; + +export interface ArgumentOutOfRangeError extends Error {} export interface ArgumentOutOfRangeErrorCtor { - new(): ArgumentOutOfRangeError; + new (): ArgumentOutOfRangeError; } -const ArgumentOutOfRangeErrorImpl = (() => { - function ArgumentOutOfRangeErrorImpl(this: Error) { - Error.call(this); - this.message = 'argument out of range'; - this.name = 'ArgumentOutOfRangeError'; - return this; - } - - ArgumentOutOfRangeErrorImpl.prototype = Object.create(Error.prototype); - - return ArgumentOutOfRangeErrorImpl; -})(); - /** * An error thrown when an element was queried at a certain index of an * Observable, but no such index or position exists in that sequence. @@ -28,4 +17,6 @@ const ArgumentOutOfRangeErrorImpl = (() => { * * @class ArgumentOutOfRangeError */ -export const ArgumentOutOfRangeError: ArgumentOutOfRangeErrorCtor = ArgumentOutOfRangeErrorImpl as any; \ No newline at end of file +export const ArgumentOutOfRangeError: ArgumentOutOfRangeErrorCtor = createErrorClass('ArgumentOutOfRangeError', function () { + this.message = 'argument out of range'; +}); diff --git a/src/internal/util/EmptyError.ts b/src/internal/util/EmptyError.ts index f7609083786..492f6e41ca5 100644 --- a/src/internal/util/EmptyError.ts +++ b/src/internal/util/EmptyError.ts @@ -1,3 +1,5 @@ +import { createErrorClass } from './createErrorClass'; + export interface EmptyError extends Error { } @@ -5,19 +7,6 @@ export interface EmptyErrorCtor { new(): EmptyError; } -const EmptyErrorImpl = (() => { - function EmptyErrorImpl(this: Error) { - Error.call(this); - this.message = 'no elements in sequence'; - this.name = 'EmptyError'; - return this; - } - - EmptyErrorImpl.prototype = Object.create(Error.prototype); - - return EmptyErrorImpl; -})(); - /** * An error thrown when an Observable or a sequence was queried but has no * elements. @@ -28,4 +17,6 @@ const EmptyErrorImpl = (() => { * * @class EmptyError */ -export const EmptyError: EmptyErrorCtor = EmptyErrorImpl as any; \ No newline at end of file +export const EmptyError: EmptyErrorCtor = createErrorClass('EmptyError', function () { + this.message = 'no elements in sequence'; +}); \ No newline at end of file diff --git a/src/internal/util/NotFoundError.ts b/src/internal/util/NotFoundError.ts index f97c0ce8ed6..c198aa761ff 100644 --- a/src/internal/util/NotFoundError.ts +++ b/src/internal/util/NotFoundError.ts @@ -1,23 +1,12 @@ -export interface NotFoundError extends Error { -} +/** @prettier */ +import { createErrorClass } from './createErrorClass'; + +export interface NotFoundError extends Error {} export interface NotFoundErrorCtor { - new(message: string): NotFoundError; + new (message: string): NotFoundError; } -const NotFoundErrorImpl = (() => { - function NotFoundErrorImpl(this: Error, message: string) { - Error.call(this); - this.message = message; - this.name = 'NotFoundError'; - return this; - } - - NotFoundErrorImpl.prototype = Object.create(Error.prototype); - - return NotFoundErrorImpl; -})(); - /** * An error thrown when a value or values are missing from an * observable sequence. @@ -26,4 +15,6 @@ const NotFoundErrorImpl = (() => { * * @class NotFoundError */ -export const NotFoundError: NotFoundErrorCtor = NotFoundErrorImpl as any; +export const NotFoundError: NotFoundErrorCtor = createErrorClass('NotFoundError', function (message: string) { + this.message = message; +}); diff --git a/src/internal/util/ObjectUnsubscribedError.ts b/src/internal/util/ObjectUnsubscribedError.ts index 1d603e4cfff..33432d4af86 100644 --- a/src/internal/util/ObjectUnsubscribedError.ts +++ b/src/internal/util/ObjectUnsubscribedError.ts @@ -1,23 +1,12 @@ -export interface ObjectUnsubscribedError extends Error { -} +/** @prettier */ +import { createErrorClass } from './createErrorClass'; + +export interface ObjectUnsubscribedError extends Error {} export interface ObjectUnsubscribedErrorCtor { - new(): ObjectUnsubscribedError; + new (): ObjectUnsubscribedError; } -const ObjectUnsubscribedErrorImpl = (() => { - function ObjectUnsubscribedErrorImpl(this: Error) { - Error.call(this); - this.message = 'object unsubscribed'; - this.name = 'ObjectUnsubscribedError'; - return this; - } - - ObjectUnsubscribedErrorImpl.prototype = Object.create(Error.prototype); - - return ObjectUnsubscribedErrorImpl; -})(); - /** * An error thrown when an action is invalid because the object has been * unsubscribed. @@ -27,4 +16,6 @@ const ObjectUnsubscribedErrorImpl = (() => { * * @class ObjectUnsubscribedError */ -export const ObjectUnsubscribedError: ObjectUnsubscribedErrorCtor = ObjectUnsubscribedErrorImpl as any; \ No newline at end of file +export const ObjectUnsubscribedError: ObjectUnsubscribedErrorCtor = createErrorClass('ObjectUnsubscribedError', function () { + this.message = 'object unsubscribed'; +}); diff --git a/src/internal/util/SequenceError.ts b/src/internal/util/SequenceError.ts index 01379d7e719..6a43250aa21 100644 --- a/src/internal/util/SequenceError.ts +++ b/src/internal/util/SequenceError.ts @@ -1,23 +1,12 @@ -export interface SequenceError extends Error { -} +/** @prettier */ +import { createErrorClass } from './createErrorClass'; + +export interface SequenceError extends Error {} export interface SequenceErrorCtor { - new(message: string): SequenceError; + new (message: string): SequenceError; } -const SequenceErrorImpl = (() => { - function SequenceErrorImpl(this: Error, message: string) { - Error.call(this); - this.message = message; - this.name = 'SequenceError'; - return this; - } - - SequenceErrorImpl.prototype = Object.create(Error.prototype); - - return SequenceErrorImpl; -})(); - /** * An error thrown when something is wrong with the sequence of * values arriving on the observable. @@ -26,4 +15,6 @@ const SequenceErrorImpl = (() => { * * @class SequenceError */ -export const SequenceError: SequenceErrorCtor = SequenceErrorImpl as any; +export const SequenceError: SequenceErrorCtor = createErrorClass('SequenceError', function (message: string) { + this.message = message; +}); diff --git a/src/internal/util/UnsubscriptionError.ts b/src/internal/util/UnsubscriptionError.ts index 1e1e9ed7881..48b5c2b6607 100644 --- a/src/internal/util/UnsubscriptionError.ts +++ b/src/internal/util/UnsubscriptionError.ts @@ -1,29 +1,23 @@ +/** @prettier */ +import { createErrorClass } from './createErrorClass'; + export interface UnsubscriptionError extends Error { readonly errors: any[]; } export interface UnsubscriptionErrorCtor { - new(errors: any[]): UnsubscriptionError; + new (errors: any[]): UnsubscriptionError; } -const UnsubscriptionErrorImpl = (() => { - function UnsubscriptionErrorImpl(this: Error, errors: (Error|string)[]) { - Error.call(this); - this.message = errors ? - `${errors.length} errors occurred during unsubscription: -${errors.map((err, i) => `${i + 1}) ${err.toString()}`).join('\n ')}` : ''; - this.name = 'UnsubscriptionError'; - (this as any).errors = errors; - return this; - } - - UnsubscriptionErrorImpl.prototype = Object.create(Error.prototype); - - return UnsubscriptionErrorImpl; -})(); - /** * An error thrown when one or more errors have occurred during the * `unsubscribe` of a {@link Subscription}. */ -export const UnsubscriptionError: UnsubscriptionErrorCtor = UnsubscriptionErrorImpl as any; +export const UnsubscriptionError: UnsubscriptionErrorCtor = createErrorClass('', function (errors: (Error | string)[]) { + this.message = errors + ? `${errors.length} errors occurred during unsubscription: +${errors.map((err, i) => `${i + 1}) ${err.toString()}`).join('\n ')}` + : ''; + this.name = 'UnsubscriptionError'; + (this as any).errors = errors; +}); diff --git a/src/internal/util/createErrorClass.ts b/src/internal/util/createErrorClass.ts new file mode 100644 index 00000000000..3453c002c9f --- /dev/null +++ b/src/internal/util/createErrorClass.ts @@ -0,0 +1,15 @@ +/** @prettier */ + +export function createErrorClass(name: string, setup: (this: Error, ...args: any[]) => void): T { + function ErrorImpl(this: Error, ...args: any[]) { + Error.call(this); + this.stack = new Error().stack; + this.name = name; + setup.apply(this, args); + return this; + } + + ErrorImpl.prototype = Object.create(Error.prototype); + + return ErrorImpl as any; +}