From 5c8c9dc223fe603de6ef01a51260c4a69b3cbcba Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Thu, 14 Mar 2019 05:46:55 -0400 Subject: [PATCH] feat(swaps): handle taker getRoutes exceptions This adds logic to catch any exceptions from the lnd `getRoutes` call and properly log and set the failure reason. Previously, it was possible for `getRoutes` to throw an exception when no path exists which would be swallowed when called by the swap taker. --- lib/swaps/Swaps.ts | 45 +++++++++++++++++++++++----------- test/integration/Swaps.spec.ts | 12 ++++----- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts index 8ee823beb..3940630ce 100644 --- a/lib/swaps/Swaps.ts +++ b/lib/swaps/Swaps.ts @@ -191,29 +191,46 @@ class Swaps extends EventEmitter { throw new Error(`${currency} client's pubKey not found for peer ${peerPubKey}`); } req.setPubKey(pubKey); - const routes = (await client.queryRoutes(req)).getRoutesList(); - this.logger.debug(`got ${routes.length} routes to destination: ${routes}`); - return routes; + try { + const routes = (await client.queryRoutes(req)).getRoutesList(); + this.logger.debug(`got ${routes.length} routes to destination: ${routes}`); + return routes; + } catch (err) { + this.logger.debug(`error calling getRoutes for ${currency} to ${pubKey}: ${JSON.stringify(err)}`); + if (typeof err.message === 'string' && ( + err.message.includes('unable to find a path to destination') || + err.message.includes('target not found') + )) { + return []; + } else { + throw err; + } + } } /** * Checks if a swap for two given orders can be executed. * @param maker maker order * @param taker taker order - * @returns nothing if the swap can be executed, throws an error with a reason otherwise + * @returns `void` if the swap can be executed, throws a [[SwapFailureReason]] otherwise */ - private verifyExecution = async (maker: PeerOrder, taker: OwnOrder) => { + private verifyExecution = async (maker: PeerOrder, taker: OwnOrder): Promise => { if (maker.pairId !== taker.pairId || !this.isPairSupported(maker.pairId)) { - throw new Error('pairId does not match or pair is not supported'); + throw SwapFailureReason.SwapClientNotSetup; } // TODO: right now we only verify that a route exists when we are the taker, we should // generalize the logic below to work for when we are the maker as well const { makerCurrency } = Swaps.deriveCurrencies(maker.pairId, maker.isBuy); const { makerAmount } = Swaps.calculateSwapAmounts(taker.quantity, maker.price, maker.isBuy); - const routes = await this.getRoutes(makerCurrency, makerAmount, maker.peerPubKey); + let routes: lndrpc.Route[]; + try { + routes = await this.getRoutes(makerCurrency, makerAmount, maker.peerPubKey); + } catch (err) { + throw SwapFailureReason.UnexpectedLndError; + } if (routes.length === 0) { - throw new Error('Can not swap. unable to find route to destination'); + throw SwapFailureReason.NoRouteFound; } } @@ -385,13 +402,13 @@ class Swaps extends EventEmitter { try { deal.makerToTakerRoutes = await this.getRoutes(deal.takerCurrency, takerAmount, deal.peerPubKey); - if (deal.makerToTakerRoutes.length === 0) throw new Error(); + if (deal.makerToTakerRoutes.length === 0) { + this.failDeal(deal, SwapFailureReason.NoRouteFound, 'Unable to find route to destination'); + await this.sendErrorToPeer(peer, deal.rHash, deal.failureReason!, deal.errorMessage, requestPacket.header.id); + return false; + } } catch (err) { - const cannotSwap = 'Unable to find route to destination'; - const errMsg = err && err.message - ? `${cannotSwap}` - : `${cannotSwap}: ${err.message}`; - this.failDeal(deal, SwapFailureReason.NoRouteFound, errMsg); + this.failDeal(deal, SwapFailureReason.UnexpectedLndError, err.message); await this.sendErrorToPeer(peer, deal.rHash, deal.failureReason!, deal.errorMessage, requestPacket.header.id); return false; } diff --git a/test/integration/Swaps.spec.ts b/test/integration/Swaps.spec.ts index 7fed1f9f0..ce722cfdf 100644 --- a/test/integration/Swaps.spec.ts +++ b/test/integration/Swaps.spec.ts @@ -8,6 +8,7 @@ import LndClient from '../../lib/lndclient/LndClient'; import Logger, { Level } from '../../lib/Logger'; import DB from '../../lib/db/DB'; import { waitForSpy } from '../utils'; +import { SwapFailureReason } from '../../lib/constants/enums'; chai.use(chaiAsPromised); @@ -170,13 +171,13 @@ describe('Swaps.Integration', () => { pairId: INVALID_PAIR_ID, }; expect(swaps.executeSwap(invalidMakerOrder, validTakerOrder())) - .to.eventually.be.rejectedWith('pairId does not match or pair is not supported'); + .to.eventually.be.rejected.and.equal(SwapFailureReason.SwapClientNotSetup); const invalidTakerOrder = { ...validTakerOrder(), pairId: INVALID_PAIR_ID, }; expect(swaps.executeSwap(validMakerOrder(), invalidTakerOrder)) - .to.eventually.be.rejectedWith('pairId does not match or pair is not supported'); + .to.eventually.be.rejected.and.equal(SwapFailureReason.SwapClientNotSetup); }); it('will reject if unable to retrieve routes', async () => { @@ -188,15 +189,14 @@ describe('Swaps.Integration', () => { lndClients.BTC!.queryRoutes = noRoutesFound; lndClients.LTC!.queryRoutes = noRoutesFound; expect(swaps.executeSwap(validMakerOrder(), validTakerOrder())) - .to.eventually.be.rejectedWith('Can not swap. unable to find route to destination'); - const EXPECTED_ERROR_MSG = 'UNKNOWN'; + .to.eventually.be.rejected.and.equal(SwapFailureReason.NoRouteFound); const rejectsWithUnknownError = () => { - return Promise.reject(EXPECTED_ERROR_MSG); + return Promise.reject('UNKNOWN'); }; lndClients.BTC!.queryRoutes = rejectsWithUnknownError; lndClients.LTC!.queryRoutes = rejectsWithUnknownError; expect(swaps.executeSwap(validMakerOrder(), validTakerOrder())) - .to.eventually.be.rejectedWith(EXPECTED_ERROR_MSG); + .to.eventually.be.rejected.and.equal(SwapFailureReason.UnexpectedLndError); }); });