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 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 ReactiveX#4250
  • Loading branch information
benlesh committed Sep 1, 2020
1 parent c088b0e commit d3cc139
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 d3cc139

Please sign in to comment.