[isomorphic-core] Move buildMime to MessageUtils, add includeBcc option

Summary:
Since buildMime isn't always used explictly for the send operation (we
mostly use it for stuffing the sent folder, which is a separate
operation), move it to MessageUtils. Additionally, add an includeBcc
option. We don't want the BCC header to be visible on outbound messages,
but it needs to be present on the version that is saved to the sent
folder.

When we weren't including the BCC header in the sent folder version, we
were getting an id mismatch between our optimistic message and the
message that gets processed by the sync loop (because we include the bcc
participants when computing the id hash). When the sent message was a
reply, the optimistic message included the threadId, and this had the
side effect of leaving both versions of the sent message in the thread.
Since the optimistic message never receieved it's imap UID, any action
performed on the thread would fail with the missing UID error as seen in
T7941. Adding this BCC header will allow us to compute the proper id
hash and reconcile the synced message with the optimistic message.

Test Plan: manual

Reviewers: evan, mark, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4365

packages/client-sync/src/local-sync-worker/syncback-tasks/ensure-message-in-sent-folder.imap.es6

@@ -1,5 +1,8 @@
-const {SendmailClient, Provider,
-       Errors: {APIError}, MessageUtils: {getReplyHeaders}} = require('isomorphic-core')
+const {
+  Provider,
+  Errors: {APIError},
+  MessageUtils: {getReplyHeaders, buildMime},
+} = require('isomorphic-core')
 const {SyncbackIMAPTask} = require('./syncback-task')
 const SyncTaskFactory = require('../sync-task-factory');
 
@@ -39,8 +42,7 @@ async function* saveSentMessage({db, account, syncWorker, logger, imap, provider
     const sentFolder = yield Folder.find({where: {role: 'sent'}});
     if (!sentFolder) { throw new APIError(`Can't find sent folder - could not save message to sent folder.`) }
 
-    const sender = new SendmailClient(account, logger);
-    const rawMime = yield sender.buildMime(baseMessage);
+    const rawMime = yield buildMime(baseMessage, {includeBcc: true});
     const box = yield imap.openBox(sentFolder.name);
     yield box.append(rawMime, {flags: 'SEEN'});
 
@@ -71,8 +73,7 @@ async function* saveSentMessage({db, account, syncWorker, logger, imap, provider
   // as sent. This is because that means that we sent a message per recipient for
   // tracking, but we actually /just/ want to show the baseMessage as sent
   if (customSentMessage) {
-    const sender = new SendmailClient(account, logger);
-    const rawMime = yield sender.buildMime(baseMessage);
+    const rawMime = yield buildMime(baseMessage, {includeBcc: true});
     const box = yield imap.openBox(allMailFolder.name);
 
     yield box.append(rawMime, {flags: 'SEEN'})

packages/isomorphic-core/src/message-utils.es6

@@ -1,4 +1,5 @@
 /* eslint no-useless-escape: 0 */
+import mailcomposer from 'mailcomposer'
 const mimelib = require('mimelib');
 const encoding = require('encoding');
 const he = require('he');
@@ -437,6 +438,65 @@ async function buildForSend(db, json) {
   return instance;
 }
 
+const formatParticipants = (participants) => {
+  // Something weird happens with the mime building when the participant name
+  // has an @ symbol in it (e.g. a name and email of hello@gmail.com turns into
+  // 'hello@ <gmail.com hello@gmail.com>'), so replace it with whitespace.
+  return participants.map(p => `${p.name.replace('@', ' ')} <${p.email}>`).join(',');
+}
+
+// Transforms the message into a json object with the properties formatted in
+// the way mailer libraries (e.g. nodemailer, mailcomposer) expect.
+function getMailerPayload(message) {
+  const msgData = {};
+  for (const field of ['from', 'to', 'cc', 'bcc']) {
+    if (message[field]) {
+      msgData[field] = formatParticipants(message[field])
+    }
+  }
+  msgData.date = message.date;
+  msgData.subject = message.subject;
+  msgData.html = message.body;
+  msgData.messageId = message.headerMessageId || message.message_id_header;
+
+  msgData.attachments = []
+  const uploads = message.uploads || []
+  for (const upload of uploads) {
+    msgData.attachments.push({
+      filename: upload.filename,
+      content: fs.createReadStream(upload.targetPath),
+      cid: upload.inline ? upload.id : null,
+    })
+  }
+
+  if (message.replyTo) {
+    msgData.replyTo = formatParticipants(message.replyTo);
+  }
+
+  msgData.inReplyTo = message.inReplyTo;
+  msgData.references = message.references;
+  // message.headers is usually unset, but in the case that we do add
+  // headers elsewhere, we don't want to override them here
+  msgData.headers = message.headers || {};
+  msgData.headers['User-Agent'] = `NylasMailer-K2`
+
+  return msgData;
+}
+
+async function buildMime(message, {includeBcc = false} = {}) {
+  const payload = getMailerPayload(message)
+  const builder = mailcomposer(payload)
+  const mimeNode = await (new Promise((resolve, reject) => {
+    builder.build((error, result) => (
+      error ? reject(error) : resolve(result)
+    ))
+  }));
+  if (!includeBcc || !message.bcc || message.bcc.length === 0) {
+    return mimeNode.toString('ascii')
+  }
+  return `Bcc: ${formatParticipants(message.bcc)}\n${mimeNode.toString('ascii')}`
+}
+
 module.exports = {
   buildForSend,
   getReplyHeaders,
@@ -446,4 +506,6 @@ module.exports = {
   stripTrackingLinksFromBody,
   buildTrackingBodyForRecipient,
   replaceMessageIdInBodyTrackingLinks,
+  getMailerPayload,
+  buildMime,
 }

packages/isomorphic-core/src/sendmail-client.es6

@@ -1,19 +1,11 @@
 /* eslint no-useless-escape: 0 */
-import fs from 'fs'
 import nodemailer from 'nodemailer'
-import mailcomposer from 'mailcomposer'
 import {APIError} from './errors'
 import {convertSmtpError} from './smtp-errors'
+import {getMailerPayload, buildMime} from './message-utils'
 
 const MAX_RETRIES = 1;
 
-const formatParticipants = (participants) => {
-  // Something weird happens with the mime building when the participant name
-  // has an @ symbol in it (e.g. a name and email of hello@gmail.com turns into
-  // 'hello@ <gmail.com hello@gmail.com>'), so replace it with whitespace.
-  return participants.map(p => `${p.name.replace('@', ' ')} <${p.email}>`).join(',');
-}
-
 class SendmailClient {
 
   constructor(account, logger) {
@@ -65,58 +57,11 @@ class SendmailClient {
     });
   }
 
-  _getSendPayload(message) {
-    const msgData = {};
-    for (const field of ['from', 'to', 'cc', 'bcc']) {
-      if (message[field]) {
-        msgData[field] = formatParticipants(message[field])
-      }
-    }
-    msgData.date = message.date;
-    msgData.subject = message.subject;
-    msgData.html = message.body;
-    msgData.messageId = message.headerMessageId || message.message_id_header;
-
-    msgData.attachments = []
-    const uploads = message.uploads || []
-    for (const upload of uploads) {
-      msgData.attachments.push({
-        filename: upload.filename,
-        content: fs.createReadStream(upload.targetPath),
-        cid: upload.inline ? upload.id : null,
-      })
-    }
-
-    if (message.replyTo) {
-      msgData.replyTo = formatParticipants(message.replyTo);
-    }
-
-    msgData.inReplyTo = message.inReplyTo;
-    msgData.references = message.references;
-    // message.headers is usually unset, but in the case that we do add
-    // headers elsewhere, we don't want to override them here
-    msgData.headers = message.headers || {};
-    msgData.headers['User-Agent'] = `NylasMailer-K2`
-
-    return msgData;
-  }
-
-  async buildMime(message) {
-    const payload = this._getSendPayload(message)
-    const builder = mailcomposer(payload)
-    const mimeNode = await (new Promise((resolve, reject) => {
-      builder.build((error, result) => (
-        error ? reject(error) : resolve(result)
-      ))
-    }));
-    return mimeNode.toString('ascii')
-  }
-
   async send(message) {
     if (message.isSent) {
       throw new Error(`Cannot send message ${message.id}, it has already been sent`);
     }
-    const payload = this._getSendPayload(message)
+    const payload = getMailerPayload(message)
     await this._send(payload);
   }
 
@@ -126,7 +71,7 @@ class SendmailClient {
       envelope[field] = recipients[field].map(r => r.email);
     }
     envelope.from = customMessage.from.map(c => c.email)
-    const raw = await this.buildMime(customMessage);
+    const raw = await buildMime(customMessage);
     await this._send({raw, envelope});
   }
 }