From 6d31025bf739b5f4bb51b893bfda60babdb017b2 Mon Sep 17 00:00:00 2001 From: Michael DeBonis Date: Tue, 18 Jul 2017 21:11:15 -0500 Subject: [PATCH] Added better error handling --- lib/modules/directChannels.js | 52 +++++++++++++----------- lib/modules/directPosts.js | 64 ++++++++++++++++-------------- lib/modules/posts.js | 53 ++++++++++++++----------- lib/modules/test/fixtures/users.js | 6 +++ lib/modules/test/posts.js | 27 ++++++++++--- lib/modules/test/users.js | 6 ++- lib/modules/users.js | 24 ++++++----- lib/modules/utils.js | 2 +- 8 files changed, 138 insertions(+), 96 deletions(-) diff --git a/lib/modules/directChannels.js b/lib/modules/directChannels.js index 4898049..5734bf4 100644 --- a/lib/modules/directChannels.js +++ b/lib/modules/directChannels.js @@ -37,33 +37,37 @@ module.exports = function(context) { // Define the tranform // transform(function(result, encoding, callback) { - // - // Generate the members array for the - // direct channel - // - let members = Utils.members( - context.values.users, - result.to_jid, - result.from_jid - ) - // - // If there at least two members - // - if(Utils.membersAreValid(members)) { - var key = `${members[0]}|${members[1]}` + try { + // + // Generate the members array for the + // direct channel + // + let members = Utils.members( + context.values.users, + result.to_jid, + result.from_jid + ) // - // And we haven't processed this pair - // already + // If there at least two members // - if(!channels[key]) { - channels[key] = members - log.info(`... writing ${members}`) - context.output.write( - Factory.directChannel({ - members - }) - ) + if(Utils.membersAreValid(members)) { + var key = `${members[0]}|${members[1]}` + // + // And we haven't processed this pair + // already + // + if(!channels[key]) { + channels[key] = members + log.info(`... writing ${members}`) + context.output.write( + Factory.directChannel({ + members + }) + ) + } } + } catch (err) { + log.error(`... ignoring directChannel from: ${result.from_jid} to: ${result.to_jid} on error: ${err.message}.`) } // // Invoke the call to mark that we are diff --git a/lib/modules/directPosts.js b/lib/modules/directPosts.js index 4853572..ce5b68e 100644 --- a/lib/modules/directPosts.js +++ b/lib/modules/directPosts.js @@ -37,40 +37,44 @@ module.exports = function(context) { // Define the tranform // transform(function(message, encoding, callback) { - // - // Generate the members array for the - // direct channel - // - let members = Utils.members( - context.values.users, - message.to_jid, - message.from_jid - ) - // - // Ensure we have at least two channel - // members before we can write the message - // - if (Utils.membersAreValid(members)) { - context.output.write( - Factory.directPost({ - channel_members: members, - user: Utils.username(context.values.users, message.from_jid), - message: Utils.body(message), - create_at: Utils.millis(message.sent_date) - }) + try { + // + // Generate the members array for the + // direct channel + // + let members = Utils.members( + context.values.users, + message.to_jid, + message.from_jid ) // - // Log progress periodically + // Ensure we have at least two channel + // members before we can write the message // - written += 1 - if(written % 1000 == 0) { - log.info(`... wrote ${written} posts`) + if (Utils.membersAreValid(members)) { + context.output.write( + Factory.directPost({ + channel_members: members, + user: Utils.username(context.values.users, message.from_jid), + message: Utils.body(message), + create_at: Utils.millis(message.sent_date) + }) + ) + // + // Log progress periodically + // + written += 1 + if(written % 1000 == 0) { + log.info(`... wrote ${written} posts`) + } + } else { + log.warn({ + to_jid: message.to_jid, + from_jid: message.from_jid + }, '... skipping message with invalid channel members') } - } else { - log.warn({ - to_jid: message.to_jid, - from_jid: message.from_jid - }, '... skipping message with invalid channel members') + } catch (err) { + log.error(`... ignoring directPost from: ${message.from_jid} to: ${message.to_jid} on error: ${err.message}.`) } // // Invoke the call to mark that we are diff --git a/lib/modules/posts.js b/lib/modules/posts.js index 2854ac7..c627a64 100644 --- a/lib/modules/posts.js +++ b/lib/modules/posts.js @@ -41,30 +41,35 @@ module.exports = function(context) { // Define the tranform // transform(function(message, encoding, callback) { - log.debug(message) - // - // Write the post object to the - // output - // - context.output.write( - Factory.post({ - team: context.values.team.name, - channel: Utils.channelName( - context.values.channels, message.to_jid - ), - user: Utils.username( - context.values.users, message.from_jid - ), - message: Utils.body(message), - create_at: Utils.millis(message.sent_date) - }) - ) - // - // Log progress periodically - // - written += 1 - if(written % 1000 == 0) { - log.info(`... wrote ${written} posts`) + try { + log.debug(message) + // + // Write the post object to the + // output + // + context.output.write( + Factory.post({ + team: context.values.team.name, + channel: Utils.channelName( + context.values.channels, message.to_jid + ), + user: Utils.username( + context.values.users, message.from_jid + ), + message: Utils.body(message), + create_at: Utils.millis(message.sent_date) + }) + ) + // + // Log progress periodically + // + written += 1 + if(written % 1000 == 0) { + log.info(`... wrote ${written} posts`) + } + } + catch(err) { + log.error(`... ignoring message id:${message.msg_id} on error: ${err.message}.`) } // // Invoke the call to mark that we are diff --git a/lib/modules/test/fixtures/users.js b/lib/modules/test/fixtures/users.js index 3953501..d659a20 100644 --- a/lib/modules/test/fixtures/users.js +++ b/lib/modules/test/fixtures/users.js @@ -47,5 +47,11 @@ module.exports = [ }, { room_jid: 'tonyd_20161206_1432@conference.example.com', real_jid: 'julie..sokol@example.com' + }, { + real_jid: 'mbcross', + room_jid: null + },{ + real_jid: 'cross,\\20michael@chat.dhs.gov', + room_jid: null } ] diff --git a/lib/modules/test/posts.js b/lib/modules/test/posts.js index 5ca7518..616e4b9 100644 --- a/lib/modules/test/posts.js +++ b/lib/modules/test/posts.js @@ -80,6 +80,7 @@ describe('modules.posts', function() { }) }) + it('should fail on user not found', function(done) { // // Set up the DB @@ -89,14 +90,27 @@ describe('modules.posts', function() { // Process the posts // posts(context).then(function(c) { - expect(c).to.be.undefined + expect(c).to.equal(context) + expect(c.output.write.callCount).to.equal(1) + let post = c.output.write.args[0][0] + expect(post).to.deep.equal({ + type: 'post', + post: { + team: 'test', + channel: 'uat-appsupport', + user: 'micahel.cross', + message: 'I meant thick', + create_at: 1496693318263 + } + }) + done() }).catch(function(e){ - expect(e).to.be.an('error') - expect(e.message).to.equal('user terrence.flynn@example.com not found') + expect(e).to.be.null done() }) }) + it('should fail on body not found', function(done) { // // Set up the DB @@ -106,10 +120,11 @@ describe('modules.posts', function() { // Process the posts // posts(context).then(function(c) { - expect(c).to.be.undefined + expect(c).to.equal(context) + expect(c.output.write.callCount).to.equal(0) + done() }).catch(function(e){ - expect(e).to.be.an('error') - expect(e.message).to.equal('message 3315 body is empty') + expect(e).to.be.null done() }) }) diff --git a/lib/modules/test/users.js b/lib/modules/test/users.js index 246c4e1..6a3a94c 100644 --- a/lib/modules/test/users.js +++ b/lib/modules/test/users.js @@ -5,8 +5,7 @@ const context = require('./context')() describe('modules.users', function() { - it('should process user objects', function(done) { - + before(function(){ context.values = { team: context.config.define.team, channels: { @@ -28,6 +27,9 @@ describe('modules.users', function() { } } } + }) + + it('should process user objects', function(done) { context.jabber.fetch.returns(Promise.resolve({recordset: Fixtures.users})) diff --git a/lib/modules/users.js b/lib/modules/users.js index aabc0b2..6647967 100644 --- a/lib/modules/users.js +++ b/lib/modules/users.js @@ -34,7 +34,6 @@ module.exports = function(context) { // Map of users // var users = {} - // // Iterate over the record set and // assemble the user objects @@ -61,7 +60,8 @@ module.exports = function(context) { }) // // Look up the channel based on the - // room id + // room id. For direct messages, the room_jid + // will be null. // var channel = context.values.channels[record.room_jid] // @@ -71,20 +71,26 @@ module.exports = function(context) { user.teams[0].channels = _.unionBy(user.teams[0].channels, [{ name: channel.name }], 'name') + } else { + record.room_jid && log.warn(`... channel not found for ${record.room_jid}`) } }) - // // Now that the users are assembled, write // them to the output // - _.forEach(users, function(user) { - log.info(`... writing ${user.username}`) - context.output.write( - Factory.user(user) - ) + _.forEach(users, function(user, key) { + try { + context.output.write( + Factory.user(user) + ) + log.info(`... writing ${user.username}`) + } + catch(err) { + log.error(`... ignoring ${user.username} on error: ${err.message}.`) + delete users[key] + } }) - // // Add the users map to the context // diff --git a/lib/modules/utils.js b/lib/modules/utils.js index 7eec946..5c403ea 100644 --- a/lib/modules/utils.js +++ b/lib/modules/utils.js @@ -38,7 +38,7 @@ utils.username = function (users, jid) { } // -// Obtain the username from a jid +// Obtain the channel name from a jid // utils.channelName = function (channels, jid) { return utils.lookup('channel', channels, jid).name