Skip to content

Commit

Permalink
Clean up error handling (coinbase#206)
Browse files Browse the repository at this point in the history
* Return a Promise.reject instead of throwing an Error.

* Add GDAXExchangeAPI error handling unit tests.

These describe the current state.

* Return a Promise.reject() instead of throwing an HTTPError.

* Rename GTTError's cause parameter.

* SimpleRateCalculator.ts: fix indentation.

* Clean up code involving Promise.reject().
  • Loading branch information
blair authored and fb55 committed Apr 14, 2018
1 parent da0df0e commit fe89dcb
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 20 deletions.
6 changes: 3 additions & 3 deletions src/FXService/calculators/SimpleRateCalculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ export default class SimpleRateCalculator extends FXRateCalculator {
const promises: Promise<FXObject>[] = pairs.map((pair: CurrencyPair) => {
return this.provider.fetchCurrentRate(pair)
.catch((err: Error) => {
this.logger.log('warn', err.message, (err as any).details || null);
return null;
});
this.logger.log('warn', err.message, (err as any).details || null);
return null;
});
});
// Wait for all promises to resolve before sending results back
return Promise.all(promises);
Expand Down
2 changes: 1 addition & 1 deletion src/FXService/providers/YahooFXProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default class YahooFinanceFXProvider extends FXProvider {
}
});
return this.isSupportedPair(pair);
}, (err: Error) => Promise.reject(err));
});
}

protected downloadCurrentRate(pair: CurrencyPair): Promise<FXObject> {
Expand Down
2 changes: 1 addition & 1 deletion src/exchanges/bitfinex/BitfinexExchangeAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ export class BitfinexExchangeAPI implements PublicExchangeAPI, AuthenticatedExch
}

loadCandles(options: CandleRequestOptions): Promise<Candle[]> {
return Promise.reject(new GTTError('Error loading products from Bitfinex'));
return Promise.reject(new GTTError('Not implemented'));
}

// -------------------------- Transfer methods -------------------------------------------------
Expand Down
2 changes: 0 additions & 2 deletions src/exchanges/bittrex/BittrexFeed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ export class BittrexFeed extends ExchangeFeed {
})).then(() => {
// Every result is guaranteed to be true
return true;
}).catch((err) => {
return Promise.reject(err);
});
}

Expand Down
16 changes: 8 additions & 8 deletions src/exchanges/ccxt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,19 +329,19 @@ export default class CCXTExchangeWrapper implements PublicExchangeAPI, Authentic
}

cancelOrder(id: string): Promise<string> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

cancelAllOrders(gdaxProduct?: string): Promise<string[]> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

loadOrder(id: string): Promise<LiveOrder> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

loadAllOrders(gdaxProduct?: string): Promise<LiveOrder[]> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

loadBalances(): Promise<Balances> {
Expand Down Expand Up @@ -369,19 +369,19 @@ export default class CCXTExchangeWrapper implements PublicExchangeAPI, Authentic
}

requestCryptoAddress(cur: string): Promise<CryptoAddress> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

requestTransfer(request: TransferRequest): Promise<TransferResult> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

requestWithdrawal(request: WithdrawalRequest): Promise<TransferResult> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

transfer(cur: string, amount: BigJS, from: string, to: string, options: any): Promise<TransferResult> {
throw new Error('Not implemented yet');
return Promise.reject(new Error('Not implemented yet'));
}

/**
Expand Down
7 changes: 4 additions & 3 deletions src/exchanges/gdax/GDAXExchangeAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ export class GDAXExchangeAPI implements PublicExchangeAPI, AuthenticatedExchange
loadMidMarketPrice(product: string): Promise<BigJS> {
return this.loadTicker(product).then((ticker) => {
if (!ticker || !ticker.bid || !ticker.ask) {
throw new HTTPError(`Loading midmarket price for ${product} failed because ticker data was incomplete or unavailable`, {status: 200, body: ticker});
const err = new HTTPError(`Loading midmarket price for ${product} failed because ticker data was incomplete or unavailable`, {status: 200, body: ticker});
return Promise.reject(err);
}
return ticker.ask.plus(ticker.bid).times(0.5);
return Promise.resolve(ticker.ask.plus(ticker.bid).times(0.5));
});
}

Expand Down Expand Up @@ -246,7 +247,7 @@ export class GDAXExchangeAPI implements PublicExchangeAPI, AuthenticatedExchange
} as OrderParams; // Override for incomplete definition in GDAX lib
break;
default:
return Promise.reject(new GTTError('Invalid Order type: ' + order.type));
return Promise.reject(new GTTError(`Invalid order type: ${order.type}`));
}

return this.authClient.placeOrder(gdaxOrder).then((result: OrderResult) => {
Expand Down
4 changes: 2 additions & 2 deletions src/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export class GTTError extends Error implements StreamError {
readonly cause: Error;
readonly time: Date;

constructor(msg: string, err?: Error) {
constructor(msg: string, cause?: Error) {
super(msg);
this.cause = err;
this.cause = cause;
this.time = new Date();
}

Expand Down
27 changes: 27 additions & 0 deletions test/exchanges/GDAXExchangeTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,33 @@ describe('GDAX Authenticated Exchange API', () => {
gdax = new GDAXExchangeAPI({logger: null, auth: {key: 'key', secret: 'secret', passphrase: 'pass'}});
});

it('handles errors from the gdax module', () => {
nock('https://api.gdax.com', {encodedQueryParams: true})
.get('/products')
.reply(400, {reason: 'Fake failure'});
return gdax.loadProducts().then((products) => {
return Promise.reject(`Unexpected success ${products}`);
}, (err) => {
assert(err);
assert(err.response);
assert.equal(err.response.status, undefined);
assert.equal(err.response.body,
JSON.stringify({reason: 'Fake failure'}));
});
});

it('handles errors when it does API calls', () => {
nock('https://api.gdax.com', {encodedQueryParams: true})
.delete('/orders/foobar')
.reply(400, {reason: 'Fake failure'});
return gdax.cancelOrder('foobar').then((id) => {
return Promise.reject(`Unexpected success ${id}`);
}, (err) => {
assert(err);
assert(!err.response);
});
});

it('returns a crypto address', () => {
nock('https://api.gdax.com', {encodedQueryParams: true})
.get('/coinbase-accounts')
Expand Down

0 comments on commit fe89dcb

Please sign in to comment.