Skip to content

Commit

Permalink
Normalized error messages: use of HttpErrors on platforms.
Browse files Browse the repository at this point in the history
  • Loading branch information
nick13jaremek authored and igznicolasjaremek committed Aug 27, 2015
1 parent 7dfee10 commit d27099e
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 47 deletions.
3 changes: 2 additions & 1 deletion lib/platforms/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ function createChannel(channelData, callback) {
if (err) {
return callback(new errors.InternalError('Channel creation process failed'));
}

if (results && !_.isEmpty(results)) {
return callback(new errors.ConflictError('There already exists a channel with the provided name'));
}
Expand Down Expand Up @@ -229,7 +230,7 @@ function deleteIdentityFromChannel(channelId, identityId, callback) {
identities.get(identityId, function(err, foundIdentity) {

if (err) {
return done(new errors.BadRequestError('Could not fetch requested Identity object'));
return done(new errors.InternalError('Could not fetch requested Identity object'));
}

if (_.isEmpty(foundIdentity)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/platforms/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var config = require('config');
var _ = require('lodash');
var log = require('../util/logger');
var transport = require('../transport/mailgun');
var errors = require('../util/errors');

var BATCH_LIMIT = config.get('platform_batch_limits.mailgun');
var CUSTOM_BATCH_LIMIT = config.get('transport.mailgun.batch_limit') || BATCH_LIMIT;
Expand All @@ -17,8 +18,7 @@ function sendEmailNotification(identities, body, callback) {
transport.send(emails, body.content, function (err, response, body) {

if (err) {
log.error('Error sending request to send emails', err);
return callback(err);
return callback(new errors.BadRequestError('Could not send email notification to destinations'));
}

var output = {
Expand Down
48 changes: 24 additions & 24 deletions lib/platforms/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var _ = require('lodash');
var config = require('config');
var async = require('async');
var log = require('../util/logger');
var errors = require('../util/errors');

module.exports = {
get: getIdentity,
Expand Down Expand Up @@ -55,9 +56,7 @@ function updateIdentityData(identity, changes, callback) {
checkForIdentityExistence(identity, function(err, result) {

if (err) {

log.error('Error checking the id existence', err);
return callback(err);
return callback(err); // Controlled error
}

async.waterfall([
Expand All @@ -68,9 +67,7 @@ function updateIdentityData(identity, changes, callback) {
identity.save(function(err){

if (err) {

log.error('Error saving identity', err);
return done(err);
return done(new errors.InternalError('Could not update identity object data'));
}

return done();
Expand All @@ -80,16 +77,17 @@ function updateIdentityData(identity, changes, callback) {
subscribeIdentityToChannels(identity.id, identity.channels, function(err, output) {

if (err) {
return done(err);
return done(err); // Controlled error
}

return done(null, output);
});
}
],
function (err, output) {

if (err) {
return callback(err);
return callback(err); // Controlled error
}

return callback(null, output);
Expand Down Expand Up @@ -133,7 +131,7 @@ function createIdentity(identityObj, callback) {
checkForIdentityExistence(element, function(err) {

if (err) {
return done(err);
return done(err); // Controlled error
}

return done();
Expand All @@ -149,9 +147,7 @@ function createIdentity(identityObj, callback) {
identity.save(function(err, savedIdentity) {

if (err) {

log.error('Error saving the identity', err);
return done(err);
return done(new errors.InternalError('Could not save new identity object'));
}

return done(null, savedIdentity);
Expand All @@ -162,7 +158,7 @@ function createIdentity(identityObj, callback) {
subscribeIdentityToChannels(savedIdentity.id, savedIdentity.channels, function(err, subscribedIdentity) {

if (err) {
return done(err);
return done(err); // Controlled error
}

return done(null, subscribedIdentity);
Expand All @@ -171,7 +167,7 @@ function createIdentity(identityObj, callback) {
], function finishSubscription(err, subscribedIdentity) {

if (err) {
return callback(err);
return callback(err); // Controlled error
}

return callback(null, subscribedIdentity);
Expand All @@ -185,7 +181,7 @@ function checkForDuplicates(identity, field, callback) {
Identity.find().where(field).in(fieldValuesToCheck).exec(function(err, result) {

if (err) {
return callback(err);
return callback(new errors.InternalError('Could not check identity uniqueness for field ' + field));
}

var duplicatedIdentities =_.filter(result, function(item) {
Expand All @@ -206,7 +202,7 @@ function checkForDuplicates(identity, field, callback) {
function checkForIdentityExistence(identityObj, callback) {

if (!identityObj) {
return callback(true);
return callback(new errors.BadRequestError('Cannot check a missing identity object'));
}

var mustCheck = {};
Expand Down Expand Up @@ -253,8 +249,12 @@ function checkForIdentityExistence(identityObj, callback) {
return element === false;
});

if (err || !isValid) {
return callback(true);
if (err) {
return callback(err);
}

if (!isValid) {
return callback(new errors.ConflictError('There already exists an identity object with some of the provided field values'));
}

return callback(null, identityObj);
Expand All @@ -276,7 +276,7 @@ function removeValuesFromField(identityList, field, valuesToRemove, callback) {
Identity.update(queryOptions, { $pull: pullOptions}, {multi: true}, function(err, result) {

if (err) {
return callback(true);
return callback(new InternalError('Could not update identities for field ' + field));
}

return callback(null, result);
Expand All @@ -288,7 +288,7 @@ function findIdentitiesByFieldValue(field, values, callback) {
Identity.find().where(field).in(values).exec(function(err, foundIdentities) {

if (err) {
return callback(err);
return callback(new errors.InternalError('Could not find identities for field ' + field + ' and values ' + values));
}

return callback(err, foundIdentities);
Expand All @@ -302,7 +302,7 @@ function subscribeIdentityToChannels(subscribedIdentity, channelNames, callback)
Channel.find({'name': {$in: channelNames}}, function(err, foundChannels) {

if (err) {
return done(err);
return done(new InternalError('Could not find the specified channels for subscription'));
}

var filteredChannelNames = _.flatten(_.map(foundChannels, function(channelItem) {
Expand All @@ -318,7 +318,7 @@ function subscribeIdentityToChannels(subscribedIdentity, channelNames, callback)
Channel.update({'name': {$in: filteredChannelNames }}, {$push: {'identityRef': subscribedIdentity}}, null, function(err) {

if (err) {
return microDone(err);
return microDone(new InternalError('Could not subscribe identity to channels'));
}

return microDone();
Expand All @@ -328,7 +328,7 @@ function subscribeIdentityToChannels(subscribedIdentity, channelNames, callback)
Identity.update({'_id': subscribedIdentity}, {$set: { 'channels': filteredChannelNames }}, null, function(err) {

if (err) {
return microDone(err);
return microDone(new InternalError('Could not create relationship from channels to identity'));
}

return microDone();
Expand All @@ -339,7 +339,7 @@ function subscribeIdentityToChannels(subscribedIdentity, channelNames, callback)
], function(err) {

if (err) {
return callback(err);
return callback(err); // Controlled error
}

return callback(null, subscribedIdentity);
Expand Down
16 changes: 9 additions & 7 deletions lib/platforms/orchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var async = require('async');
var _ = require('lodash');
var log = require('../util/logger');
var identityPlatform = require('./identity');
var errors = require('../util/errors');

module.exports = {
sendNotification: sendNotification,
Expand Down Expand Up @@ -65,7 +66,7 @@ function sendPushNotifications(requestBody, platformModule, callback) {
}, function(err) {

if (err) {
return callback(err);
return callback(err); // Controlled error
}

return callback(null, {response: 'Queued', body: {output: 'Queued notifications' } });
Expand All @@ -84,8 +85,7 @@ function batchNotifications(requestBody, platformModule, queryOptions, callback)
], function(err) {

if (err) {
log.error('Sending notifications failed', err);
return callback(err);
return callback(new errors.InternalError('Batch notifications process failed for request ' + requestBody));
}

return callback(null, {response: 'Queued', body: {output: 'Queued notifications' } });
Expand Down Expand Up @@ -118,6 +118,10 @@ function buildQueryOptions(options) {
function countIdentities(queryOptions, done) {
identityPlatform.count(queryOptions, function(err, numIdentities) {

if (err) {
return done(new errors.InternalError('Could not obtain number of identity objects'));
}

log.info('Counted ' + numIdentities + ' identities');
return done(null, queryOptions, numIdentities);
});
Expand All @@ -142,8 +146,7 @@ function sendBatches(requestBody, queryOptions, totalIdentities, platformModule,
log.info('batch %d found identities %s', page, foundIdentities);

if (err) {
log.error('Fatal error while searching for identities to send notifications');
return nextBatch(err);
return nextBatch(new errors.InternalError('Could not find identity objects for batch %d', page));
}

if (foundIdentities.length < pageSize || totalIdentities === (pageSize * (page + 1))) {
Expand All @@ -157,8 +160,7 @@ function sendBatches(requestBody, queryOptions, totalIdentities, platformModule,
log.info('Sending notifications');

if (err) {
log.error('Fatal error while sending notifications', err);
return nextBatch(err);
return nextBatch(new errors.InternalError('Fatal error while sending notifications batch'));
}

return nextBatch();
Expand Down
13 changes: 8 additions & 5 deletions lib/platforms/push.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var gcm = require('../transport/gcm');
var apn = require('../transport/apn');
var Identity = require('../models/identity');
var log = require('../util/logger');
var errors = require('../util/errors');

var APN_BATCH_LIMIT = config.get('platform_batch_limits.apn');
var GCM_BATCH_LIMIT = config.get('platform_batch_limits.gcm');
Expand Down Expand Up @@ -40,16 +41,18 @@ function deleteApnDevices(devices, callback){

log.info('Deleting Apn devices');
log.debug('Deleting Apn devices', devices);

async.forEachSeries(devices, function(device, done){
Identity.update({'devices.apn':{$in:[device]}}, {$pull:{'devices.apn':device}}, function (err) {

Identity.update({'devices.apn':{$in:[device]}}, {$pull:{'devices.apn':device}}, done);
if (err) {
return done(new errors.InternalError('Could not delete devices from identity'));
}
return done();
});
}, function(err){

if (err) {

log.error('Error deleting apn devices', err);
return callback(err);
return callback(err); // Controlled error
}

callback(null);
Expand Down
6 changes: 3 additions & 3 deletions lib/platforms/sms.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var async = require('async');
var config = require('config');
var _ = require('lodash');
var log = require('../util/logger');
var errors = require('../util/errors');

var transport = require('../transport/twilio');

Expand Down Expand Up @@ -30,7 +31,7 @@ function sendSmsNotification(identities, body, callback) {
transport.sendSms(sms, function (err) {

if (err) {
return done(err);
return done(new errors.InternalError('Could not send SMS notifications to destinations'));
}

messageCounter++;
Expand All @@ -39,8 +40,7 @@ function sendSmsNotification(identities, body, callback) {
}, function (err) {

if (err) {
log.error('Error sending sms notifications', err);
return callback(err);
return callback(err); // Controlled error
}

log.info('Sent %d SMS messages to [%s]', messageCounter, smsTargets);
Expand Down
7 changes: 4 additions & 3 deletions lib/routes/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ routes.getIdentity = function(req, res, next) {
routes.createIdentity = function(req, res, next) {

identities.createIdentity(req.body, function(err, createdIdentityId) {

if (err) {
return next(new errors.ConflictError('Could not create requested Identity object'));
return next(err);
}

res.status(201).send({id: createdIdentityId});
Expand All @@ -46,7 +47,7 @@ routes.updateIdentity = function(req, res, next) {
identities.updateIdentityData(identityObj, req.body, function(err) {

if (err) {
return next(new errors.BadRequestError('Could not update identity data'));
return next(err);
}

res.sendStatus(204);
Expand All @@ -65,7 +66,7 @@ routes.getIdentityById = function(req, res, next) {
identities.get(identityId, function(err, foundIdentity) {

if (err) {
return next(new errors.NotFoundError('Could not find identity for provided identifier'));
return next(err);
}

if (!foundIdentity) {
Expand Down
2 changes: 1 addition & 1 deletion lib/routes/push.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ routes.sendPushNotification = function(req, res, next) {
orchestrator.sendPushNotifications(req.body, pushPlatform, function(err /*, response */) {

if (err) {
return next(new errors.InternalError('Could not send PUSH Notification to destination'));
return next(err);
}

res.sendStatus(204);
Expand Down
2 changes: 1 addition & 1 deletion lib/routes/sms.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ routes.sendSmsNotification = function(req, res, next) {
orchestrator.sendNotification(req.body, smsPlatform, function(err/*, response */) {

if (err) {
return next(new errors.InternalError('Could not send SMS to destination'));
return next(err); // Controlled error
}

res.sendStatus(204);
Expand Down

0 comments on commit d27099e

Please sign in to comment.