Skip to content

Commit

Permalink
feat: create API error functions
Browse files Browse the repository at this point in the history
This commit creates error functions for common HTTP error status codes.
The following have been implemented:
 1. BadRequest [400]
 2. Unauthorized [401]
 3. Forbidden [403]
 4. NotFound [404]
 5. InternalServerError [500]

Error functions are really easy to import and use in controllers.  See
the following example:
```js

const NotFound = require('lib/errors/NotFound');

// throw the error to demonstrate that it is an error
try {
  throw new NotFound('An example description.');

  // NOTE - this used to be done like this:
  // throw new req.codes.ERR_NOT_FOUND();

// pass the error to the first error handling middleware
} catch (e) {
  next(e);
}
```

**NOTE** - this commit __deprecates__ the use of `req.codes`.  All
controller code should now import the codes they need and no longer
depend the on `req.codes` property, as it will be removed in the future.

Closes #182.
  • Loading branch information
jniles committed Mar 23, 2016
1 parent 593270e commit dd930a5
Show file tree
Hide file tree
Showing 14 changed files with 328 additions and 29 deletions.
5 changes: 4 additions & 1 deletion server/config/codes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
* @example
* next(new res.codes.ERR_NOT_FOUND());
* Promise.then(function () { throw new res.codes.ERR_NOT_FOUND(); }).catch(next);
*
* @deprecated This module should not longer be used. See issue #182.
*/

/** custom errorFactor function to throw in API endpoints */
/** custom errorFactory function to throw in API endpoints */
function makeError(name, defn) {

// must call 'new' on this function
Expand All @@ -34,6 +36,7 @@ function makeError(name, defn) {
return AppError;
}

/** @deprecated See issue #182 */
module.exports = {

/** application error codes */
Expand Down
1 change: 1 addition & 0 deletions server/config/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ exports.configure = function configure(app) {
exports.errorHandling = function errorHandling(app) {
'use strict';

app.use(interceptors.newErrorHandler);
app.use(interceptors.apiErrorHandler);
app.use(interceptors.databaseErrorHandler);
app.use(interceptors.catchAllErrorHandler);
Expand Down
46 changes: 44 additions & 2 deletions server/config/interceptors.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,27 @@
* @todo Import logging functionality into this module.
*
* @requires config/codes
* @requires lib/errors/Forbidden
* @requires lib/errors/Unauthorized
* @requires lib/errors/BadRequest
* @requires lib/errors/NotFound
* @requires lib/errors/InternalServerError
*/

var errors = require('./codes');
var winston = require('winston');

var BadRequest = require('../lib/errors/BadRequest');
var Unauthorized = require('../lib/errors/Unauthorized');
var Forbidden = require('../lib/errors/Forbidden');
var NotFound = require('../lib/errors/NotFound');
var InternalServerError = require('../lib/errors/InternalServerError');

/**
* This function identifies whether a parameter is a native error of JS or not.
*
* @param error
* @returns {boolean} bool True if the error is a native error, false otherwise
* @param {Error} error
* @returns {boolean} bool - true if the error is a native error, false otherwise
*/
function isNativeError(error) {
return (
Expand All @@ -34,6 +45,37 @@ function isNativeError(error) {
);
}

/**
* This function identifies whether an error is an error constructed by the API
* as defined in lib/errors/*.js.
*
* @param {Error} error
* @returns {boolean} bool - true if the error is an API error, false otherwise
*/
function isApiError(error) {
return (
error instanceof NotFound ||
error instanceof Unauthorized ||
error instanceof Forbidden ||
error instanceof BadRequest ||
error instanceof InternalServerError
);
}

/**
* This error handler exists while we are migrating to the new error imports and
* deprecating req.codes. It should eventually replace all API error handling.
* A catchall error handler will still be necessary.
*/
exports.newErrorHandler = function newErrorHandler(error, req, res, next) {

// skip this middleware if not an API error
if (!isApiError(error)) { return next(error); }

// send to the client with the appropriate status code
res.status(error.status).json(error);
};


/**
* Handles errors specifically generated by the application API.
Expand Down
36 changes: 27 additions & 9 deletions server/controllers/auth.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
// responsible for authentication routes
/**
* Authentication Controller
*
* This controller is responsible for managing user authentication and
* authorization to the entire application stack.
*
* @todo - user roles should be designed and implemented to restrict a
* user's ability to selected routes.
*
* @requires lib/db
* @requires lib/errors/Unauthorized
* @requires lib/errors/Forbidden
* @requires lib/errors/InternalServerError
*/

var db = require('../lib/db');
var Unauthorized = require('../lib/errors/Unauthorized');
var Forbidden = require('../lib/errors/Forbidden');
var InternalServerError = require('../lib/errors/InternalServerError');

// POST /login
// This route will accept a login request with the
Expand All @@ -26,8 +42,8 @@ exports.login = function login(req, res, next) {
.then(function (rows) {

// if no data found, we return a login error
if (rows.length < 1) {
throw new req.codes.ERR_BAD_CREDENTIALS();
if (rows.length === 0) {
throw new Unauthorized('Bad username and password combination.');
}

// we assume only one match for the user
Expand All @@ -43,7 +59,7 @@ exports.login = function login(req, res, next) {

// if no permissions, notify the user that way
if (rows.length === 0) {
throw new req.codes.ERR_NO_PERMISSIONS();
throw new Unauthorized('This user does not have any permissions.');
}

// update the database for when the user logged in
Expand All @@ -65,7 +81,7 @@ exports.login = function login(req, res, next) {
})
.then(function (rows) {
if (rows.length === 0) {
throw new req.codes.ERR_NO_ENTERPRISE();
throw new InternalServerError('There are no enterprises registered in the database!');
}

enterprise = rows[0];
Expand All @@ -77,7 +93,7 @@ exports.login = function login(req, res, next) {
})
.then(function (rows) {
if (rows.length === 0) {
throw new req.codes.ERR_NO_PROJECT();
throw new Unauthorized('No project matching the provided id.');
}

project = rows[0];
Expand All @@ -87,12 +103,14 @@ exports.login = function login(req, res, next) {
req.session.enterprise = enterprise;
req.session.project = project;

// send the data back to the client
res.status(200).json({
var data = {
user : user,
enterprise : enterprise,
project : project
});
};

// send the data back to the client
res.status(200).json(data);
})
.catch(next)
.done();
Expand Down
25 changes: 17 additions & 8 deletions server/controllers/finance/cash.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,16 @@
* select a cashbox which implicitly bundles in cash accounts for all supported
* currencies. The API accepts a cashbox ID during cash payment creation and
* looks up the correct account based on the cashbox_id + currency.
*
* @requires lib/db
* @requires node-uuid
* @requires lib/errors/NotFound
* @requires lib/errors/BadRequest
*/
const db = require('../../lib/db');
const uuid = require('node-uuid');
var NotFound = require('../../lib/errors/NotFound');
var BadRequest = require('../../lib/errors/BadRequest');

/** retrieves the details of a cash payment */
exports.detail = detail;
Expand Down Expand Up @@ -61,7 +68,7 @@ function lookupCashRecord(id, codes) {
.then(function (rows) {

if (rows.length === 0) {
throw new codes.ERR_NOT_FOUND();
throw new NotFound('No cash record by uuid: ?'.replace('?', uuid));
}

// store the record for return
Expand Down Expand Up @@ -179,12 +186,11 @@ function create(req, res, next) {
});
}

// disallow invoice payments with empty items.
// disallow invoice payments with empty items by returning a 400 to the client
if (!data.is_caution && (!items || !items.length)) {
return res.status(400).json({
code : 'CASH.VOUCHER.ERRORS.NO_CASH_ITEMS',
reason : 'You must submit cash items with the cash items payment.'
});
return next(
new BadRequest('You must submit cash items with the cash items payment.')
);
}

const writeCashSql =
Expand Down Expand Up @@ -291,16 +297,19 @@ function debitNote(req, res, next) {
*/
function reference(req, res, next) {

// alias the reference
var ref = req.params.reference;

const sql =
'SELECT BUID(c.uuid) AS uuid FROM (' +
'SELECT cash.uuid, CONCAT(project.abbr, cash.reference) AS reference ' +
'FROM cash JOIN project ON cash.project_id = project.id' +
')c WHERE c.reference = ?;';

db.exec(sql, [ req.params.reference ])
db.exec(sql, [ ref ])
.then(function (rows) {
if (rows.length === 0) {
throw new req.codes.ERR_NOT_FOUND();
throw new NotFound('No cash record with reference: ?'.replace('?', ref));
}

// references should be unique - return the first one
Expand Down
38 changes: 38 additions & 0 deletions server/lib/errors/BadRequest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
var util = require('util');

/**
* This implements an HTTP error code that should eventually be passed through
* next() to an error handling middleware.
*
* @param {String} description - a custom description to be sent to the client
*
* @example
* // import the error into a controller
* const BadRequest = require('lib/errors/BadRequest');
*
* // use the error in either a promise chain or directly via next()
* return next(new BadRequest('Some description...'));
*
* @constructor
*/
function BadRequest(description) {
'use strict';

// make sure we have a working stack trace
Error.captureStackTrace(this, this.constructor);

// HTTP status code
this.status = 400;

// bhima status code (for $translation)
this.code = 'ERRORS.BAD_REQUEST';

// default to an empty string if no description passed in
this.description = description || '';
}

// prototypically inherit from the global error object
util.inherits(BadRequest, Error);

// expose the function to an external controller
module.exports = BadRequest;
38 changes: 38 additions & 0 deletions server/lib/errors/Forbidden.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
var util = require('util');

/**
* This implements an HTTP error code that should eventually be passed through
* next() to an error handling middleware.
*
* @param {String} description - a custom description to be sent to the client
*
* @example
* // import the error into a controller
* const Forbidden = require('lib/errors/Forbidden');
*
* // use the error in either a promise chain or directly via next()
* return next(new Forbidden('Some description...'));
*
* @constructor
*/
function Forbidden(description) {
'use strict';

// make sure we have a working stack trace
Error.captureStackTrace(this, this.constructor);

// HTTP status code
this.status = 403;

// bhima status code (for $translation)
this.code = 'ERRORS.FORBIDDEN';

// default to an empty string if no description passed in
this.description = description || '';
}

// prototypically inherit from the global error object
util.inherits(Forbidden, Error);

// expose the function to an external controller
module.exports = Forbidden;
38 changes: 38 additions & 0 deletions server/lib/errors/InternalServerError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
var util = require('util');

/**
* This implements an HTTP error code that should eventually be passed through
* next() to an error handling middleware.
*
* @param {String} description - a custom description to be sent to the client
*
* @example
* // import the error into a controller
* const InternalServerError = require('lib/errors/InternalServerError');
*
* // use the error in either a promise chain or directly via next()
* return next(new InternalServerError('Some description...'));
*
* @constructor
*/
function InternalServerError(description) {
'use strict';

// make sure we have a working stack trace
Error.captureStackTrace(this, this.constructor);

// HTTP status code
this.status = 500;

// bhima status code (for $translation)
this.code = 'ERRORS.INTERNAL_SERVER_ERROR';

// default to an empty string if no description passed in
this.description = description || '';
}

// prototypically inherit from the global error object
util.inherits(InternalServerError, Error);

// expose the function to an external controller
module.exports = InternalServerError;
38 changes: 38 additions & 0 deletions server/lib/errors/NotFound.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
var util = require('util');

/**
* This implements an HTTP error code that should eventually be passed through
* next() to an error handling middleware.
*
* @param {String} description - a custom description to be sent to the client
*
* @example
* // import the error into a controller
* const NotFound = require('lib/errors/NotFound');
*
* // use the error in either a promise chain or directly via next()
* return next(new NotFound('Some description...'));
*
* @constructor
*/
function NotFound(description) {
'use strict';

// make sure we have a working stack trace
Error.captureStackTrace(this, this.constructor);

// HTTP status code
this.status = 404;

// bhima status code (for $translation)
this.code = 'ERRORS.NOT_FOUND';

// default to an empty string if no description passed in
this.description = description || '';
}

// prototypically inherit from the global error object
util.inherits(NotFound, Error);

// expose the function to an external controller
module.exports = NotFound;
Loading

0 comments on commit dd930a5

Please sign in to comment.