diff --git a/src/notifications.js b/src/notifications.js index 5760dbe2a..f479e2ba9 100644 --- a/src/notifications.js +++ b/src/notifications.js @@ -1,8 +1,6 @@ 'use strict'; exports = module.exports = { - NotificationsError: NotificationsError, - get: get, ack: ack, getAllPaged: getAllPaged, @@ -25,6 +23,7 @@ exports = module.exports = { let assert = require('assert'), async = require('async'), auditSource = require('./auditsource.js'), + BoxError = require('./boxerror.js'), changelog = require('./changelog.js'), custom = require('./custom.js'), DatabaseError = require('./databaseerror.js'), @@ -33,30 +32,7 @@ let assert = require('assert'), mailer = require('./mailer.js'), notificationdb = require('./notificationdb.js'), settings = require('./settings.js'), - users = require('./users.js'), - util = require('util'); - -function NotificationsError(reason, errorOrMessage) { - assert.strictEqual(typeof reason, 'string'); - assert(errorOrMessage instanceof Error || typeof errorOrMessage === 'string' || typeof errorOrMessage === 'undefined'); - - Error.call(this); - Error.captureStackTrace(this, this.constructor); - - this.name = this.constructor.name; - this.reason = reason; - if (typeof errorOrMessage === 'undefined') { - this.message = reason; - } else if (typeof errorOrMessage === 'string') { - this.message = errorOrMessage; - } else { - this.message = 'Internal error'; - this.nestedError = errorOrMessage; - } -} -util.inherits(NotificationsError, Error); -NotificationsError.INTERNAL_ERROR = 'Internal Error'; -NotificationsError.NOT_FOUND = 'Not Found'; + users = require('./users.js'); function add(userId, eventId, title, message, callback) { assert.strictEqual(typeof userId, 'string'); @@ -74,8 +50,8 @@ function add(userId, eventId, title, message, callback) { message: message, acknowledged: false }, function (error, result) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new NotificationsError(NotificationsError.NOT_FOUND, error.message)); - if (error) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new BoxError(BoxError.NOT_FOUND, error.message)); + if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); callback(null, { id: result }); }); @@ -86,8 +62,8 @@ function get(id, callback) { assert.strictEqual(typeof callback, 'function'); notificationdb.get(id, function (error, result) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new NotificationsError(NotificationsError.NOT_FOUND)); - if (error) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new BoxError(BoxError.NOT_FOUND)); + if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); callback(null, result); }); @@ -98,8 +74,8 @@ function ack(id, callback) { assert.strictEqual(typeof callback, 'function'); notificationdb.update(id, { acknowledged: true }, function (error) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new NotificationsError(NotificationsError.NOT_FOUND)); - if (error) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new BoxError(BoxError.NOT_FOUND)); + if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); callback(null); }); @@ -114,7 +90,7 @@ function getAllPaged(userId, acknowledged, page, perPage, callback) { assert.strictEqual(typeof callback, 'function'); notificationdb.listByUserIdPaged(userId, page, perPage, function (error, result) { - if (error) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); if (acknowledged === null) return callback(null, result); @@ -129,7 +105,7 @@ function actionForAllAdmins(skippingUserIds, iterator, callback) { assert.strictEqual(typeof callback, 'function'); users.getAllAdmins(function (error, result) { - if (error) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); // filter out users we want to skip (like the user who did the action or the user the action was performed on) result = result.filter(function (r) { return skippingUserIds.indexOf(r.id) === -1; }); @@ -334,15 +310,15 @@ function alert(id, title, message, callback) { }; notificationdb.getByUserIdAndTitle(admin.id, title, function (error, result) { - if (error && error.reason !== DatabaseError.NOT_FOUND) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error && error.reason !== DatabaseError.NOT_FOUND) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); if (!result && acknowledged) return callback(); // do not add acked alerts let updateFunc = !result ? notificationdb.add.bind(null, data) : notificationdb.update.bind(null, result.id, data); updateFunc(function (error) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new NotificationsError(NotificationsError.NOT_FOUND, error.message)); - if (error) return callback(new NotificationsError(NotificationsError.INTERNAL_ERROR, error)); + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new BoxError(BoxError.NOT_FOUND, error.message)); + if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); callback(null); }); diff --git a/src/routes/notifications.js b/src/routes/notifications.js index 4a5fc57ba..a4a1ed8ab 100644 --- a/src/routes/notifications.js +++ b/src/routes/notifications.js @@ -8,17 +8,26 @@ exports = module.exports = { }; let assert = require('assert'), + BoxError = require('../boxerror.js'), HttpError = require('connect-lastmile').HttpError, HttpSuccess = require('connect-lastmile').HttpSuccess, - notifications = require('../notifications.js'), - NotificationsError = notifications.NotificationsError; + notifications = require('../notifications.js'); + +function toHttpError(error) { + switch (error.reason) { + case BoxError.NOT_FOUND: + return new HttpError(404, error); + case BoxError.DATABASE_ERROR: + default: + return new HttpError(500, error); + } +} function verifyOwnership(req, res, next) { if (!req.params.notificationId) return next(); // skip for listing notifications.get(req.params.notificationId, function (error, result) { - if (error && error.reason === NotificationsError.NOT_FOUND) return next(new HttpError(404, 'No such notification')); - if (error) return next(new HttpError(500, error)); + if (error) return next(toHttpError(error)); if (result.userId !== req.user.id) return next(new HttpError(403, 'User is not owner')); @@ -46,7 +55,7 @@ function list(req, res, next) { else if (req.query.acknowledged) acknowledged = req.query.acknowledged === 'true' ? true : false; notifications.getAllPaged(req.user.id, acknowledged, page, perPage, function (error, result) { - if (error) return next(new HttpError(500, error)); + if (error) return next(toHttpError(error)); next(new HttpSuccess(200, { notifications: result })); }); @@ -56,8 +65,7 @@ function ack(req, res, next) { assert.strictEqual(typeof req.params.notificationId, 'string'); notifications.ack(req.params.notificationId, function (error) { - if (error && error.reason === NotificationsError.NOT_FOUND) return next(new HttpError(404, 'No such notification')); - if (error) return next(new HttpError(500, error)); + if (error) return next(toHttpError(error)); next(new HttpSuccess(204, {})); }); diff --git a/src/test/notifications-test.js b/src/test/notifications-test.js index 8f478413c..1db796255 100644 --- a/src/test/notifications-test.js +++ b/src/test/notifications-test.js @@ -7,12 +7,12 @@ 'use strict'; var async = require('async'), + BoxError = require('../boxerror.js'), database = require('../database.js'), users = require('../users.js'), userdb = require('../userdb.js'), eventlogdb = require('../eventlogdb.js'), notifications = require('../notifications.js'), - NotificationsError = notifications.NotificationsError, expect = require('expect.js'); // owner @@ -92,8 +92,8 @@ describe('Notifications', function () { it('get of unknown id fails', function (done) { notifications.get('notfoundid', function (error, result) { - expect(error).to.be.a(NotificationsError); - expect(error.reason).to.be(NotificationsError.NOT_FOUND); + expect(error).to.be.a(BoxError); + expect(error.reason).to.be(BoxError.NOT_FOUND); expect(result).to.not.be.ok(); done(); @@ -128,8 +128,8 @@ describe('Notifications', function () { it('ack fails for nonexisting id', function (done) { notifications.ack('id does not exist', function (error) { - expect(error).to.be.a(NotificationsError); - expect(error.reason).to.be(NotificationsError.NOT_FOUND); + expect(error).to.be.a(BoxError); + expect(error.reason).to.be(BoxError.NOT_FOUND); done(); });