Skip to content

Commit

Permalink
fix(errors): Custom RxJS errors now all have a call stack
Browse files Browse the repository at this point in the history
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 ReactiveX#4250
  • Loading branch information
benlesh committed Sep 1, 2020
1 parent c088b0e commit 2409790
Show file tree
Hide file tree
Showing 18 changed files with 113 additions and 155 deletions.
12 changes: 1 addition & 11 deletions spec/helpers/test-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,4 @@ export const createObservableInputs = <T>(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[] = [];
5 changes: 3 additions & 2 deletions spec/operators/concatWith-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
4 changes: 2 additions & 2 deletions spec/operators/single-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
4 changes: 2 additions & 2 deletions spec/operators/takeLast-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
3 changes: 3 additions & 0 deletions spec/util/ArgumentOutOfRangeError-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
3 changes: 3 additions & 0 deletions spec/util/EmptyError-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
3 changes: 3 additions & 0 deletions spec/util/ObjectUnsubscribedError-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
3 changes: 3 additions & 0 deletions spec/util/TimeoutError-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
1 change: 1 addition & 0 deletions spec/util/UnsubscriptionError-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
});
});
45 changes: 22 additions & 23 deletions src/internal/observable/dom/AjaxObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 6 additions & 15 deletions src/internal/operators/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, R = T, M = unknown> {
/**
Expand Down Expand Up @@ -72,20 +73,6 @@ export interface TimeoutErrorCtor {
new <T = unknown, M = unknown>(info?: TimeoutInfo<T, M>): TimeoutError<T, M>;
}

const TimeoutErrorImpl = (() => {
function TimeoutErrorImpl(this: any, info: TimeoutInfo<any> | 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.
*
Expand All @@ -98,7 +85,11 @@ const TimeoutErrorImpl = (() => {
*
* @class TimeoutError
*/
export const TimeoutError: TimeoutErrorCtor = TimeoutErrorImpl as any;
export const TimeoutError: TimeoutErrorCtor = createErrorClass('TimeoutError', function (info: TimeoutInfo<any> | 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
Expand Down
25 changes: 8 additions & 17 deletions src/internal/util/ArgumentOutOfRangeError.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -28,4 +17,6 @@ const ArgumentOutOfRangeErrorImpl = (() => {
*
* @class ArgumentOutOfRangeError
*/
export const ArgumentOutOfRangeError: ArgumentOutOfRangeErrorCtor = ArgumentOutOfRangeErrorImpl as any;
export const ArgumentOutOfRangeError: ArgumentOutOfRangeErrorCtor = createErrorClass('ArgumentOutOfRangeError', function () {
this.message = 'argument out of range';
});
19 changes: 5 additions & 14 deletions src/internal/util/EmptyError.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,12 @@
import { createErrorClass } from './createErrorClass';

export interface EmptyError extends Error {
}

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.
Expand All @@ -28,4 +17,6 @@ const EmptyErrorImpl = (() => {
*
* @class EmptyError
*/
export const EmptyError: EmptyErrorCtor = EmptyErrorImpl as any;
export const EmptyError: EmptyErrorCtor = createErrorClass('EmptyError', function () {
this.message = 'no elements in sequence';
});
25 changes: 8 additions & 17 deletions src/internal/util/NotFoundError.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
});
25 changes: 8 additions & 17 deletions src/internal/util/ObjectUnsubscribedError.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -27,4 +16,6 @@ const ObjectUnsubscribedErrorImpl = (() => {
*
* @class ObjectUnsubscribedError
*/
export const ObjectUnsubscribedError: ObjectUnsubscribedErrorCtor = ObjectUnsubscribedErrorImpl as any;
export const ObjectUnsubscribedError: ObjectUnsubscribedErrorCtor = createErrorClass('ObjectUnsubscribedError', function () {
this.message = 'object unsubscribed';
});
Loading

0 comments on commit 2409790

Please sign in to comment.