Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Commit

Permalink
Show stack traces for uncaught exceptions in middlewares (RLJS-277)
Browse files Browse the repository at this point in the history
- Return 400 error for client errors in Balances, Trustlines, Payments,
  and Orders.
- Silence api/logger.js during tests. The server/logger.js still
  logs unhandled exceptions during tests.
- Includes some lint error fixes
  • Loading branch information
Chris Clark authored and Alan Cohen committed Mar 17, 2015
1 parent eca2fe9 commit 5501cae
Show file tree
Hide file tree
Showing 13 changed files with 270 additions and 216 deletions.
5 changes: 4 additions & 1 deletion api/balances.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ function getBalances(account, options, callback) {
if (validate.fail([
validate.account(account),
validate.currency(options.currency, true),
validate.counterparty(options.counterparty, true)
validate.counterparty(options.counterparty, true),
validate.ledger(options.ledger, true),
validate.limit(options.limit, true),
validate.paging(options,true)
], callback)) {
return;
}
Expand Down
28 changes: 20 additions & 8 deletions api/lib/errors.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
/* eslint-disable valid-jsdoc */
'use strict';

/**
* Base class for all errors
*/
function RippleError(message) {
this.message = message;
}
RippleError.prototype = new Error;
RippleError.prototype.name = 'RippleError';


/**
* Invalid Request Error
* Missing parameters or invalid parameters
*/
function InvalidRequestError(message) {
this.message = message;
}
InvalidRequestError.prototype = new Error();
InvalidRequestError.prototype = new RippleError;
InvalidRequestError.prototype.name = 'InvalidRequestError';
InvalidRequestError.prototype.error = 'restINVALID_PARAMETER';

Expand All @@ -18,7 +29,7 @@ InvalidRequestError.prototype.error = 'restINVALID_PARAMETER';
function NetworkError(message) {
this.message = message;
}
NetworkError.prototype = new Error();
NetworkError.prototype = new RippleError;
NetworkError.prototype.name = 'NetworkError';

/**
Expand All @@ -38,7 +49,7 @@ RippledNetworkError.prototype.error = 'restRIPPLED_NETWORK_ERR';
function TransactionError(message) {
this.message = message;
}
TransactionError.prototype = new Error();
TransactionError.prototype = new RippleError;
TransactionError.prototype.name = 'TransactionError';

/**
Expand All @@ -48,7 +59,7 @@ TransactionError.prototype.name = 'TransactionError';
function NotFoundError(message) {
this.message = message;
}
NotFoundError.prototype = new Error();
NotFoundError.prototype = new RippleError;
NotFoundError.prototype.name = 'NotFoundError';
NotFoundError.prototype.error = 'restNOT_FOUND';

Expand All @@ -59,7 +70,7 @@ NotFoundError.prototype.error = 'restNOT_FOUND';
function TimeOutError(message) {
this.message = message;
}
TimeOutError.prototype = new Error();
TimeOutError.prototype = new RippleError;
TimeOutError.prototype.name = 'TimeOutError';

/**
Expand All @@ -69,7 +80,7 @@ TimeOutError.prototype.name = 'TimeOutError';
function ApiError(message) {
this.message = message;
}
ApiError.prototype = new Error();
ApiError.prototype = new RippleError;
ApiError.prototype.name = 'ApiError';

/**
Expand All @@ -89,7 +100,7 @@ DuplicateTransactionError.prototype.error = 'restDUPLICATE_TRANSACTION';
function DatabaseError(message) {
this.message = message;
}
DatabaseError.prototype = new Error();
DatabaseError.prototype = new RippleError;
DatabaseError.prototype.name = 'DatabaseError';

module.exports = {
Expand All @@ -101,5 +112,6 @@ module.exports = {
TimeOutError: TimeOutError,
ApiError: ApiError,
DuplicateTransactionError: DuplicateTransactionError,
DatabaseError: DatabaseError
DatabaseError: DatabaseError,
RippleError: RippleError
};
17 changes: 17 additions & 0 deletions api/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
'use strict';
var bignum = require('bignumber.js');
var validator = require('./schema-validator.js');
var ripple = require('ripple-lib');

function dropsToXrp(drops) {
return bignum(drops).dividedBy(1000000.0).toString();
Expand Down Expand Up @@ -94,7 +95,22 @@ function compareTransactions(first, second) {
return first.ledger_index < second.ledger_index ? -1 : 1;
}

function isValidLedgerSequence(ledger) {
return (Number(ledger) >= 0) && isFinite(Number(ledger));
}

function isValidLedgerHash(ledger) {
return ripple.UInt256.is_valid(ledger);
}

function isValidLedgerWord(ledger) {
return (/^current$|^closed$|^validated$/.test(ledger));
}

module.exports = {
isValidLedgerSequence: isValidLedgerSequence,
isValidLedgerWord: isValidLedgerWord,
isValidLedgerHash: isValidLedgerHash,
dropsToXrp: dropsToXrp,
xrpToDrops: xrpToDrops,
parseLedger: parseLedger,
Expand All @@ -103,3 +119,4 @@ module.exports = {
txFromRestAmount: txFromRestAmount,
compareTransactions: compareTransactions
};

51 changes: 46 additions & 5 deletions api/lib/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var _ = require('lodash');
var errors = require('./errors.js');
var validator = require('./schema-validator');
var ripple = require('ripple-lib');
var utils = require('./utils');

function addOptionalOption(validatorFunction) {
return function(parameter, optional) {
Expand Down Expand Up @@ -44,6 +45,41 @@ function validateIssue(issue) {
]) || null;
}


function validateLedger(ledger) {
if (!(utils.isValidLedgerSequence(ledger)
|| utils.isValidLedgerHash(ledger)
|| utils.isValidLedgerWord(ledger)))
{
return new errors.InvalidRequestError(
'Invalid or Missing Parameter: ledger');
}

return null;
}

function validatePaging(options) {
if (options.marker) {
if (!options.ledger ||
!(utils.isValidLedgerSequence(options.ledger
|| utils.isValidLedgerHash(options.ledger))))
{
return new errors.InvalidRequestError(
'Invalid or Missing Parameter: ledger');
}
}
return null;
}

function validateLimit(limit) {
if (!(limit === 'all' || !_.isNaN(Number(limit)))) {
return new errors.InvalidRequestError(
'Invalid or Missing Parameter: limit');
}
return null;
}


function fail(results, callback) {
var error = _.find(results);
if (error) {
Expand All @@ -53,8 +89,13 @@ function fail(results, callback) {
return false;
}

module.exports.account = addOptionalOption(validateAccount);
module.exports.currency = addOptionalOption(validateCurrency);
module.exports.counterparty = addOptionalOption(validateCounterparty);
module.exports.issue = addOptionalOption(validateIssue);
module.exports.fail = fail;
module.exports = {
account: addOptionalOption(validateAccount),
currency: addOptionalOption(validateCurrency),
counterparty: addOptionalOption(validateCounterparty),
issue: addOptionalOption(validateIssue),
ledger: addOptionalOption(validateLedger),
limit: addOptionalOption(validateLimit),
paging: addOptionalOption(validatePaging),
fail: fail
};
23 changes: 12 additions & 11 deletions api/orders.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var errors = require('./lib/errors.js');
var TxToRestConverter = require('./lib/tx-to-rest-converter.js');
var validator = require('./lib/schema-validator.js');
var bignum = require('bignumber.js');
var validate = require('./lib/validate');

var InvalidRequestError = errors.InvalidRequestError;

Expand Down Expand Up @@ -42,13 +43,13 @@ var DefaultPageLimit = 200;
function getOrders(account, options, callback) {
var self = this;

function validateOptions() {
if (!ripple.UInt160.is_valid(account)) {
return Promise.reject(new InvalidRequestError(
'Parameter is not a valid Ripple address: account'));
}

return Promise.resolve();
if (validate.fail([
validate.account(account),
validate.ledger(options.ledger, true),
validate.limit(options.limit, true),
validate.paging(options,true)
], callback)) {
return;
}

function getAccountOrders(prevResult) {
Expand Down Expand Up @@ -132,10 +133,10 @@ function getOrders(account, options, callback) {

return promise;
}
validateOptions(options)
.then(getAccountOrders)
.then(respondWithOrders)
.catch(callback);

getAccountOrders()
.then(respondWithOrders)
.catch(callback);
}

/**
Expand Down
10 changes: 6 additions & 4 deletions api/payments.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ var utils = require('./lib/utils');
var RestToTxConverter = require('./lib/rest-to-tx-converter.js');
var TxToRestConverter = require('./lib/tx-to-rest-converter.js');
var SubmitTransactionHooks = require('./lib/submit_transaction_hooks.js');
var errors = require('./lib/errors.js');

var InvalidRequestError = errors.InvalidRequestError;
var NotFoundError = errors.NotFoundError;
var TimeOutError = errors.TimeOutError;
var errors = require('./lib/errors');
var InvalidRequestError = errors.InvalidRequestError;
var NetworkError = errors.NetworkError;
var NotFoundError = errors.NotFoundError;
var TimeOutError = errors.TimeOutError;
var ApiError = errors.ApiError;

var DEFAULT_RESULTS_PER_PAGE = 10;

Expand Down
5 changes: 4 additions & 1 deletion api/trustlines.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ function getTrustLines(account, options, callback) {
if (validate.fail([
validate.account(account),
validate.currency(options.currency, true),
validate.counterparty(options.counterparty, true)
validate.counterparty(options.counterparty, true),
validate.ledger(options.ledger, true),
validate.limit(options.limit, true),
validate.paging(options)
], callback)) {
return;
}
Expand Down
9 changes: 9 additions & 0 deletions server/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,14 @@ var NotFoundError = errors.NotFoundError;
var TimeOutError = errors.TimeOutError;
var ApiError = errors.ApiError;
var DatabaseError = errors.DatabaseError;
var RippleError = errors.RippleError;
var RippleLibError = require('ripple-lib').RippleError;


function isRippleError(error) {
return error instanceof RippleError || error instanceof RippleLibError;
}

function handleError(error, req, res, next) {
// If in debug mode, print errors
if (config.get('debug')) {
Expand All @@ -23,6 +29,9 @@ function handleError(error, req, res, next) {
} else {
logger.error(error);
}
} else if (!isRippleError(error)) {
// always log stack traces for uncaught exceptions
error.stack && logger.error(error.stack);
}

var err_obj = {
Expand Down
13 changes: 2 additions & 11 deletions server/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,15 @@ var config = require('./config');
var morgan = require('morgan');
var version = require('./version');


var logger = exports.logger = new winston.Logger({
transports: [
new winston.transports.Console({
prettyPrint: true,
colorize: true,
handleExceptions: false,
level: 'info',
handleExceptions: true,
level: config.get('NODE_ENV') === 'test' ? 'error' : 'info',
showLevel: true,
timestamp: true,
silent: config.get('NODE_ENV') === 'test'
})
],
exceptionHandlers: [
new winston.transports.Console({
prettyPrint: true,
colorize: true,
timestamp: true
})
],
exitOnError: true
Expand Down
Loading

0 comments on commit 5501cae

Please sign in to comment.