Skip to content

Commit

Permalink
Return uncommon errors as they are to error handler and transform the…
Browse files Browse the repository at this point in the history
…m into 500 HTTP error responses for user.
  • Loading branch information
nick13jaremek authored and igznicolasjaremek committed Aug 28, 2015
1 parent d27099e commit 393522c
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ Feature: A requester attempts to delete some Channel object's data
| 01f0000000000000003f0001 | /api/channel/:channelId | 01f0000000000000006f0001 | 204 |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006f0002 | 204 |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006fasdf | 400 |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006f0004 | 400 |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006f0004 | 404 |
| 01f0000000000000003f0002 | /api/channel/:channelId | "" | 400 |
| 01f0000000000000003f0004 | /api/channel/:channelId | 01f0000000000000006f0001 | 204 |


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Examples:
| identity_id | endpoint | channel_id | status | content |
| 01f0000000000000003f0001 | /api/channel/:channelId | 01f0000000000000006f0001 | 204 | channel/valid_channel_name_one |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006f0003 | 204 | channel/valid_channel_name_two |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006f0004 | 400 | channel/valid_channel_name_two |
| 01f0000000000000003f0002 | /api/channel/:channelId | 01f0000000000000006f0004 | 404 | channel/valid_channel_name_two |

Scenario Outline: check the update attempt of a Channel object based on the request body contents
Given an authenticated identity in the app with <identity_id>
Expand Down
2 changes: 2 additions & 0 deletions cucumber/features/channel_features/channel_get_data.feature
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ Feature: A requester attempts to retrieve the data for a Channel object
| 01f0000000000000003f0002 | /api/channel/:channelId/identities | 01f0000000000000006f0002 | 200 |
| 01f0000000000000003f0003 | /api/channel/:channelId/identities | 01f0000000000000006f0003 | 200 |
| 01f0000000000000003f0003 | /api/channel/:channelId/identities | 01f0000000000000006f0005 | 400 |
| 01f0000000000000003f0003 | /api/channel/:channelId/identities | | 404 |
| 01f0000000000000003f0003 | /api/channel/:channelId/identities | aaaaaa | 400 |


Scenario Outline: check the channel data associated to some other identity object
Expand Down
8 changes: 1 addition & 7 deletions lib/middleware/validate_identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,7 @@ module.exports = function() {
}

if (!identity) {

log.error('Identity not found');
err = new errors.UnauthorizedError('Identity not found');
res.status(err.statusCode).json(err.body);
res.end();
return next(err);
//return next(new errors.UnauthorizedError('Identity not found'));
return next(new errors.UnauthorizedError('Identity not found'));
} else {

req.identity = identity;
Expand Down
28 changes: 12 additions & 16 deletions lib/platforms/channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ function retrieveChannelDataForIdentity(identity, callback) {

identities.get(identity, function(err, foundIdentity) {

if (err || !foundIdentity) {
return callback(new errors.BadRequestError('Could not find Identity object'));
if (err) {
return callback(err);
}

Channel.find().where('name').in(foundIdentity.channels).exec(function(err, foundChannels) {

if (err) {
return callback(new errors.BadRequestError('Could not fetch the channels associated to the provided Identity object'));
return callback(err);
}

var channelData = [];
Expand All @@ -51,7 +51,7 @@ function createChannel(channelData, callback) {
Channel.findOne({name: channelData.name}, function(err, results) {

if (err) {
return callback(new errors.InternalError('Channel creation process failed'));
return callback(err);
}

if (results && !_.isEmpty(results)) {
Expand All @@ -65,7 +65,7 @@ function createChannel(channelData, callback) {
channel.save(function(err, savedChannel) {

if (err) {
return callback(new errors.InternalError('Could not save newly created Channel object to database'));
return callback(err);
}

return callback(null, savedChannel);
Expand All @@ -91,18 +91,14 @@ function preProcessChannel(channelToProcess, changes) {

function deleteChannel(channelId, callback) {

if (!channelId) {
return callback(new errors.BadRequestError('Missing channel id parameter'));
}

Channel.findOneAndRemove({'_id': channelId}, function(err, deletedChannel) {

if (err) {
return callback(new errors.InternalError('Could not fetch Channel object to delete'));
return callback(err);
}

if (!deletedChannel) {
return callback(new errors.BadRequestError('Requested channel not found in database'));
return callback(new errors.NotFoundError('Requested channel not found in database'));
}

var relatedIdentities = deletedChannel.identityRef;
Expand Down Expand Up @@ -130,11 +126,11 @@ function updateChannel(channelId, changes, callback) {
getChannel(channelId, function(err, foundChannel) {

if (err) {
return done(new errors.InternalError('Could not fetch Channel object to edit'));
return done(err);
}

if (!foundChannel) {
return done(new errors.BadRequestError('Requested Channel object not found in database'));
return done(new errors.NotFoundError('Requested Channel object not found in database'));
}

oldChannel = _.clone(foundChannel.toObject());
Expand All @@ -144,7 +140,7 @@ function updateChannel(channelId, changes, callback) {
foundChannel.save(foundChannel, function(err, updatedChannel) {

if (err) {
return done(new errors.InternalError('Could not update requested Channel object'));
return done(err);
}

return done(null, oldChannel, updatedChannel);
Expand Down Expand Up @@ -180,7 +176,7 @@ function retrieveIdentityListForChannel(channelId, callback) {
getChannel(channelId, function(err, foundChannel) {

if (err) {
return callback(new errors.InternalError('Could not fetch requested Channel object'));
return callback(err);
}

if (!foundChannel) {
Expand All @@ -194,7 +190,7 @@ function retrieveIdentityListForChannel(channelId, callback) {
identities.findIdentitiesByFieldValue('_id', foundChannel.identityRef, function(err, foundIdentities) {

if (err) {
return callback(new errors.InternalError('Could not fetch Identities for the provided Channel object'));
return callback(err);
}

var identityList = [];
Expand Down
3 changes: 1 addition & 2 deletions lib/platforms/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ 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 @@ -18,7 +17,7 @@ function sendEmailNotification(identities, body, callback) {
transport.send(emails, body.content, function (err, response, body) {

if (err) {
return callback(new errors.BadRequestError('Could not send email notification to destinations'));
return callback(err);
}

var output = {
Expand Down
26 changes: 13 additions & 13 deletions lib/platforms/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function updateIdentityData(identity, changes, callback) {
checkForIdentityExistence(identity, function(err, result) {

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

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

if (err) {
return done(new errors.InternalError('Could not update identity object data'));
return done(err);
}

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

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

return done(null, output);
Expand All @@ -87,7 +87,7 @@ function updateIdentityData(identity, changes, callback) {
function (err, output) {

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

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

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

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

if (err) {
return done(new errors.InternalError('Could not save new identity object'));
return done(err);
}

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

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

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

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

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

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

var duplicatedIdentities =_.filter(result, function(item) {
Expand Down Expand Up @@ -302,7 +302,7 @@ function subscribeIdentityToChannels(subscribedIdentity, channelNames, callback)
Channel.find({'name': {$in: channelNames}}, function(err, foundChannels) {

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

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(new InternalError('Could not subscribe identity to channels'));
return microDone(err);
}

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(new InternalError('Could not create relationship from channels to identity'));
return microDone(err);
}

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

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

return callback(null, subscribedIdentity);
Expand Down
11 changes: 5 additions & 6 deletions lib/platforms/orchestrator.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ 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 @@ -66,7 +65,7 @@ function sendPushNotifications(requestBody, platformModule, callback) {
}, function(err) {

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

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

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

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

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

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

if (err) {
return nextBatch(new errors.InternalError('Could not find identity objects for batch %d', page));
return nextBatch(err);
}

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

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

return nextBatch();
Expand Down
5 changes: 2 additions & 3 deletions lib/platforms/push.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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 @@ -45,14 +44,14 @@ function deleteApnDevices(devices, callback){
Identity.update({'devices.apn':{$in:[device]}}, {$pull:{'devices.apn':device}}, function (err) {

if (err) {
return done(new errors.InternalError('Could not delete devices from identity'));
return done(err);
}
return done();
});
}, function(err){

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

callback(null);
Expand Down
5 changes: 2 additions & 3 deletions lib/platforms/sms.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ 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 @@ -31,7 +30,7 @@ function sendSmsNotification(identities, body, callback) {
transport.sendSms(sms, function (err) {

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

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

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

log.info('Sent %d SMS messages to [%s]', messageCounter, smsTargets);
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(err); // Controlled error
return next(err);
}

res.sendStatus(204);
Expand Down

0 comments on commit 393522c

Please sign in to comment.