Fix: Emit error events and return error on pathfind

src/api/common/errors.js

@@ -77,13 +77,13 @@ ApiError.prototype = new RippleError();
 ApiError.prototype.name = 'ApiError';
 
 module.exports = {
-  ValidationError: ValidationError,
-  NetworkError: NetworkError,
-  TransactionError: TransactionError,
-  RippledNetworkError: RippledNetworkError,
-  NotFoundError: NotFoundError,
-  MissingLedgerHistoryError: MissingLedgerHistoryError,
-  TimeOutError: TimeOutError,
-  ApiError: ApiError,
-  RippleError: RippleError
+  ValidationError,
+  NetworkError,
+  TransactionError,
+  RippledNetworkError,
+  NotFoundError,
+  MissingLedgerHistoryError,
+  TimeOutError,
+  ApiError,
+  RippleError
 };

src/api/common/index.js

@@ -12,6 +12,7 @@ module.exports = {
   generateAddress: utils.generateAddress,
   composeAsync: utils.composeAsync,
   wrapCatch: utils.wrapCatch,
+  convertErrors: utils.convertErrors,
   convertExceptions: utils.convertExceptions,
   convertKeysFromSnakeCaseToCamelCase:
     utils.convertKeysFromSnakeCaseToCamelCase,

src/api/common/utils.js

@@ -66,6 +66,16 @@ function composeAsync(wrapper: Wrapper, callback: Callback): Callback {
   };
 }
 
+function convertErrors(callback: () => void): () => void {
+  return function(error, data) {
+    if (error && !(error instanceof errors.RippleError)) {
+      callback(new errors.RippleError(error));
+    } else {
+      callback(error, data);
+    }
+  };
+}
+
 function convertExceptions<T>(f: () => T): () => T {
   return function() {
     try {
@@ -105,6 +115,7 @@ module.exports = {
   composeAsync,
   wrapCatch,
   convertExceptions,
+  convertErrors,
   convertKeysFromSnakeCaseToCamelCase,
   promisify
 };

src/api/ledger/accountinfo.js

@@ -5,6 +5,7 @@ const utils = require('./utils');
 const removeUndefined = require('./parse/utils').removeUndefined;
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 
 type AccountData = {
   Sequence: number,
@@ -66,7 +67,7 @@ function getAccountInfoAsync(account: string, options: AccountInfoOptions,
   };
 
   this.remote.requestAccountInfo(request,
-    composeAsync(formatAccountInfo, callback));
+    composeAsync(formatAccountInfo, convertErrors(callback)));
 }
 
 function getAccountInfo(account: string, options: AccountInfoOptions = {}

src/api/ledger/balances.js

@@ -6,6 +6,7 @@ const utils = require('./utils');
 const getTrustlines = require('./trustlines');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 
 function getTrustlineBalanceAmount(trustline) {
   return {
@@ -39,7 +40,7 @@ function getBalancesAsync(account, options, callback) {
   async.parallel({
     xrp: _.partial(utils.getXRPBalance, this.remote, account, ledgerVersion),
     trustlines: _.partial(getTrustlinesAsync.bind(this), account, options)
-  }, composeAsync(formatBalances, callback));
+  }, composeAsync(formatBalances, convertErrors(callback)));
 }
 
 function getBalances(account: string, options = {}) {

src/api/ledger/ledger.js

@@ -3,6 +3,7 @@
 const utils = require('./utils');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 const parseLedger = require('./parse/ledger');
 
 function getLedgerAsync(options, callback) {
@@ -16,7 +17,8 @@ function getLedgerAsync(options, callback) {
   };
 
   this.remote.requestLedger(request,
-    composeAsync(response => parseLedger(response.ledger), callback));
+    composeAsync(response => parseLedger(response.ledger),
+    convertErrors(callback)));
 }
 
 function getLedger(options = {}) {

src/api/ledger/orderbook.js

@@ -3,9 +3,10 @@
 const _ = require('lodash');
 const async = require('async');
 const utils = require('./utils');
-const parseOrderbookOrder = require('./parse/orderbook-order');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
+const parseOrderbookOrder = require('./parse/orderbook-order');
 
 // account is to specify a "perspective", which affects which unfunded offers
 // are returned
@@ -18,7 +19,7 @@ function getBookOffers(remote, account, ledgerVersion, limit,
     ledger: ledgerVersion || 'validated',
     limit: limit,
     taker: account
-  }), composeAsync(data => data.offers, callback));
+  }), composeAsync(data => data.offers, convertErrors(callback)));
 }
 
 function isSameIssue(a, b) {

src/api/ledger/orders.js

@@ -4,10 +4,11 @@ const _ = require('lodash');
 const utils = require('./utils');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 const parseAccountOrder = require('./parse/account-order');
 
-function requestAccountOffers(remote, address, ledgerVersion, options,
-    marker, limit, callback
+function requestAccountOffers(remote, address, ledgerVersion, marker, limit,
+  callback
 ) {
   remote.requestAccountOffers({
     account: address,
@@ -18,7 +19,7 @@ function requestAccountOffers(remote, address, ledgerVersion, options,
   composeAsync((data) => ({
     marker: data.marker,
     results: data.offers.map(_.partial(parseAccountOrder, address))
-  }), callback));
+  }), convertErrors(callback)));
 }
 
 function getOrdersAsync(account, options, callback) {
@@ -28,7 +29,7 @@ function getOrdersAsync(account, options, callback) {
   const ledgerVersion = options.ledgerVersion
                       || this.remote.getLedgerSequence();
   const getter = _.partial(requestAccountOffers, this.remote, account,
-                           ledgerVersion, options);
+                           ledgerVersion);
   utils.getRecursive(getter, options.limit,
     composeAsync((orders) => _.sortBy(orders,
       (order) => order.properties.sequence), callback));

src/api/ledger/pathfind.js

@@ -8,6 +8,7 @@ const validate = utils.common.validate;
 const parsePathfind = require('./parse/pathfind');
 const NotFoundError = utils.common.errors.NotFoundError;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 
 type PathFindParams = {
   src_currencies?: Array<string>, src_account: string, dst_amount: string,
@@ -47,7 +48,7 @@ function requestPathFind(remote, pathfind: PathFind, callback) {
   }
 
   remote.createPathFind(params,
-    composeAsync(_.partial(addParams, params), callback));
+    composeAsync(_.partial(addParams, params), convertErrors(callback)));
 }
 
 function addDirectXrpPath(paths, xrpBalance) {

src/api/ledger/settings.js

@@ -6,6 +6,7 @@ const validate = utils.common.validate;
 const parseFields = require('./parse/fields');
 const composeAsync = utils.common.composeAsync;
 const AccountFlags = utils.common.constants.AccountFlags;
+const convertErrors = utils.common.convertErrors;
 
 function parseFlags(value) {
   const settings = {};
@@ -34,7 +35,7 @@ function getSettingsAsync(account, options, callback) {
   };
 
   this.remote.requestAccountInfo(request,
-    composeAsync(formatSettings, callback));
+    composeAsync(formatSettings, convertErrors(callback)));
 }
 
 function getSettings(account: string, options = {}) {

src/api/ledger/transaction.js

@@ -6,6 +6,7 @@ const utils = require('./utils');
 const parseTransaction = require('./parse/transaction');
 const validate = utils.common.validate;
 const errors = utils.common.errors;
+const convertErrors = utils.common.convertErrors;
 const RippleError = require('../../core/rippleerror').RippleError;
 
 import type {Remote} from '../../core/remote';
@@ -69,9 +70,9 @@ function getTransactionAsync(identifier: string, options: TransactionOptions,
     } else if (!error && tx && !isTransactionInRange(tx, options)) {
       callback(new errors.NotFoundError('Transaction not found'));
     } else if (error) {
-      callback(error);
+      convertErrors(callback)(error);
     } else if (!tx) {
-      callback(new Error('Internal error'));
+      callback(new errors.ApiError('Internal error'));
     } else {
       callback(error, parseTransaction(tx));
     }

src/api/ledger/transactions.js

@@ -7,6 +7,7 @@ const parseTransaction = require('./parse/transaction');
 const getTransaction = require('./transaction');
 const validate = utils.common.validate;
 const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 
 function parseAccountTxTransaction(tx) {
   // rippled uses a different response format for 'account_tx' than 'tx'
@@ -55,6 +56,17 @@ function orderFilter(options, tx) {
     utils.compareTransactions(tx, options.startTx) < 0);
 }
 
+function formatPartialResponse(address, options, data) {
+  return {
+    marker: data.marker,
+    results: data.transactions
+      .filter((tx) => tx.validated)
+      .map(parseAccountTxTransaction)
+      .filter(_.partial(transactionFilter, address, options))
+      .filter(_.partial(orderFilter, options))
+  };
+}
+
 function getAccountTx(remote, address, options, marker, limit, callback) {
   const params = {
     account: address,
@@ -66,16 +78,9 @@ function getAccountTx(remote, address, options, marker, limit, callback) {
     marker: marker
   };
 
-  remote.requestAccountTx(params, (error, data) => {
-    return error ? callback(error) : callback(null, {
-      marker: data.marker,
-      results: data.transactions
-        .filter((tx) => tx.validated)
-        .map(parseAccountTxTransaction)
-        .filter(_.partial(transactionFilter, address, options))
-        .filter(_.partial(orderFilter, options))
-    });
-  });
+  remote.requestAccountTx(params,
+    composeAsync(_.partial(formatPartialResponse, address, options),
+      convertErrors(callback)));
 }
 
 function checkForLedgerGaps(remote, options, transactions) {

src/api/ledger/trustlines.js

@@ -3,12 +3,22 @@
 const _ = require('lodash');
 const utils = require('./utils');
 const validate = utils.common.validate;
+const composeAsync = utils.common.composeAsync;
+const convertErrors = utils.common.convertErrors;
 const parseAccountTrustline = require('./parse/account-trustline');
 
 function currencyFilter(currency, trustline) {
   return currency === null || trustline.specification.currency === currency;
 }
 
+function formatResponse(options, data) {
+  return {
+    marker: data.marker,
+    results: data.lines.map(parseAccountTrustline)
+      .filter(_.partial(currencyFilter, options.currency || null))
+  };
+}
+
 function getAccountLines(remote, address, ledgerVersion, options, marker, limit,
     callback
 ) {
@@ -20,14 +30,9 @@ function getAccountLines(remote, address, ledgerVersion, options, marker, limit,
     peer: options.counterparty
   };
 
-  remote.requestAccountLines(requestOptions, (error, data) => {
-    return error ? callback(error) :
-      callback(null, {
-        marker: data.marker,
-        results: data.lines.map(parseAccountTrustline)
-          .filter(_.partial(currencyFilter, options.currency || null))
-      });
-  });
+  remote.requestAccountLines(requestOptions,
+    composeAsync(_.partial(formatResponse, options),
+      convertErrors(callback)));
 }
 
 function getTrustlinesAsync(account: string, options: {currency: string,

src/api/transaction/submit.js

@@ -3,16 +3,17 @@
 const utils = require('./utils');
 const validate = utils.common.validate;
 const Request = utils.common.core.Request;
+const convertErrors = utils.common.convertErrors;
 
-function submitAsync(txBlob: string, callback: (err: any, data: any) => void):
-    void {
+function submitAsync(txBlob: string, callback: (err: any, data: any) => void
+): void {
   validate.blob(txBlob);
   const request = new Request(this.remote, 'submit');
   request.message.tx_blob = txBlob;
   request.request(null,
     utils.common.composeAsync(
       data => utils.common.convertKeysFromSnakeCaseToCamelCase(data),
-      callback));
+      convertErrors(callback)));
 }
 
 function submit(txBlob: string) {

src/core/remote.js

@@ -1816,10 +1816,7 @@ Remote.prototype.createPathFind = function(options, callback) {
         callback(null, data);
       }
     });
-    pathFind.on('error', (error) => {
-      pathFind.close();
-      callback(error);
-    });
+    pathFind.on('error', callback);
   }
 
   this._cur_path_find = pathFind;

src/core/request.js

@@ -63,6 +63,10 @@ Request.prototype.request = function(servers, callback) {
   return this;
 };
 
+function isResponseNotError(res) {
+  return typeof res === 'object' && !res.hasOwnProperty('error');
+}
+
 /**
  * Broadcast request to all servers, filter responses if a function is
  * provided. Return first response that satisfies the filter. Pre-filter
@@ -74,16 +78,17 @@ Request.prototype.request = function(servers, callback) {
  * @param [Function] fn
  */
 
+
 Request.prototype.filter =
 Request.prototype.addFilter =
-Request.prototype.broadcast = function(filterFn = Boolean) {
+Request.prototype.broadcast = function(isResponseSuccess = isResponseNotError) {
   const self = this;
 
   if (!this.requested) {
     // Defer until requested, and prevent the normal request() from executing
     this.once('before', function() {
       self.requested = true;
-      self.broadcast(filterFn);
+      self.broadcast(isResponseSuccess);
     });
     return this;
   }
@@ -111,7 +116,7 @@ Request.prototype.broadcast = function(filterFn = Boolean) {
       // Listen for proxied success/error event and apply filter
       self.once('proposed', function(res) {
         lastResponse = res;
-        callback(filterFn(res));
+        callback(isResponseSuccess(res));
       });
 
       return server._request(self);

src/core/server.js

@@ -1,6 +1,5 @@
 'use strict';
 
-
 const _ = require('lodash');
 const assert = require('assert');
 const util = require('util');
@@ -130,9 +129,8 @@ function Server(remote, opts_) {
     self._updateScore('ledgerclose', ledger);
   });
 
-/* eslint-disable no-unused-vars */
   this.on('response_ping', function onPingResponse(message, request) {
-/* eslint-enable no-unused-vars */
+    _.noop(message);
     self._updateScore('response', request);
   });
 

test/api-test.js

@@ -391,7 +391,7 @@ describe('RippleAPI', function() {
     return this.api.getTransactions(address, options).then(() => {
       assert(false, 'Should throw RippleError');
     }).catch(error => {
-      assert(error instanceof common.core.RippleError);
+      assert(error instanceof this.api.errors.RippleError);
     });
   });
 
@@ -552,6 +552,14 @@ describe('RippleAPI', function() {
     });
   });
 
+  it('getPaths - error: srcActNotFound', function() {
+    const pathfind = _.assign({}, requests.getPaths.normal,
+      {source: {address: addresses.NOTFOUND}});
+    return this.api.getPaths(pathfind).catch(error => {
+      assert(error instanceof this.api.errors.RippleError);
+    });
+  });
+
   it('getLedgerVersion', function() {
     assert.strictEqual(this.api.getLedgerVersion(), 8819951);
   });

test/fixtures/api/rippled/index.js

@@ -19,7 +19,8 @@ module.exports = {
   path_find: {
     generate: require('./path-find'),
     sendUSD: require('./path-find-send-usd'),
-    XrpToXrp: require('./path-find-xrp-to-xrp')
+    XrpToXrp: require('./path-find-xrp-to-xrp'),
+    srcActNotFound: require('./path-find-srcActNotFound')
   },
   tx: {
     Payment: require('./tx/payment.json'),

test/fixtures/api/rippled/path-find-srcActNotFound.json

@@ -0,0 +1,20 @@
+{
+  "error": "srcActNotFound",
+  "error_code": 58,
+  "error_message": "Source account not found.",
+  "id": 0,
+  "request": {
+    "command": "path_find",
+    "destination_account": "r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59",
+    "destination_amount": {
+      "currency": "USD",
+      "issuer": "r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59",
+      "value": "5"
+    },
+    "id": 1,
+    "source_account": "rajTAg3hon5Lcu1RxQQPxTgHvqfhc1EaUS",
+    "subcommand": "create"
+  },
+  "status": "error",
+  "type": "response"
+}

test/mock-rippled.js

@@ -253,7 +253,9 @@ module.exports = function(port) {
     if (request.subcommand === 'close') {
       return;
     }
-    if (request.source_account === addresses.OTHER_ACCOUNT) {
+    if (request.source_account === addresses.NOTFOUND) {
+      response = createResponse(request, fixtures.path_find.srcActNotFound);
+    } else if (request.source_account === addresses.OTHER_ACCOUNT) {
       response = createResponse(request, fixtures.path_find.sendUSD);
     } else if (request.source_account === addresses.THIRD_ACCOUNT) {
       response = createResponse(request, fixtures.path_find.XrpToXrp, {