From 2409790c0cd31c0c98191e1c4df9b37f03353c63 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 implementation at the top level. This will go away as we 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; +}