Skip to content

Commit

Permalink
refactor: remove most uses of tryCatch helper (#4492)
Browse files Browse the repository at this point in the history 
The `tryCatch` helper is a leftover from the CrankshaftScript days and
is considered bad practice for modern JavaScript engines. Same for the
`errorObject` trick.
  • Loading branch information
@bmeurer @benlesh
bmeurer authored and benlesh committed Jan 29, 2019
1 parent a6c0017 commit e691379
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 201 deletions.
24 changes: 10 additions & 14 deletions src/internal/Subscription.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { isArray } from './util/isArray';
import { isObject } from './util/isObject';
import { isFunction } from './util/isFunction';
import { tryCatch } from './util/tryCatch';
import { errorObject } from './util/errorObject';
import { UnsubscriptionError } from './util/UnsubscriptionError';
import { SubscriptionLike, TeardownLogic } from './types';

Expand Down Expand Up @@ -84,13 +82,11 @@ export class Subscription implements SubscriptionLike {
}

if (isFunction(_unsubscribe)) {
let trial = tryCatch(_unsubscribe).call(this);
if (trial === errorObject) {
try {
_unsubscribe.call(this);
} catch (e) {
hasErrors = true;
errors = errors || (
errorObject.e instanceof UnsubscriptionError ?
flattenUnsubscriptionErrors(errorObject.e.errors) : [errorObject.e]
);
errors = e instanceof UnsubscriptionError ? flattenUnsubscriptionErrors(e.errors) : [e];
}
}

Expand All @@ -102,15 +98,15 @@ export class Subscription implements SubscriptionLike {
while (++index < len) {
const sub = _subscriptions[index];
if (isObject(sub)) {
let trial = tryCatch(sub.unsubscribe).call(sub);
if (trial === errorObject) {
try {
sub.unsubscribe();
} catch (e) {
hasErrors = true;
errors = errors || [];
let err = errorObject.e;
if (err instanceof UnsubscriptionError) {
errors = errors.concat(flattenUnsubscriptionErrors(err.errors));
if (e instanceof UnsubscriptionError) {
errors = errors.concat(flattenUnsubscriptionErrors(e.errors));
} else {
errors.push(err);
errors.push(e);
}
}
}
Expand Down
83 changes: 37 additions & 46 deletions src/internal/observable/dom/AjaxObservable.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { root } from '../../util/root';
import { tryCatch } from '../../util/tryCatch';
import { errorObject } from '../../util/errorObject';
import { Observable } from '../../Observable';
import { Subscriber } from '../../Subscriber';
import { TeardownLogic } from '../../types';
Expand Down Expand Up @@ -221,43 +219,33 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
next(e: Event): void {
this.done = true;
const { xhr, request, destination } = this;
const response = new AjaxResponse(e, xhr, request);
if (response.response === errorObject) {
destination.error(errorObject.e);
} else {
destination.next(response);
let result;
try {
result = new AjaxResponse(e, xhr, request);
} catch (err) {
return destination.error(err);
}
destination.next(result);
}

private send(): XMLHttpRequest {
private send(): void {
const {
request,
request: { user, method, url, async, password, headers, body }
} = this;
const createXHR = request.createXHR;
const xhr: XMLHttpRequest = tryCatch(createXHR).call(request);

if (<any>xhr === errorObject) {
this.error(errorObject.e);
} else {
this.xhr = xhr;
try {
const xhr = this.xhr = request.createXHR();

// set up the events before open XHR
// https://developer.mozilla.org/en/docs/Web/API/XMLHttpRequest/Using_XMLHttpRequest
// You need to add the event listeners before calling open() on the request.
// Otherwise the progress events will not fire.
this.setupEvents(xhr, request);
// open XHR
let result: any;
if (user) {
result = tryCatch(xhr.open).call(xhr, method, url, async, user, password);
xhr.open(method, url, async, user, password);
} else {
result = tryCatch(xhr.open).call(xhr, method, url, async);
}

if (result === errorObject) {
this.error(errorObject.e);
return null;
xhr.open(method, url, async);
}

// timeout, responseType and withCredentials can be set once the XHR is open
Expand All @@ -274,14 +262,14 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
this.setHeaders(xhr, headers);

// finally send the request
result = body ? tryCatch(xhr.send).call(xhr, body) : tryCatch(xhr.send).call(xhr);
if (result === errorObject) {
this.error(errorObject.e);
return null;
if (body) {
xhr.send(body);
} else {
xhr.send();
}
} catch (err) {
this.error(err);
}

return xhr;
}

private serializeBody(body: any, contentType?: string) {
Expand Down Expand Up @@ -329,17 +317,18 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
private setupEvents(xhr: XMLHttpRequest, request: AjaxRequest) {
const progressSubscriber = request.progressSubscriber;

function xhrTimeout(this: XMLHttpRequest, e: ProgressEvent) {
function xhrTimeout(this: XMLHttpRequest, e: ProgressEvent): void {
const {subscriber, progressSubscriber, request } = (<any>xhrTimeout);
if (progressSubscriber) {
progressSubscriber.error(e);
}
const ajaxTimeoutError = new AjaxTimeoutError(this, request); //TODO: Make betterer.
if (ajaxTimeoutError.response === errorObject) {
subscriber.error(errorObject.e);
} else {
subscriber.error(ajaxTimeoutError);
let error;
try {
error = new AjaxTimeoutError(this, request); // TODO: Make betterer.
} catch (err) {
error = err;
}
subscriber.error(error);
}
xhr.ontimeout = xhrTimeout;
(<any>xhrTimeout).request = request;
Expand All @@ -365,12 +354,13 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
if (progressSubscriber) {
progressSubscriber.error(e);
}
const ajaxError = new AjaxError('ajax error', this, request);
if (ajaxError.response === errorObject) {
subscriber.error(errorObject.e);
} else {
subscriber.error(ajaxError);
let error;
try {
error = new AjaxError('ajax error', this, request);
} catch (err) {
error = err;
}
subscriber.error(error);
};
xhr.onerror = xhrError;
(<any>xhrError).request = request;
Expand Down Expand Up @@ -412,12 +402,13 @@ export class AjaxSubscriber<T> extends Subscriber<Event> {
if (progressSubscriber) {
progressSubscriber.error(e);
}
const ajaxError = new AjaxError('ajax error ' + status, this, request);
if (ajaxError.response === errorObject) {
subscriber.error(errorObject.e);
} else {
subscriber.error(ajaxError);
let error;
try {
error = new AjaxError('ajax error ' + status, this, request);
} catch (err) {
error = err;
}
subscriber.error(error);
}
}
}
Expand Down Expand Up @@ -523,7 +514,7 @@ function parseJson(xhr: XMLHttpRequest) {
function parseXhrResponse(responseType: string, xhr: XMLHttpRequest) {
switch (responseType) {
case 'json':
return tryCatch(parseJson)(xhr);
return parseJson(xhr);
case 'xml':
return xhr.responseXML;
case 'text':
Expand Down
55 changes: 25 additions & 30 deletions src/internal/observable/dom/WebSocketSubject.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import { Subscription } from '../../Subscription';
import { Operator } from '../../Operator';
import { ReplaySubject } from '../../ReplaySubject';
import { Observer, NextObserver } from '../../types';
import { tryCatch } from '../../util/tryCatch';
import { errorObject } from '../../util/errorObject';

export interface WebSocketSubjectConfig<T> {
/** The url of the socket server to connect to */
Expand Down Expand Up @@ -131,30 +129,29 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
multiplex(subMsg: () => any, unsubMsg: () => any, messageFilter: (value: T) => boolean) {
const self = this;
return new Observable((observer: Observer<any>) => {
const result = tryCatch(subMsg)();
if (result === errorObject) {
observer.error(errorObject.e);
} else {
self.next(result);
try {
self.next(subMsg());
} catch (err) {
observer.error(err);
}

let subscription = self.subscribe(x => {
const result = tryCatch(messageFilter)(x);
if (result === errorObject) {
observer.error(errorObject.e);
} else if (result) {
observer.next(x);
const subscription = self.subscribe(x => {
try {
if (messageFilter(x)) {
observer.next(x);
}
} catch (err) {
observer.error(err);
}
},
err => observer.error(err),
() => observer.complete());

return () => {
const result = tryCatch(unsubMsg)();
if (result === errorObject) {
observer.error(errorObject.e);
} else {
self.next(result);
try {
self.next(unsubMsg());
} catch (err) {
observer.error(err);
}
subscription.unsubscribe();
};
Expand Down Expand Up @@ -197,13 +194,12 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
this.destination = Subscriber.create<T>(
(x) => {
if (socket.readyState === 1) {
const { serializer } = this._config;
const msg = tryCatch(serializer)(x);
if (msg === errorObject) {
this.destination.error(errorObject.e);
return;
try {
const { serializer } = this._config;
socket.send(serializer(x));
} catch (e) {
this.destination.error(e);
}
socket.send(msg);
}
},
(e) => {
Expand Down Expand Up @@ -252,12 +248,11 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
};

socket.onmessage = (e: MessageEvent) => {
const { deserializer } = this._config;
const result = tryCatch(deserializer)(e);
if (result === errorObject) {
observer.error(errorObject.e);
} else {
observer.next(result);
try {
const { deserializer } = this._config;
observer.next(deserializer(e));
} catch (err) {
observer.error(err);
}
};
}
Expand Down
22 changes: 11 additions & 11 deletions src/internal/operators/audit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { Observable } from '../Observable';
import { Subscription } from '../Subscription';
import { MonoTypeOperatorFunction, SubscribableOrPromise, TeardownLogic } from '../types';

import { tryCatch } from '../util/tryCatch';
import { errorObject } from '../util/errorObject';
import { OuterSubscriber } from '../OuterSubscriber';
import { subscribeToResult } from '../util/subscribeToResult';

Expand Down Expand Up @@ -87,16 +85,18 @@ class AuditSubscriber<T, R> extends OuterSubscriber<T, R> {
this.value = value;
this.hasValue = true;
if (!this.throttled) {
const duration = tryCatch(this.durationSelector)(value);
if (duration === errorObject) {
this.destination.error(errorObject.e);
let duration;
try {
const { durationSelector } = this;
duration = durationSelector(value);
} catch (err) {
return this.destination.error(err);
}
const innerSubscription = subscribeToResult(this, duration);
if (!innerSubscription || innerSubscription.closed) {
this.clearThrottle();
} else {
const innerSubscription = subscribeToResult(this, duration);
if (!innerSubscription || innerSubscription.closed) {
this.clearThrottle();
} else {
this.add(this.throttled = innerSubscription);
}
this.add(this.throttled = innerSubscription);
}
}
}
Expand Down
26 changes: 12 additions & 14 deletions src/internal/operators/bufferWhen.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import { Operator } from '../Operator';
import { Subscriber } from '../Subscriber';
import { Observable } from '../Observable';
import { Subscription } from '../Subscription';
import { tryCatch } from '../util/tryCatch';
import { errorObject } from '../util/errorObject';
import { OuterSubscriber } from '../OuterSubscriber';
import { InnerSubscriber } from '../InnerSubscriber';
import { subscribeToResult } from '../util/subscribeToResult';
Expand Down Expand Up @@ -112,7 +110,6 @@ class BufferWhenSubscriber<T> extends OuterSubscriber<T, any> {
}

openBuffer() {

let { closingSubscription } = this;

if (closingSubscription) {
Expand All @@ -127,17 +124,18 @@ class BufferWhenSubscriber<T> extends OuterSubscriber<T, any> {

this.buffer = [];

const closingNotifier = tryCatch(this.closingSelector)();

if (closingNotifier === errorObject) {
this.error(errorObject.e);
} else {
closingSubscription = new Subscription();
this.closingSubscription = closingSubscription;
this.add(closingSubscription);
this.subscribing = true;
closingSubscription.add(subscribeToResult(this, closingNotifier));
this.subscribing = false;
let closingNotifier;
try {
const { closingSelector } = this;
closingNotifier = closingSelector();
} catch (err) {
return this.error(err);
}
closingSubscription = new Subscription();
this.closingSubscription = closingSubscription;
this.add(closingSubscription);
this.subscribing = true;
closingSubscription.add(subscribeToResult(this, closingNotifier));
this.subscribing = false;
}
}

0 comments on commit e691379

Please sign in to comment.